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.

Sign up for GitHub

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

Jump to bottom

provider/aws: add support for group name and path changes with group update function #3237

Merged
merged 4 commits into from
Nov 17, 2015
Merged

provider/aws: add support for group name and path changes with group update function #3237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f5267df
add support for group name and path changes with group update function
Sep 15, 2015
9ab5596
will not swallow error on aws iam group update if we get NoSuchEntity…
pgray Oct 5, 2015
6a2d3ea
remove unneeded print function
Oct 31, 2015
8fcc13a
update aws_iam_group tests to check proper paths and name
Oct 31, 2015
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
31 changes: 25 additions & 6 deletions builtin/providers/aws/resource_aws_iam_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ func resourceAwsIamGroup() *schema.Resource {
return &schema.Resource{
Create: resourceAwsIamGroupCreate,
Read: resourceAwsIamGroupRead,
// TODO
//Update: resourceAwsIamGroupUpdate,
Update: resourceAwsIamGroupUpdate,
Delete: resourceAwsIamGroupDelete,

Schema: map[string]*schema.Schema{
Expand All @@ -30,13 +29,11 @@ func resourceAwsIamGroup() *schema.Resource {
"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"path": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "/",
ForceNew: true,
},
},
}
Expand All @@ -45,9 +42,10 @@ func resourceAwsIamGroup() *schema.Resource {
func resourceAwsIamGroupCreate(d *schema.ResourceData, meta interface{}) error {
iamconn := meta.(*AWSClient).iamconn
name := d.Get("name").(string)
path := d.Get("path").(string)

request := &iam.CreateGroupInput{
Path: aws.String(d.Get("path").(string)),
Path: aws.String(path),
GroupName: aws.String(name),
}

Expand All @@ -60,9 +58,10 @@ func resourceAwsIamGroupCreate(d *schema.ResourceData, meta interface{}) error {

func resourceAwsIamGroupRead(d *schema.ResourceData, meta interface{}) error {
iamconn := meta.(*AWSClient).iamconn
name := d.Get("name").(string)

request := &iam.GetGroupInput{
GroupName: aws.String(d.Id()),
GroupName: aws.String(name),
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems superfluous; name isn't used outside of this iam.GetGroupInput struct.
Does it serve some purpose I don't see?

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 thought it made the code clearer.

}

getResp, err := iamconn.GetGroup(request)
Expand Down Expand Up @@ -93,6 +92,26 @@ func resourceAwsIamGroupReadResult(d *schema.ResourceData, group *iam.Group) err
return nil
}

func resourceAwsIamGroupUpdate(d *schema.ResourceData, meta interface{}) error {
if d.HasChange("name") || d.HasChange("path") {
iamconn := meta.(*AWSClient).iamconn
on, nn := d.GetChange("name")
op, np := d.GetChange("path")

request := &iam.UpdateGroupInput{
GroupName: aws.String(on.(string)),
NewGroupName: aws.String(nn.(string)),
NewPath: aws.String(np.(string)),
}
_, err := iamconn.UpdateGroup(request)
if err != nil {
return fmt.Errorf("Error updating IAM Group %s: %s", d.Id(), err)
}
return resourceAwsIamGroupRead(d, meta)
}
return nil
}

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

Expand Down
25 changes: 19 additions & 6 deletions builtin/providers/aws/resource_aws_iam_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@ func TestAccAWSIAMGroup_basic(t *testing.T) {
Config: testAccAWSGroupConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGroupExists("aws_iam_group.group", &conf),
testAccCheckAWSGroupAttributes(&conf),
testAccCheckAWSGroupAttributes(&conf, "test-group", "/"),
),
},
resource.TestStep{
Config: testAccAWSGroupConfig2,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGroupExists("aws_iam_group.group", &conf),
testAccCheckAWSGroupAttributes(&conf, "test-group2", "/funnypath/"),
),
},
},
Expand Down Expand Up @@ -85,14 +92,14 @@ func testAccCheckAWSGroupExists(n string, res *iam.GetGroupOutput) resource.Test
}
}

func testAccCheckAWSGroupAttributes(group *iam.GetGroupOutput) resource.TestCheckFunc {
func testAccCheckAWSGroupAttributes(group *iam.GetGroupOutput, name string, path string) resource.TestCheckFunc {
return func(s *terraform.State) error {
if *group.Group.GroupName != "test-group" {
return fmt.Errorf("Bad name: %s", *group.Group.GroupName)
if *group.Group.GroupName != name {
return fmt.Errorf("Bad name: %s when %s was expected", *group.Group.GroupName, name)
}

if *group.Group.Path != "/" {
return fmt.Errorf("Bad path: %s", *group.Group.Path)
if *group.Group.Path != path {
return fmt.Errorf("Bad path: %s when %s was expected", *group.Group.Path, path)
}

return nil
Expand All @@ -105,3 +112,9 @@ resource "aws_iam_group" "group" {
path = "/"
}
`
const testAccAWSGroupConfig2 = `
resource "aws_iam_group" "group" {
name = "test-group2"
path = "/funnypath/"
Copy link
Contributor

Choose a reason for hiding this comment

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

won't this fail the test? Maybe we could supply testAccCheckAWSGroupAttributes with the expected path, and compare them

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 made the testAccCheckAWSGroupAttributes function handle for the expected names and paths. Hope it makes sense.

}
`