Issue #3407 aws_ssm_association handles updates incorrectly

[danlaramay]

Fixes #3407

Changes proposed in this pull request:

• Fixed Typo: Corrected Assoscation to Assocation
• resourceAwsSsmAssocationUpdate needs to pass everything to AWS because the AWS API creates a new version of the association rather than updating it in-place. I updated the function to check for any changes and update everything if there are.

Output from acceptance testing:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSSSMAssociation'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSSSMAssociation -timeout 120m
=== RUN   TestAccAWSSSMAssociation_basic
--- PASS: TestAccAWSSSMAssociation_basic (146.89s)
=== RUN   TestAccAWSSSMAssociation_withTargets
--- PASS: TestAccAWSSSMAssociation_withTargets (38.64s)
=== RUN   TestAccAWSSSMAssociation_withParameters
--- PASS: TestAccAWSSSMAssociation_withParameters (26.51s)
=== RUN   TestAccAWSSSMAssociation_withAssociationName
--- PASS: TestAccAWSSSMAssociation_withAssociationName (25.49s)
=== RUN   TestAccAWSSSMAssociation_withDocumentVersion
--- PASS: TestAccAWSSSMAssociation_withDocumentVersion (16.49s)
=== RUN   TestAccAWSSSMAssociation_withOutputLocation
--- PASS: TestAccAWSSSMAssociation_withOutputLocation (68.60s)
=== RUN   TestAccAWSSSMAssociation_withScheduleExpression
--- PASS: TestAccAWSSSMAssociation_withScheduleExpression (25.53s)
PASS
ok  	github.com/dpldb/terraform-provider-aws/aws	348.188s
...

[ajjl]

I am encountering the same issue. Thanks for addressing this.

[bflad]

Thanks for fixing this, @dpldb! Left one minor code cleanup note but more importantly, are you able to write an acceptance test that fails against master and succeeds with this pull request? We would like to verify the fix as necessary and prevent regressions in the future. Please reach out if you need help with that or do not have time to implement. Thanks!

[bflad]

The resourceAwsSsmAssociationUpdate function is not performing any other API calls beyond UpdateAssociation so all the hasChanges logic can be removed 👍

[danlaramay]

Good point. I'll get rid of this.

[danlaramay]

@bflad Thanks for the code review and quick response. Happy to help. I'll take a look at the acceptance test.

[bflad]

LGTM, thanks @dpldb! Since its been two weeks and this is a bug fix, I have added an acceptance test that covers this behavior and refactored out the hasChanges variable in the update function so we can ship this. 🚀

--- PASS: TestAccAWSSSMAssociation_withDocumentVersion (23.27s)
--- PASS: TestAccAWSSSMAssociation_withAssociationNameAndScheduleExpression (26.62s)
--- PASS: TestAccAWSSSMAssociation_withScheduleExpression (28.05s)
--- PASS: TestAccAWSSSMAssociation_withParameters (29.40s)
--- PASS: TestAccAWSSSMAssociation_withAssociationName (32.30s)
--- PASS: TestAccAWSSSMAssociation_withTargets (47.62s)
--- PASS: TestAccAWSSSMAssociation_withOutputLocation (68.47s)
--- PASS: TestAccAWSSSMAssociation_basic (132.95s)

[danlaramay]

Thanks @bflad

[bflad]

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

[ghost]

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!