AWS Systems Manager resources: Add support for resource tags #8426

kmoe · 2019-04-24T16:21:05Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Changes proposed in this pull request:

Support adding and updating tags wherever possible for SSM Activations, Parameters, Maintenance Windows, Patch Baselines, and Documents

Output from acceptance testing:

$  make testacc TEST=./aws TESTARGS='"-run=TestAccAWSSSM(Parameter|Document|MaintenanceWindow|Activation|PatchBaseline)"'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 "-run=TestAccAWSSSM(Parameter|Document|MaintenanceWindow|Activation|PatchBaseline)" -timeout 120m
=== RUN   TestAccAWSSSMActivation_basic
=== PAUSE TestAccAWSSSMActivation_basic
=== RUN   TestAccAWSSSMActivation_expirationDate
=== PAUSE TestAccAWSSSMActivation_expirationDate
=== RUN   TestAccAWSSSMDocument_basic
=== PAUSE TestAccAWSSSMDocument_basic
=== RUN   TestAccAWSSSMDocument_update
=== PAUSE TestAccAWSSSMDocument_update
=== RUN   TestAccAWSSSMDocument_permission_public
=== PAUSE TestAccAWSSSMDocument_permission_public
=== RUN   TestAccAWSSSMDocument_permission_private
=== PAUSE TestAccAWSSSMDocument_permission_private
=== RUN   TestAccAWSSSMDocument_permission_batching
=== PAUSE TestAccAWSSSMDocument_permission_batching
=== RUN   TestAccAWSSSMDocument_permission_change
=== PAUSE TestAccAWSSSMDocument_permission_change
=== RUN   TestAccAWSSSMDocument_params
=== PAUSE TestAccAWSSSMDocument_params
=== RUN   TestAccAWSSSMDocument_automation
=== PAUSE TestAccAWSSSMDocument_automation
=== RUN   TestAccAWSSSMDocument_session
=== PAUSE TestAccAWSSSMDocument_session
=== RUN   TestAccAWSSSMDocument_DocumentFormat_YAML
=== PAUSE TestAccAWSSSMDocument_DocumentFormat_YAML
=== RUN   TestAccAWSSSMDocument_Tags
=== PAUSE TestAccAWSSSMDocument_Tags
=== RUN   TestAccAWSSSMMaintenanceWindowTarget_basic
=== PAUSE TestAccAWSSSMMaintenanceWindowTarget_basic
=== RUN   TestAccAWSSSMMaintenanceWindowTarget_update
=== PAUSE TestAccAWSSSMMaintenanceWindowTarget_update
=== RUN   TestAccAWSSSMMaintenanceWindowTask_basic
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_basic
=== RUN   TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== PAUSE TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== RUN   TestAccAWSSSMMaintenanceWindow_basic
=== PAUSE TestAccAWSSSMMaintenanceWindow_basic
=== RUN   TestAccAWSSSMMaintenanceWindow_tags
=== PAUSE TestAccAWSSSMMaintenanceWindow_tags
=== RUN   TestAccAWSSSMMaintenanceWindow_disappears
=== PAUSE TestAccAWSSSMMaintenanceWindow_disappears
=== RUN   TestAccAWSSSMMaintenanceWindow_multipleUpdates
=== PAUSE TestAccAWSSSMMaintenanceWindow_multipleUpdates
=== RUN   TestAccAWSSSMMaintenanceWindow_Cutoff
=== PAUSE TestAccAWSSSMMaintenanceWindow_Cutoff
=== RUN   TestAccAWSSSMMaintenanceWindow_Duration
=== PAUSE TestAccAWSSSMMaintenanceWindow_Duration
=== RUN   TestAccAWSSSMMaintenanceWindow_Enabled
=== PAUSE TestAccAWSSSMMaintenanceWindow_Enabled
=== RUN   TestAccAWSSSMMaintenanceWindow_EndDate
=== PAUSE TestAccAWSSSMMaintenanceWindow_EndDate
=== RUN   TestAccAWSSSMMaintenanceWindow_Schedule
=== PAUSE TestAccAWSSSMMaintenanceWindow_Schedule
=== RUN   TestAccAWSSSMMaintenanceWindow_ScheduleTimezone
=== PAUSE TestAccAWSSSMMaintenanceWindow_ScheduleTimezone
=== RUN   TestAccAWSSSMMaintenanceWindow_StartDate
=== PAUSE TestAccAWSSSMMaintenanceWindow_StartDate
=== RUN   TestAccAWSSSMParameter_importBasic
=== PAUSE TestAccAWSSSMParameter_importBasic
=== RUN   TestAccAWSSSMParameter_basic
=== PAUSE TestAccAWSSSMParameter_basic
=== RUN   TestAccAWSSSMParameter_disappears
=== PAUSE TestAccAWSSSMParameter_disappears
=== RUN   TestAccAWSSSMParameter_overwrite
=== PAUSE TestAccAWSSSMParameter_overwrite
=== RUN   TestAccAWSSSMParameter_updateTags
=== PAUSE TestAccAWSSSMParameter_updateTags
=== RUN   TestAccAWSSSMParameter_updateDescription
=== PAUSE TestAccAWSSSMParameter_updateDescription
=== RUN   TestAccAWSSSMParameter_changeNameForcesNew
=== PAUSE TestAccAWSSSMParameter_changeNameForcesNew
=== RUN   TestAccAWSSSMParameter_fullPath
=== PAUSE TestAccAWSSSMParameter_fullPath
=== RUN   TestAccAWSSSMParameter_secure
=== PAUSE TestAccAWSSSMParameter_secure
=== RUN   TestAccAWSSSMParameter_secure_with_key
=== PAUSE TestAccAWSSSMParameter_secure_with_key
=== RUN   TestAccAWSSSMParameter_secure_keyUpdate
=== PAUSE TestAccAWSSSMParameter_secure_keyUpdate
=== RUN   TestAccAWSSSMPatchBaseline_basic
=== PAUSE TestAccAWSSSMPatchBaseline_basic
=== RUN   TestAccAWSSSMPatchBaseline_disappears
=== PAUSE TestAccAWSSSMPatchBaseline_disappears
=== RUN   TestAccAWSSSMPatchBaseline_OperatingSystem
=== PAUSE TestAccAWSSSMPatchBaseline_OperatingSystem
=== CONT  TestAccAWSSSMActivation_basic
=== CONT  TestAccAWSSSMMaintenanceWindow_Duration
=== CONT  TestAccAWSSSMParameter_updateDescription
=== CONT  TestAccAWSSSMParameter_secure_keyUpdate
=== CONT  TestAccAWSSSMPatchBaseline_OperatingSystem
=== CONT  TestAccAWSSSMPatchBaseline_disappears
=== CONT  TestAccAWSSSMPatchBaseline_basic
=== CONT  TestAccAWSSSMDocument_DocumentFormat_YAML
=== CONT  TestAccAWSSSMMaintenanceWindow_Cutoff
=== CONT  TestAccAWSSSMMaintenanceWindow_multipleUpdates
=== CONT  TestAccAWSSSMMaintenanceWindow_disappears
=== CONT  TestAccAWSSSMMaintenanceWindow_tags
=== CONT  TestAccAWSSSMMaintenanceWindow_basic
=== CONT  TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
=== CONT  TestAccAWSSSMParameter_secure
=== CONT  TestAccAWSSSMParameter_secure_with_key
=== CONT  TestAccAWSSSMDocument_permission_batching
=== CONT  TestAccAWSSSMDocument_session
=== CONT  TestAccAWSSSMDocument_automation
=== CONT  TestAccAWSSSMDocument_params
--- FAIL: TestAccAWSSSMDocument_automation (3.01s)
    testing.go:568: Step 0 error: config is invalid: 2 problems:
        
        - "roles": [DEPRECATED] Use `role` instead. Only a single role can be passed to an IAM Instance Profile
        - Missing required argument: The argument "owners" is required, but no definition was found.
=== CONT  TestAccAWSSSMDocument_permission_change
--- PASS: TestAccAWSSSMPatchBaseline_disappears (18.86s)
=== CONT  TestAccAWSSSMParameter_changeNameForcesNew
--- PASS: TestAccAWSSSMMaintenanceWindow_disappears (19.04s)
=== CONT  TestAccAWSSSMDocument_update
--- PASS: TestAccAWSSSMMaintenanceWindow_basic (31.23s)
=== CONT  TestAccAWSSSMDocument_permission_private
--- PASS: TestAccAWSSSMDocument_permission_batching (31.35s)
=== CONT  TestAccAWSSSMDocument_permission_public
--- PASS: TestAccAWSSSMActivation_basic (35.06s)
=== CONT  TestAccAWSSSMDocument_basic
--- PASS: TestAccAWSSSMDocument_session (36.16s)
=== CONT  TestAccAWSSSMParameter_fullPath
--- PASS: TestAccAWSSSMMaintenanceWindow_tags (46.18s)
=== CONT  TestAccAWSSSMMaintenanceWindowTarget_basic
--- PASS: TestAccAWSSSMMaintenanceWindow_Cutoff (46.60s)
=== CONT  TestAccAWSSSMMaintenanceWindowTarget_update
--- PASS: TestAccAWSSSMMaintenanceWindow_Duration (47.44s)
=== CONT  TestAccAWSSSMDocument_Tags
--- PASS: TestAccAWSSSMParameter_secure (47.98s)
=== CONT  TestAccAWSSSMParameter_importBasic
--- PASS: TestAccAWSSSMDocument_DocumentFormat_YAML (56.83s)
=== CONT  TestAccAWSSSMParameter_updateTags
--- PASS: TestAccAWSSSMDocument_permission_public (25.92s)
=== CONT  TestAccAWSSSMActivation_expirationDate
--- PASS: TestAccAWSSSMDocument_permission_private (26.08s)
=== CONT  TestAccAWSSSMParameter_overwrite
--- PASS: TestAccAWSSSMPatchBaseline_OperatingSystem (57.85s)
=== CONT  TestAccAWSSSMParameter_basic
--- PASS: TestAccAWSSSMMaintenanceWindow_multipleUpdates (57.87s)
=== CONT  TestAccAWSSSMParameter_disappears
--- PASS: TestAccAWSSSMPatchBaseline_basic (59.56s)
=== CONT  TestAccAWSSSMMaintenanceWindow_StartDate
--- PASS: TestAccAWSSSMDocument_basic (24.96s)
=== CONT  TestAccAWSSSMMaintenanceWindow_ScheduleTimezone
--- PASS: TestAccAWSSSMParameter_updateDescription (64.34s)
=== CONT  TestAccAWSSSMMaintenanceWindow_EndDate
--- PASS: TestAccAWSSSMParameter_secure_with_key (64.44s)
=== CONT  TestAccAWSSSMMaintenanceWindow_Enabled
--- PASS: TestAccAWSSSMDocument_update (47.28s)
=== CONT  TestAccAWSSSMMaintenanceWindowTask_basic
--- PASS: TestAccAWSSSMParameter_changeNameForcesNew (49.99s)
=== CONT  TestAccAWSSSMMaintenanceWindow_Schedule
--- PASS: TestAccAWSSSMDocument_permission_change (69.52s)
--- PASS: TestAccAWSSSMDocument_params (75.32s)
--- PASS: TestAccAWSSSMParameter_disappears (18.93s)
--- PASS: TestAccAWSSSMParameter_importBasic (32.66s)
--- PASS: TestAccAWSSSMMaintenanceWindowTarget_basic (35.66s)
--- PASS: TestAccAWSSSMParameter_basic (26.37s)
--- PASS: TestAccAWSSSMParameter_secure_keyUpdate (87.25s)
--- PASS: TestAccAWSSSMMaintenanceWindowTarget_update (41.25s)
--- PASS: TestAccAWSSSMActivation_expirationDate (33.59s)
--- PASS: TestAccAWSSSMParameter_updateTags (45.37s)
--- PASS: TestAccAWSSSMParameter_overwrite (44.99s)
--- PASS: TestAccAWSSSMMaintenanceWindow_Enabled (42.50s)
--- PASS: TestAccAWSSSMDocument_Tags (60.80s)
--- PASS: TestAccAWSSSMParameter_fullPath (73.48s)
--- PASS: TestAccAWSSSMMaintenanceWindow_Schedule (43.07s)
--- PASS: TestAccAWSSSMMaintenanceWindow_ScheduleTimezone (58.01s)
--- PASS: TestAccAWSSSMMaintenanceWindow_StartDate (60.76s)
--- PASS: TestAccAWSSSMMaintenanceWindow_EndDate (58.20s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (130.77s)
--- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (117.63s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	184.000s
make: *** [GNUmakefile:20: testacc] Error 1

Note the one existing test fail, which will be fixed by #8324.

radeksimko

While ~200 LOC may not look like too much, AFAIK we generally speaking prefer many smaller PRs (typically 1 per resource where possible) over few bigger ones.

I have left you some more comments inline, I hope you find these useful. 😉

Overall this looks very good for a first PR!

aws/resource_aws_ssm_document_test.go

aws/resource_aws_ssm_parameter_test.go

aws/resource_aws_ssm_maintenance_window.go

aws/resource_aws_ssm_patch_baseline.go

aws/tagsSSM.go

aws/resource_aws_ssm_patch_baseline.go

kmoe · 2019-04-25T13:56:13Z

Thanks for the very helpful review @radeksimko. I've rebased incorporating your suggestions.

radeksimko

LGTM, but another 👍 from Brian would be nice as I haven't properly worked on this repo for some time 😅

nywilken

This looks good. Thank you for addressing this issue. I've left a few comments around acceptance testing and formatting but this is otherwise good to go. Please let me know if you need any help with the requested tests.

aws/resource_aws_ssm_activation.go

aws/resource_aws_ssm_activation_test.go

aws/resource_aws_ssm_document_test.go

aws/resource_aws_ssm_maintenance_window_test.go

aws/tagsSSM.go

Note that "You can't add tags to or delete tags from an existing activation.". See https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_CreateActivation.html

Tag update after document creation is already supported.

Adding and updating tags on an SSM Parameter is already supported.

kmoe · 2019-04-30T13:34:50Z

Thank you very much @nywilken. Review comments addressed mainly in rebased commits, with one new commit added for testing the tags ForceNew behaviour on SSM Activations.

… below acceptance test functions

nywilken

@kmoe this is good to go 🎉

I made one change before merging which was to move the testAccCheckAWSSSMActivationRecreated definition after all of the acceptance test functions

jonathanallen · 2019-05-07T19:35:03Z

Getting HTTP 400 errors when running 2.9.0 with existing SSM resources:

2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: 2019/05/07 19:29:23 [DEBUG] [aws-sdk-go] DEBUG: Response ssm/ListTagsForResource Details:
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: ---[ RESPONSE ]--------------------------------------
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: HTTP/1.1 400 Bad Request
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: Connection: close
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: Content-Type: application/x-amz-json-1.1
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: Date: Tue, 07 May 2019 19:29:22 GMT
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: X-Amzn-Requestid: XXXXXXXXXXXXX
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4:
2019/05/07 19:29:23 [ERROR] root: eval: *terraform.EvalRefresh, err: aws_ssm_maintenance_window.rhel_prod_monday: error saving tags for SSM Maintenance Window (mw-088c0d8331e36b67e): Error retrieving tags for SSM resource: mw-088c0d8331e36b67e
2019/05/07 19:29:23 [ERROR] root: eval: *terraform.EvalSequence, err: aws_ssm_maintenance_window.rhel_prod_monday: error saving tags for SSM Maintenance Window (mw-088c0d8331e36b67e): Error retrieving tags for SSM resource: mw-088c0d8331e36b67e
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4:
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: -----------------------------------------------------
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: 2019/05/07 19:29:23 [DEBUG] [aws-sdk-go] {"__type":"UnrecognizedClientException","message":"The security token included in the request is invalid."}
2019-05-07T19:29:23.139Z [DEBUG] plugin.terraform-provider-aws_v2.9.0_x4: 2019/05/07 19:29:23 [DEBUG] [aws-sdk-go] DEBUG: Validate Response ssm/ListTagsForResource failed, not retrying, error UnrecognizedClientException: The security token included in the request is invalid.
2019/05/07 19:29:23 [TRACE] root: eval: *terraform.EvalSequence
2019/05/07 19:29:23 [TRACE] root: eval: *terraform.EvalGetProvider
2019/05/07 19:29:23 [TRACE] root: eval: *terraform.EvalReadState

AWS credential has full Administrator access, backing out to 2.8.0, no issue.

kmoe · 2019-05-08T12:08:11Z

Thanks for the comment @jonathanallen. Would you mind submitting a bug report in this repo with some more details and your Terraform configuration?

In particular, it will be very useful to know whether your SSM Maintenance Window has tags defined.

jonathanallen · 2019-05-13T14:57:04Z

I was able to reproduce this outside of Terraform, I opened a support case with AWS and about 8 hours later the issue disappeared, still waiting for confirmation from AWS that they actually fixed something but this appears not to be an issue with this Terraform release. Thanks for your time.

ghost · 2020-03-30T17:20:09Z

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!

add tagsSchemaForceNew schema type

5bfe1da

kmoe requested review from bflad and a team April 24, 2019 16:35

radeksimko reviewed Apr 24, 2019

View reviewed changes

kmoe force-pushed the ssm-tags-on-create branch from a978cca to 781456a Compare April 25, 2019 13:41

kmoe requested a review from radeksimko April 25, 2019 13:55

radeksimko approved these changes Apr 25, 2019

View reviewed changes

nywilken reviewed Apr 29, 2019

View reviewed changes

kmoe added 7 commits April 30, 2019 13:58

add saveTagsSSM method

036db0d

support adding tags when creating SSM Activation resources

1c2dd8d

Note that "You can't add tags to or delete tags from an existing activation.". See https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_CreateActivation.html

support adding tags when creating SSM Document resources

6423e57

Tag update after document creation is already supported.

support adding and updating tags on SSM Maintenance Window resources

c31a479

support adding and updating tags on SSM Patch Baseline resources

c9f36aa

add tests for adding and updating tags on SSM Parameter resources

cd97149

Adding and updating tags on an SSM Parameter is already supported.

add test for ForceNew on SSM Activation tags update

116426b

kmoe force-pushed the ssm-tags-on-create branch from 781456a to 116426b Compare April 30, 2019 13:32

kmoe requested a review from nywilken April 30, 2019 13:33

nywilken added this to the v2.9.0 milestone May 1, 2019

aws/resource_aws_ssm_activation_test: Move helper function definition…

62eafc2

… below acceptance test functions

nywilken approved these changes May 1, 2019

View reviewed changes

nywilken merged commit de34fb1 into hashicorp:master May 1, 2019

nywilken added a commit that referenced this pull request May 1, 2019

update CHANGELOG for #8426

f20ae99

This was referenced May 13, 2019

Update aws_ssm_maintenance_window resource to support tags #8326

Closed

Update aws_ssm_maintenance_window resource to support tags #8006

Closed

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWS Systems Manager resources: Add support for resource tags #8426

AWS Systems Manager resources: Add support for resource tags #8426

kmoe commented Apr 24, 2019

radeksimko left a comment

kmoe commented Apr 25, 2019

radeksimko left a comment

nywilken left a comment

kmoe commented Apr 30, 2019

nywilken left a comment

jonathanallen commented May 7, 2019

kmoe commented May 8, 2019

jonathanallen commented May 13, 2019

ghost commented Mar 30, 2020

AWS Systems Manager resources: Add support for resource tags #8426

AWS Systems Manager resources: Add support for resource tags #8426

Conversation

kmoe commented Apr 24, 2019

Community Note

radeksimko left a comment

Choose a reason for hiding this comment

kmoe commented Apr 25, 2019

radeksimko left a comment

Choose a reason for hiding this comment

nywilken left a comment

Choose a reason for hiding this comment

kmoe commented Apr 30, 2019

nywilken left a comment

Choose a reason for hiding this comment

jonathanallen commented May 7, 2019

kmoe commented May 8, 2019

jonathanallen commented May 13, 2019

ghost commented Mar 30, 2020