r/iam_user: support tags #6497

kl4w · 2018-11-16T18:04:56Z

Fixes #0000

Changes proposed in this pull request:

support for tags

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSUser_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSUser_ -timeout 120m
=== RUN   TestAccAWSUser_importBasic
=== PAUSE TestAccAWSUser_importBasic
=== RUN   TestAccAWSUser_basic
=== PAUSE TestAccAWSUser_basic
=== RUN   TestAccAWSUser_disappears
=== PAUSE TestAccAWSUser_disappears
=== RUN   TestAccAWSUser_ForceDestroy_AccessKey
=== PAUSE TestAccAWSUser_ForceDestroy_AccessKey
=== RUN   TestAccAWSUser_ForceDestroy_LoginProfile
=== PAUSE TestAccAWSUser_ForceDestroy_LoginProfile
=== RUN   TestAccAWSUser_ForceDestroy_MFADevice
=== PAUSE TestAccAWSUser_ForceDestroy_MFADevice
=== RUN   TestAccAWSUser_ForceDestroy_SSHKey
=== PAUSE TestAccAWSUser_ForceDestroy_SSHKey
=== RUN   TestAccAWSUser_nameChange
=== PAUSE TestAccAWSUser_nameChange
=== RUN   TestAccAWSUser_pathChange
=== PAUSE TestAccAWSUser_pathChange
=== RUN   TestAccAWSUser_permissionsBoundary
=== PAUSE TestAccAWSUser_permissionsBoundary
=== RUN   TestAccAWSUser_tags
=== PAUSE TestAccAWSUser_tags
=== CONT  TestAccAWSUser_importBasic
=== CONT  TestAccAWSUser_ForceDestroy_SSHKey
=== CONT  TestAccAWSUser_ForceDestroy_AccessKey
=== CONT  TestAccAWSUser_ForceDestroy_MFADevice
=== CONT  TestAccAWSUser_ForceDestroy_LoginProfile
=== CONT  TestAccAWSUser_disappears
=== CONT  TestAccAWSUser_pathChange
=== CONT  TestAccAWSUser_permissionsBoundary
=== CONT  TestAccAWSUser_nameChange
=== CONT  TestAccAWSUser_basic
=== CONT  TestAccAWSUser_tags
--- PASS: TestAccAWSUser_disappears (6.95s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (8.92s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (8.98s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (9.04s)
--- PASS: TestAccAWSUser_importBasic (9.09s)
--- PASS: TestAccAWSUser_pathChange (13.52s)
--- PASS: TestAccAWSUser_basic (13.58s)
--- PASS: TestAccAWSUser_nameChange (13.60s)
--- PASS: TestAccAWSUser_tags (14.04s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (24.69s)
--- PASS: TestAccAWSUser_permissionsBoundary (31.57s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	31.618s

kl4w · 2018-11-16T19:10:43Z

references #6492

make testacc TEST=./aws TESTARGS='-run=TestAccAWSUser_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSUser_ -timeout 120m
=== RUN   TestAccAWSUser_importBasic
=== PAUSE TestAccAWSUser_importBasic
=== RUN   TestAccAWSUser_basic
=== PAUSE TestAccAWSUser_basic
=== RUN   TestAccAWSUser_disappears
=== PAUSE TestAccAWSUser_disappears
=== RUN   TestAccAWSUser_ForceDestroy_AccessKey
=== PAUSE TestAccAWSUser_ForceDestroy_AccessKey
=== RUN   TestAccAWSUser_ForceDestroy_LoginProfile
=== PAUSE TestAccAWSUser_ForceDestroy_LoginProfile
=== RUN   TestAccAWSUser_ForceDestroy_MFADevice
=== PAUSE TestAccAWSUser_ForceDestroy_MFADevice
=== RUN   TestAccAWSUser_ForceDestroy_SSHKey
=== PAUSE TestAccAWSUser_ForceDestroy_SSHKey
=== RUN   TestAccAWSUser_nameChange
=== PAUSE TestAccAWSUser_nameChange
=== RUN   TestAccAWSUser_pathChange
=== PAUSE TestAccAWSUser_pathChange
=== RUN   TestAccAWSUser_permissionsBoundary
=== PAUSE TestAccAWSUser_permissionsBoundary
=== RUN   TestAccAWSUser_tags
=== PAUSE TestAccAWSUser_tags
=== CONT  TestAccAWSUser_importBasic
=== CONT  TestAccAWSUser_ForceDestroy_SSHKey
=== CONT  TestAccAWSUser_basic
=== CONT  TestAccAWSUser_ForceDestroy_AccessKey
=== CONT  TestAccAWSUser_nameChange
=== CONT  TestAccAWSUser_ForceDestroy_MFADevice
=== CONT  TestAccAWSUser_ForceDestroy_LoginProfile
=== CONT  TestAccAWSUser_disappears
=== CONT  TestAccAWSUser_permissionsBoundary
=== CONT  TestAccAWSUser_tags
=== CONT  TestAccAWSUser_pathChange
--- PASS: TestAccAWSUser_disappears (6.40s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (8.61s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (8.62s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (8.67s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (8.85s)
--- PASS: TestAccAWSUser_importBasic (8.98s)
--- PASS: TestAccAWSUser_pathChange (12.84s)
--- PASS: TestAccAWSUser_basic (12.92s)
--- PASS: TestAccAWSUser_nameChange (13.04s)
--- PASS: TestAccAWSUser_tags (13.34s)
--- PASS: TestAccAWSUser_permissionsBoundary (30.70s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	30.744s

bflad

Hi @kl4w 👋 Thanks as usual for contributing. Left some initial comments below. Please let us know if you have any questions or do not have time to implement the feedback.

bflad · 2018-11-19T03:01:07Z

aws/resource_aws_iam_user.go

@@ -121,6 +127,7 @@ func resourceAwsIamUserRead(d *schema.ResourceData, meta interface{}) error {
 		d.Set("permissions_boundary", output.User.PermissionsBoundary.PermissionsBoundaryArn)
 	}
 	d.Set("unique_id", output.User.UserId)
+	d.Set("tags", tagsToMapIAM(output.User.Tags))


We should perform error checking here to catch any unexpected type conversion issues. 👍

Suggested change

d.Set("tags", tagsToMapIAM(output.User.Tags))

if err := d.Set("tags", tagsToMapIAM(output.User.Tags)); err != nil {

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

}

bflad · 2018-11-19T03:02:20Z

aws/resource_aws_iam_user.go

+		n := nraw.(map[string]interface{})
+		c, r := diffTagsIAM(tagsFromMapIAM(o), tagsFromMapIAM(n))
+
+		_, untagErr := iamconn.UntagUser(&iam.UntagUserInput{


We should perform length checks here to ensure we don't try to call UntagUser with an empty set of TagKeys, e.g.

if len(r) > 0 { // existing logic }

bflad · 2018-11-19T03:03:15Z

aws/resource_aws_iam_user.go

+			return fmt.Errorf("error deleting IAM user tags: %s", untagErr)
+		}
+
+		input := &iam.TagUserInput{


We should perform length check here to ensure we don't try to call TagUser with an empty Tags, e.g.

if len(c) > 0 { // existing logic }

bflad · 2018-11-19T03:11:43Z

aws/tagsIAM.go

+		if !ok || old != aws.StringValue(t.Value) {
+			// Delete it!
+			remove = append(remove, t)
+		}


This function should skip existing tags (see also diffTags()), e.g.

Suggested change

}

} else if ok {

// already present so remove from new

delete(create, aws.StringValue(t.Key))

}

This can be verified by updating the unit test cases (see also TestDiffTags), e.g.

cases := []struct { Old, New map[string]interface{} Create, Remove map[string]string }{ // Add { Old: map[string]interface{}{ "foo": "bar", }, New: map[string]interface{}{ "foo": "bar", "bar": "baz", }, Create: map[string]string{ "bar": "baz", }, Remove: map[string]string{}, }, // Modify { Old: map[string]interface{}{ "foo": "bar", }, New: map[string]interface{}{ "foo": "baz", }, Create: map[string]string{ "foo": "baz", }, Remove: map[string]string{ "foo": "bar", }, }, // Overlap { Old: map[string]interface{}{ "foo": "bar", "hello": "world", }, New: map[string]interface{}{ "foo": "baz", "hello": "world", }, Create: map[string]string{ "foo": "baz", }, Remove: map[string]string{ "foo": "bar", }, }, // Remove { Old: map[string]interface{}{ "foo": "bar", "bar": "baz", }, New: map[string]interface{}{ "foo": "bar", }, Create: map[string]string{}, Remove: map[string]string{ "bar": "baz", }, }, }

bflad · 2018-11-19T03:14:08Z

website/docs/r/iam_user.html.markdown

@@ -16,6 +16,9 @@ Provides an IAM user.
 resource "aws_iam_user" "lb" {
  name = "loadbalancer"
  path = "/system/"
+  tags {


Since tags is a schema.TypeMap, there should be an equals sign here. This will be a requirement in Terraform 0.12 and the change is backwards compatible.

Suggested change

tags {

tags = {

bflad · 2018-11-19T03:15:31Z

website/docs/r/iam_user.html.markdown

@@ -53,6 +56,7 @@ The following arguments are supported:
 * `force_destroy` - (Optional, default false) When destroying this user, destroy even if it
  has non-Terraform-managed IAM access keys, login profile or MFA devices. Without `force_destroy`
  a user with non-Terraform-managed access keys and login profile will fail to be destroyed.
+* `tags` - Tags for the IAM user


Nit: This should probably specify key-value pairs or map as a hint to the format of the argument, e.g.

Suggested change

* `tags` - Tags for the IAM user

* `tags` - Key-value mapping of tags for the IAM user

kl4w · 2018-11-19T15:00:09Z

@bflad made changes as per requested

unit testing output:

go test ./aws -v -run="TestDiffIAMTags"
=== RUN   TestDiffIAMTags
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with bar
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with bar
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with bar
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with hello
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with hello
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with bar
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with foo
2018/11/19 09:59:52 [DEBUG] Matching ^aws: with bar
--- PASS: TestDiffIAMTags (0.00s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	0.036s

acceptance test output:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSUser_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSUser_ -timeout 120m
=== RUN   TestAccAWSUser_importBasic
=== PAUSE TestAccAWSUser_importBasic
=== RUN   TestAccAWSUser_basic
=== PAUSE TestAccAWSUser_basic
=== RUN   TestAccAWSUser_disappears
=== PAUSE TestAccAWSUser_disappears
=== RUN   TestAccAWSUser_ForceDestroy_AccessKey
=== PAUSE TestAccAWSUser_ForceDestroy_AccessKey
=== RUN   TestAccAWSUser_ForceDestroy_LoginProfile
=== PAUSE TestAccAWSUser_ForceDestroy_LoginProfile
=== RUN   TestAccAWSUser_ForceDestroy_MFADevice
=== PAUSE TestAccAWSUser_ForceDestroy_MFADevice
=== RUN   TestAccAWSUser_ForceDestroy_SSHKey
=== PAUSE TestAccAWSUser_ForceDestroy_SSHKey
=== RUN   TestAccAWSUser_nameChange
=== PAUSE TestAccAWSUser_nameChange
=== RUN   TestAccAWSUser_pathChange
=== PAUSE TestAccAWSUser_pathChange
=== RUN   TestAccAWSUser_permissionsBoundary
=== PAUSE TestAccAWSUser_permissionsBoundary
=== RUN   TestAccAWSUser_tags
=== PAUSE TestAccAWSUser_tags
=== CONT  TestAccAWSUser_importBasic
=== CONT  TestAccAWSUser_ForceDestroy_SSHKey
=== CONT  TestAccAWSUser_ForceDestroy_AccessKey
=== CONT  TestAccAWSUser_ForceDestroy_MFADevice
=== CONT  TestAccAWSUser_ForceDestroy_LoginProfile
=== CONT  TestAccAWSUser_disappears
=== CONT  TestAccAWSUser_permissionsBoundary
=== CONT  TestAccAWSUser_tags
=== CONT  TestAccAWSUser_basic
=== CONT  TestAccAWSUser_nameChange
=== CONT  TestAccAWSUser_pathChange
--- PASS: TestAccAWSUser_disappears (7.87s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (10.21s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (10.33s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (10.37s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (10.40s)
--- PASS: TestAccAWSUser_importBasic (10.59s)
--- PASS: TestAccAWSUser_nameChange (14.64s)
--- PASS: TestAccAWSUser_basic (14.70s)
--- PASS: TestAccAWSUser_tags (14.73s)
--- PASS: TestAccAWSUser_pathChange (14.84s)
--- PASS: TestAccAWSUser_permissionsBoundary (31.14s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	31.175s

bflad

Thanks so much, @kl4w 🚀

--- PASS: TestAccAWSUser_disappears (5.33s)
--- PASS: TestAccAWSUser_importBasic (7.47s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (8.95s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (9.14s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (9.39s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (9.58s)
--- PASS: TestAccAWSUser_basic (11.13s)
--- PASS: TestAccAWSUser_nameChange (11.20s)
--- PASS: TestAccAWSUser_pathChange (11.62s)
--- PASS: TestAccAWSUser_tags (11.71s)
--- PASS: TestAccAWSUser_permissionsBoundary (24.27s)

bflad · 2018-11-20T02:52:57Z

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

ghost · 2020-04-02T17:21:44Z

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!

kl4w added 2 commits November 16, 2018 13:02

support tags for iam user

24a16cb

update docs

dd9ae3b

fix linter

ac26c14

kl4w mentioned this pull request Nov 16, 2018

r/iam_role: support for tags #6499

Merged

bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Nov 19, 2018

bflad reviewed Nov 19, 2018

View reviewed changes

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

kl4w added 2 commits November 19, 2018 09:54

skip existing tags

6325011

api call when count > 0

74562b3

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

bflad approved these changes Nov 19, 2018

View reviewed changes

bflad added this to the v1.46.0 milestone Nov 19, 2018

bflad merged commit ae8491a into hashicorp:master Nov 19, 2018

bflad added a commit that referenced this pull request Nov 19, 2018

Update CHANGELOG for #6497

abab878

kl4w deleted the iam-user-tags branch November 19, 2018 18:28

mdlavin mentioned this pull request Nov 21, 2018

V1.46.0 patched lifeomic mdlavin/terraform-provider-aws#3

Closed

nakamasato mentioned this pull request Jan 4, 2020

cannot import tags for aws_iam_user nakamasato/practice_terraforming#11

Closed

ghost locked and limited conversation to collaborators Apr 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/iam_user: support tags #6497

r/iam_user: support tags #6497

kl4w commented Nov 16, 2018

kl4w commented Nov 16, 2018

bflad left a comment

bflad Nov 19, 2018

bflad Nov 19, 2018

bflad Nov 19, 2018

bflad Nov 19, 2018

bflad Nov 19, 2018

bflad Nov 19, 2018

kl4w commented Nov 19, 2018

bflad left a comment

bflad commented Nov 20, 2018

ghost commented Apr 2, 2020

-	d.Set("tags", tagsToMapIAM(output.User.Tags))
+	if err := d.Set("tags", tagsToMapIAM(output.User.Tags)); err != nil {
+		return fmt.Errorf("error setting tags: %s", err)
+	}

-		}
+		} else if ok {
+			// already present so remove from new
+			delete(create, aws.StringValue(t.Key))
+		}

	* `tags` - Tags for the IAM user
	* `tags` - Key-value mapping of tags for the IAM user

r/iam_user: support tags #6497

r/iam_user: support tags #6497

Conversation

kl4w commented Nov 16, 2018

kl4w commented Nov 16, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad Nov 19, 2018

Choose a reason for hiding this comment

bflad Nov 19, 2018

Choose a reason for hiding this comment

bflad Nov 19, 2018

Choose a reason for hiding this comment

bflad Nov 19, 2018

Choose a reason for hiding this comment

bflad Nov 19, 2018

Choose a reason for hiding this comment

bflad Nov 19, 2018

Choose a reason for hiding this comment

kl4w commented Nov 19, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad commented Nov 20, 2018

ghost commented Apr 2, 2020