-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
remove tags field from aws_secretsmanager_secret_rotation resource #27656
Conversation
Community NoteVoting for Prioritization
For Submitters
|
e5394d7
to
f11f9e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome @frek818 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
$ make testacc TESTARGS='-run=TestAccSecretsManagerSecretRotation' PKG=secretsmanager
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/secretsmanager/... -v -count 1 -parallel 20 -run=TestAccSecretsManagerSecretRotation -timeout 180m
=== RUN TestAccSecretsManagerSecretRotationDataSource_basic
=== PAUSE TestAccSecretsManagerSecretRotationDataSource_basic
=== RUN TestAccSecretsManagerSecretRotation_basic
=== PAUSE TestAccSecretsManagerSecretRotation_basic
=== CONT TestAccSecretsManagerSecretRotationDataSource_basic
=== CONT TestAccSecretsManagerSecretRotation_basic
--- PASS: TestAccSecretsManagerSecretRotation_basic (49.05s)
--- PASS: TestAccSecretsManagerSecretRotationDataSource_basic (52.32s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/secretsmanager 55.051s
Nice find and fix @frek818 ! |
This functionality has been released in v4.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. Thank you! |
@jar-b Why a breaking change for a point release? |
In this case we decided to move forward with removing the attribute (versus deprecating and removing in v5) because the practitioner impact should be minimal while solving the issue documented in #27655. The underlying resource doesn't support tags, and the #27597 is a similar situation where while technically breaking, the attributes modified are always computed and therefore unlikely to break a real world configuration. That changelog entry is categorized as a bug, as this could have been, but I opted for breaking to be transparent in the case this does have an unexpected impact and we need to roll back. Hope this helps to explain the line of thinking here, and please open an issue and link to this PR if you have any concerns. |
@jar-b how do I upgrade in the case where my state already has the |
Maybe only an issue when doing resource targeting, as it needs to read previous state in this case instead of recomputing? I was supplying some targets, and doing a non targeted plan was a way to work around this. |
@shidel-dev - Are you still observing the error above? If so, do you mind opening a separate issue with your terraform configuration and the plugin versions (previous and version you upgraded to)? We want to track any upgrade issues this may be causing. |
I was observing it until I rebuilt my remote state with |
We are applying with targets. I am going to try the rebuild state as mentioned above |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
This change removes the useless
tags
argument from theaws_secretsmanager_secret_rotation
resource.Relations
Closes #27655
Output from Acceptance Testing