Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provider/aws: Add IAMGroupMembership resource #2273

Merged
merged 6 commits into from
Jun 9, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions builtin/providers/aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func Provider() terraform.ResourceProvider {
"aws_iam_access_key": resourceAwsIamAccessKey(),
"aws_iam_group_policy": resourceAwsIamGroupPolicy(),
"aws_iam_group": resourceAwsIamGroup(),
"aws_iam_group_membership": resourceAwsIamGroupMembership(),
"aws_iam_instance_profile": resourceAwsIamInstanceProfile(),
"aws_iam_policy": resourceAwsIamPolicy(),
"aws_iam_role_policy": resourceAwsIamRolePolicy(),
Expand Down
156 changes: 156 additions & 0 deletions builtin/providers/aws/resource_aws_iam_group_membership.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package aws

import (
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/terraform/helper/schema"
)

func resourceAwsIamGroupMembership() *schema.Resource {
return &schema.Resource{
Create: resourceAwsIamGroupMembershipCreate,
Read: resourceAwsIamGroupMembershipRead,
Update: resourceAwsIamGroupMembershipUpdate,
Delete: resourceAwsIamGroupMembershipDelete,

Schema: map[string]*schema.Schema{
"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},

"users": &schema.Schema{
Type: schema.TypeSet,
Required: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},

"group": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
},
}
}

func resourceAwsIamGroupMembershipCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).iamconn

group := d.Get("group").(string)
userList := expandStringList(d.Get("users").(*schema.Set).List())

if err := addUsersToGroup(conn, userList, group); err != nil {
return err
}

d.SetId(d.Get("name").(string))
return resourceAwsIamGroupMembershipRead(d, meta)
}

func resourceAwsIamGroupMembershipRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).iamconn
group := d.Get("group").(string)
resp, err := conn.GetGroup(&iam.GetGroupInput{
GroupName: aws.String(group),
})

if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
// aws specific error
if awsErr.Code() == "NoSuchEntity" {
// group not found
d.SetId("")
return nil
}
}
return err
}

ul := make([]string, 0, len(resp.Users))
for _, u := range resp.Users {
ul = append(ul, *u.UserName)
}

if err := d.Set("users", ul); err != nil {
return fmt.Errorf("[WARN] Error setting user list from IAM Group Membership (%s), error: %s", group, err)
}

return nil
}

func resourceAwsIamGroupMembershipUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).iamconn

if d.HasChange("users") {
group := d.Get("group").(string)

o, n := d.GetChange("users")
if o == nil {
o = new(schema.Set)
}
if n == nil {
n = new(schema.Set)
}

os := o.(*schema.Set)
ns := n.(*schema.Set)
remove := expandStringList(os.Difference(ns).List())
add := expandStringList(ns.Difference(os).List())

if err := removeUsersFromGroup(conn, remove, group); err != nil {
return err
}

if err := addUsersToGroup(conn, add, group); err != nil {
return err
}
}

return resourceAwsIamGroupMembershipRead(d, meta)
}

func resourceAwsIamGroupMembershipDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).iamconn
userList := expandStringList(d.Get("users").(*schema.Set).List())
group := d.Get("group").(string)

if err := removeUsersFromGroup(conn, userList, group); err != nil {
return err
}

return nil
}

func removeUsersFromGroup(conn *iam.IAM, users []*string, group string) error {
for _, u := range users {
_, err := conn.RemoveUserFromGroup(&iam.RemoveUserFromGroupInput{
UserName: u,
GroupName: aws.String(group),
})

if err != nil {
return err
}
}
return nil
}

func addUsersToGroup(conn *iam.IAM, users []*string, group string) error {
for _, u := range users {
_, err := conn.AddUserToGroup(&iam.AddUserToGroupInput{
UserName: u,
GroupName: aws.String(group),
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these API calls idempotent? Should make sure we properly handle the scenario where we add some users, then crash, then need to add the rest of the users in a follow up run.

Might be as simple as catching an "already in that group" error code here, or perhaps its already considered an AWS success. Either way, worth checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are idempotent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay! 💃


if err != nil {
return err
}
}
return nil
}
169 changes: 169 additions & 0 deletions builtin/providers/aws/resource_aws_iam_group_membership_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
package aws

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestAccAWSGroupMembership_basic(t *testing.T) {
var group iam.GetGroupOutput

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSGroupMembershipDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSGroupMemberConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGroupMembershipExists("aws_iam_group_membership.team", &group),
testAccCheckAWSGroupMembershipAttributes(&group, []string{"test-user"}),
),
},

resource.TestStep{
Config: testAccAWSGroupMemberConfigUpdate,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGroupMembershipExists("aws_iam_group_membership.team", &group),
testAccCheckAWSGroupMembershipAttributes(&group, []string{"test-user-two", "test-user-three"}),
),
},
},
})
}

func testAccCheckAWSGroupMembershipDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).iamconn

for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_iam_group_membership" {
continue
}

group := rs.Primary.Attributes["group"]

resp, err := conn.GetGroup(&iam.GetGroupInput{
GroupName: aws.String(group),
})
if err != nil {
// might error here
return err
}

users := []string{"test-user", "test-user-two", "test-user-three"}
for _, u := range resp.Users {
for _, i := range users {
if i == *u.UserName {
return fmt.Errorf("Error: User (s) still a member of Group (%s)", i, *resp.Group.GroupName)
}
}
}

}

return nil
}

func testAccCheckAWSGroupMembershipExists(n string, g *iam.GetGroupOutput) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No User name is set")
}

conn := testAccProvider.Meta().(*AWSClient).iamconn
gn := rs.Primary.Attributes["group"]

resp, err := conn.GetGroup(&iam.GetGroupInput{
GroupName: aws.String(gn),
})

if err != nil {
return fmt.Errorf("Error: Group (%s) not found", gn)
}

*g = *resp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here - looks like this is just examining the IAM group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning here is this:

  • Lines 67-70 (or 74-, after 4d59019) check if it exists in the state file (since it's not truly a thing on AWS).
  • We then grab the group, and find it from AWS, and return it
  • Pass on that group result to testAccCheckAWSGroupMembershipAttributes
  • Actual membership is checked there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh sure I get it now. So this method is really more like testAccFetchGroup - it's not much of a testAccCheckAWSGroupMembershipExists because really do any checking that the group membership exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that's true; I was following a convention of <thing we're testing>Exists being a method that retrieves a thing for more testing. Except my a thing doesn't exist on AWS's side.

I can rename if you think it's necessary


return nil
}
}

func testAccCheckAWSGroupMembershipAttributes(group *iam.GetGroupOutput, users []string) resource.TestCheckFunc {
return func(s *terraform.State) error {
if *group.Group.GroupName != "test-group" {
return fmt.Errorf("Bad group membership: expected %s, got %s", "test-group", *group.Group.GroupName)
}

uc := len(users)
for _, u := range users {
for _, gu := range group.Users {
if u == *gu.UserName {
uc--
}
}
}

if uc > 0 {
return fmt.Errorf("Bad group membership count, expected (%d), but only (%d) found", len(users), uc)
}
return nil
}
}

const testAccAWSGroupMemberConfig = `
resource "aws_iam_group" "group" {
name = "test-group"
path = "/"
}

resource "aws_iam_user" "user" {
name = "test-user"
path = "/"
}

resource "aws_iam_group_membership" "team" {
name = "tf-testing-group-membership"
users = ["${aws_iam_user.user.name}"]
group = "${aws_iam_group.group.name}"
}
`

const testAccAWSGroupMemberConfigUpdate = `
resource "aws_iam_group" "group" {
name = "test-group"
path = "/"
}

resource "aws_iam_user" "user" {
name = "test-user"
path = "/"
}

resource "aws_iam_user" "user_two" {
name = "test-user-two"
path = "/"
}

resource "aws_iam_user" "user_three" {
name = "test-user-three"
path = "/"
}

resource "aws_iam_group_membership" "team" {
name = "tf-testing-group-membership"
users = [
"${aws_iam_user.user_two.name}",
"${aws_iam_user.user_three.name}",
]
group = "${aws_iam_group.group.name}"
}
`
4 changes: 4 additions & 0 deletions website/source/layouts/aws.erb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@
<a href="/docs/providers/aws/r/iam_group_policy.html">aws_iam_group_policy</a>
</li>

<li<%= sidebar_current("docs-aws-resource-iam-group-membership") %>>
<a href="/docs/providers/aws/r/iam_group_membership.html">aws_iam_group_membership</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a file here? Can't find the page this links to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

derp, added

</li>

<li<%= sidebar_current("docs-aws-resource-iam-instance-profile") %>>
<a href="/docs/providers/aws/r/iam_instance_profile.html">aws_iam_instance_profile</a>
</li>
Expand Down