Add members attribute to iam_group data source #7132

teraken0509 · 2019-01-14T10:35:33Z

Changes proposed in this pull request:

Add members attribute
Modify document

Output from acceptance testing:

$  make testacc TEST=./aws TESTARGS='-run=TestAccAWSDataSourceIAMGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSDataSourceIAMGroup_ -timeout 120m
=== RUN   TestAccAWSDataSourceIAMGroup_basic
=== PAUSE TestAccAWSDataSourceIAMGroup_basic
=== RUN   TestAccAWSDataSourceIAMGroup_member
=== PAUSE TestAccAWSDataSourceIAMGroup_member
=== CONT  TestAccAWSDataSourceIAMGroup_basic
=== CONT  TestAccAWSDataSourceIAMGroup_member
--- PASS: TestAccAWSDataSourceIAMGroup_basic (25.23s)
--- PASS: TestAccAWSDataSourceIAMGroup_member (31.90s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	31.950s

maxenglander · 2019-03-25T13:22:37Z

Hi @bflad is there anything blocking this from being merged/released?

bflad

Hi @kterada0509 👋 Thanks so much for submitting this and apologies for the delayed review. Overall this is looking really good, just a few minor things below and we should be able to get this in. Please reach out if you have any questions or do not have time to implement these items.

bflad · 2019-11-07T11:31:17Z

aws/data_source_aws_iam_group.go

@@ -58,6 +82,20 @@ func dataSourceAwsIAMGroupRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("arn", group.Arn)
 	d.Set("path", group.Path)
 	d.Set("group_id", group.GroupId)
+	d.Set("members", dataSourceUsersRead(resp.Users))


When using d.Set() with aggregate types (TypeList, TypeSet, TypeMap), we should perform error checking to prevent issues where the code is not properly able to set the Terraform state. 👍

Suggested change

d.Set("members", dataSourceUsersRead(resp.Users))

if err := d.Set("members", dataSourceUsersRead(resp.Users)); err != nil {

return fmt.Errorf("error setting members: %s", err)

}

bflad · 2019-11-07T11:33:47Z

aws/data_source_aws_iam_group_test.go

+					resource.TestCheckResourceAttrSet("data.aws_iam_group.test", "group_id"),
+					resource.TestCheckResourceAttr("data.aws_iam_group.test", "path", "/"),
+					resource.TestCheckResourceAttr("data.aws_iam_group.test", "group_name", groupName),
+					resource.TestMatchResourceAttr("data.aws_iam_group.test", "arn", regexp.MustCompile("^arn:aws:iam::[0-9]{12}:group/"+groupName)),


We can simplify this and allow this testing to work in multiple AWS partitions with the below:

Suggested change

resource.TestMatchResourceAttr("data.aws_iam_group.test", "arn", regexp.MustCompile("^arn:aws:iam::[0-9]{12}:group/"+groupName)),

testAccCheckResourceAttrGlobalARN("data.aws_iam_group.test", "arn", "iam", fmt.Sprintf("group/%s", groupName)),

bflad · 2019-11-07T11:36:03Z

aws/data_source_aws_iam_group_test.go

+					resource.TestMatchResourceAttr("data.aws_iam_group.test", "arn", regexp.MustCompile("^arn:aws:iam::[0-9]{12}:group/"+groupName)),
+					resource.TestCheckResourceAttr("data.aws_iam_group.test", "members.#", "1"),
+					resource.TestCheckResourceAttrPair("data.aws_iam_group.test", "members.0.arn", "aws_iam_user.user", "arn"),
+					resource.TestCheckResourceAttrSet("data.aws_iam_group.test", "members.0.user_id"),


I believe this can be compared using the aws_iam_user resource unique_id attribute:

Suggested change

resource.TestCheckResourceAttrSet("data.aws_iam_group.test", "members.0.user_id"),

resource.TestCheckResourceAttrPair("data.aws_iam_group.test", "members.0.user_id", "aws_iam_user.user", "unique_id"),

bflad · 2019-11-07T11:40:03Z

website/docs/d/iam_group.html.markdown

@@ -31,3 +31,15 @@ data "aws_iam_group" "example" {
 * `path` - The path to the group.

 * `group_id` - The stable and unique string identifying the group.
+
+* `members` - The member of group. See supported fields below.


With Terraform 0.12, our documentation wording for explaining types is a little more important. 😄 Could you please note that this is a list of objects? e.g.

Suggested change

* `members` - The member of group. See supported fields below.

* `members` - List of objects containing group member information. See supported fields below.

Thanks, i updated the docs

bflad · 2019-11-07T11:42:25Z

aws/data_source_aws_iam_group.go

@@ -30,6 +30,30 @@ func dataSourceAwsIAMGroup() *schema.Resource {
 				Type:     schema.TypeString,
 				Required: true,
 			},
+			"members": {


Just noting here, that we may want to consider naming this attribute users to match the API. 👍

Thanks, i fixed.

teraken0509 · 2019-11-08T08:29:30Z

Re-run acctest.

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSDataSourceIAMGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDataSourceIAMGroup_ -timeout 120m
=== RUN   TestAccAWSDataSourceIAMGroup_basic
=== PAUSE TestAccAWSDataSourceIAMGroup_basic
=== RUN   TestAccAWSDataSourceIAMGroup_users
=== PAUSE TestAccAWSDataSourceIAMGroup_users
=== CONT  TestAccAWSDataSourceIAMGroup_basic
=== CONT  TestAccAWSDataSourceIAMGroup_users
--- PASS: TestAccAWSDataSourceIAMGroup_basic (33.84s)
--- PASS: TestAccAWSDataSourceIAMGroup_users (38.82s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	38.896s

bflad

Looks great, thanks so much for this, @kterada0509 🚀

Output from acceptance testing:

--- PASS: TestAccAWSDataSourceIAMGroup_basic (10.27s)
--- PASS: TestAccAWSDataSourceIAMGroup_users (12.62s)

ghost · 2019-11-14T21:39:02Z

This has been released in version 2.36.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2019-12-09T14:50:32Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

bflad added new-data-source Introduces a new data source. enhancement Requests to existing resources that expand the functionality or scope. and removed new-data-source Introduces a new data source. labels Jan 15, 2019

aeschright requested a review from a team June 25, 2019 21:45

bflad mentioned this pull request Nov 7, 2019

IAM Group Data Source does not return iam members in group #10652

Closed

bflad suggested changes Nov 7, 2019

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Nov 7, 2019

bflad mentioned this pull request Nov 7, 2019

data-source/data_source_aws_iam_group: Retrieve users that are associated to the group #10782

Closed

teraken0509 and others added 5 commits November 8, 2019 16:55

Add member attribute

66586d6

Add error handling for members attribute setting

29da7a6

fix attribute name from members to users

919928c

fix arn test

cd909d1

fix attribute name & add data type to document

a81380b

teraken0509 force-pushed the feature/add-members-attribute-for-iam-group-data-source branch from 57c114f to a81380b Compare November 8, 2019 08:27

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Nov 8, 2019

teraken0509 requested a review from bflad November 8, 2019 08:29

bflad added this to the v2.36.0 milestone Nov 8, 2019

bflad approved these changes Nov 8, 2019

View reviewed changes

bflad merged commit 2b61253 into hashicorp:master Nov 8, 2019

bflad added a commit that referenced this pull request Nov 8, 2019

Update CHANGELOG for #7132

a9e95f9

bflad mentioned this pull request Nov 19, 2019

set users for data_iam_group #10622

Closed

ghost locked and limited conversation to collaborators Dec 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add members attribute to iam_group data source #7132

Add members attribute to iam_group data source #7132

teraken0509 commented Jan 14, 2019

maxenglander commented Mar 25, 2019

bflad left a comment

bflad Nov 7, 2019

teraken0509 Nov 8, 2019

bflad Nov 7, 2019

teraken0509 Nov 8, 2019

bflad Nov 7, 2019

teraken0509 Nov 8, 2019

bflad Nov 7, 2019

teraken0509 Nov 8, 2019

bflad Nov 7, 2019

teraken0509 Nov 8, 2019

teraken0509 commented Nov 8, 2019

bflad left a comment

ghost commented Nov 14, 2019

ghost commented Dec 9, 2019

-	d.Set("members", dataSourceUsersRead(resp.Users))
+	if err := d.Set("members", dataSourceUsersRead(resp.Users)); err != nil {
+		return fmt.Errorf("error setting members: %s", err)
+	}

	resource.TestMatchResourceAttr("data.aws_iam_group.test", "arn", regexp.MustCompile("^arn:aws:iam::[0-9]{12}:group/"+groupName)),
	testAccCheckResourceAttrGlobalARN("data.aws_iam_group.test", "arn", "iam", fmt.Sprintf("group/%s", groupName)),

	resource.TestCheckResourceAttrSet("data.aws_iam_group.test", "members.0.user_id"),
	resource.TestCheckResourceAttrPair("data.aws_iam_group.test", "members.0.user_id", "aws_iam_user.user", "unique_id"),

	* `members` - The member of group. See supported fields below.
	* `members` - List of objects containing group member information. See supported fields below.

Add members attribute to iam_group data source #7132

Add members attribute to iam_group data source #7132

Conversation

teraken0509 commented Jan 14, 2019

maxenglander commented Mar 25, 2019

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teraken0509 commented Nov 8, 2019

bflad left a comment

Choose a reason for hiding this comment

ghost commented Nov 14, 2019

ghost commented Dec 9, 2019