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.

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

Fix permanent diff on aws_opsworks ssh_key and password #10175

Conversation

xsalazar
Copy link
Contributor

@xsalazar xsalazar commented Sep 19, 2019

Summary

Previously, there was an issue when using custom_cookbooks_source in the aws_opsworks_stack resource, Terraform would continuously reapply changes to both ssh_key and password despite no changes actually happening.

This is because, on read, the resourceAwsOpsworksSetStackCustomCookbooksSource method would create an intermediate map before attempting to update the resource data. However, map entries for both Password and SshKey were removed, since the AWS API will return *****FILTERED***** for sensitive data.

Since these entries were removed, they would be read as empty strings, and when Terraform compared these read values to the values in code, there would always be a difference between the code and empty string. Thus, every terraform apply would claim a difference and rewrite those values.

To solve this issue, the old value from the resource data is copied into the intermediate map and Terraform will no longer think it detects changes to these properties where there are none.

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

Related Issues

#165
#176
#421
#4411
hashicorp/terraform#6203
hashicorp/terraform#6648
hashicorp/terraform#3635
hashicorp/terraform#6192
hashicorp/terraform#6826
hashicorp/terraform#10675
hashicorp/terraform#4790
hashicorp/terraform#17959

Release Notes

Release note for CHANGELOG:

- Fixes issue where `terraform apply` continuously suggests applying changes to `ssh_key` or `password` in `custom_cookbooks_source` property for `aws_opsworks_stack`
- Fixes issue where `terraform apply` continuously suggests applying changes to `ssh_key` or `password` in `app_source` property for `aws_opsworks_application`

Test

Output from acceptance testing:

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

… SshKey

When these values are missing from the map, Terraform attempts to
continuous reapply the same property values each time
@xsalazar xsalazar requested a review from a team September 19, 2019 20:46
@ghost ghost added size/L Managed by automation to categorize the size of a PR. service/opsworks Issues and PRs that pertain to the opsworks service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Sep 19, 2019
@xsalazar
Copy link
Contributor Author

Paging @apparentlymart since he started this work in #176. Would you be able to review this?

@wagner
Copy link

wagner commented Oct 6, 2019

@xsalazar is this the same issue affecting aws_opsworks_application.app_source.ssh_key?

@xsalazar
Copy link
Contributor Author

xsalazar commented Oct 7, 2019

@wagner Yeah, it looks like there's an issue in there as well. It's the same mapping pattern, but the existing code for getting the password is wrong, and the missing ssh_key suffers from the same issue that this PR solves.

You can also double check the AWS API and see that the password will never be returned!

@xsalazar xsalazar changed the title Fix permanent diff on aws_opsworks_stack ssh_key and password Fix permanent diff on aws_opsworks ssh_key and password Oct 7, 2019
@xsalazar
Copy link
Contributor Author

Would @apparentlymart or @bflad be able to check out this PR? It would close quite a few tickets and is pretty straightforward.

@aeschright aeschright self-assigned this Nov 18, 2019
@aeschright aeschright added this to the v2.39.0 milestone Nov 18, 2019
@aeschright
Copy link
Contributor

Looks good! We just need a documentation update, similar to https://www.terraform.io/docs/providers/aws/r/emr_cluster.html#kerberos_attributes-1

@ghost ghost added the documentation Introduces or discusses updates to documentation. label Nov 18, 2019
@xsalazar
Copy link
Contributor Author

@aeschright done!

Copy link
Contributor

@aeschright aeschright left a comment

Choose a reason for hiding this comment

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

Thanks for this patch! 🌟

--- PASS: TestAccAWSOpsworksStack_CustomCookbooks_SetPrivateProperties (35.92s)

@aeschright aeschright merged commit f853a71 into hashicorp:master Nov 18, 2019
aeschright added a commit that referenced this pull request Nov 18, 2019
@xsalazar xsalazar deleted the resource_aws_opsworks_stack/custom_cookbooks branch November 20, 2019 19:28
@xsalazar xsalazar restored the resource_aws_opsworks_stack/custom_cookbooks branch November 20, 2019 19:33
@xsalazar xsalazar deleted the resource_aws_opsworks_stack/custom_cookbooks branch November 20, 2019 19:34
@ghost
Copy link

ghost commented Nov 21, 2019

This has been released in version 2.39.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
Copy link

ghost commented Mar 29, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/opsworks Issues and PRs that pertain to the opsworks service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants