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.

Sign up for GitHub

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

Jump to bottom

Arg removal not properly applied for aws_secret_backend_role #1277

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Arg removal not properly applied for aws_secret_backend_role #1277

merged 1 commit into from
Jan 6, 2022

Conversation

AndreasSko
Copy link
Contributor

When an existing argument like policy_document is removed for aws_secret_backend_role, the change won't be properly applied to Vault, as it's only checking for existing data: If there is a removal, Terraform will simply not send the data to Vault. This results in constant attempts of Terraform to remove the data without success.

This changes the checks for what data should be sent to Vault, so arguments will always be set in case their data has changed (even if it's a change to null).

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

Closes #742

Release note for CHANGELOG:

aws_secret_backend_role: Removal of arguments will be properly applied to Vault

Output from acceptance testing:

❯ AWS_ACCESS_KEY_ID=abcd AWS_ACCESS_KEY=abc AWS_SECRET_ACCESS_KEY=abd TESTARGS="-v --run TestAccAWSSecretBackendRole*" make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./...) -v -v --run TestAccAWSSecretBackendRole* -timeout 20m
?   	github.com/hashicorp/terraform-provider-vault	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/cmd/coverage	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/cmd/generate	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/codegen	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/generated	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/helper	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/schema	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/util	(cached) [no tests to run]
=== RUN   TestAccAWSSecretBackendRole_basic
--- PASS: TestAccAWSSecretBackendRole_basic (1.87s)
=== RUN   TestAccAWSSecretBackendRole_import
--- PASS: TestAccAWSSecretBackendRole_import (1.24s)
=== RUN   TestAccAWSSecretBackendRole_nested
--- PASS: TestAccAWSSecretBackendRole_nested (1.50s)
PASS
ok  	github.com/hashicorp/terraform-provider-vault/vault	4.634s

@AndreasSko
🐛 Arg removal not properly applied for aws_secret_backend_role
f2ba12c 
When an existing argument like `policy_document` is removed for
`aws_secret_backend_role`, the change won't
be properly applied to Vault, as it's only checking
for existing data: If there is a removal, Terraform will simply
not send the data to Vault. This results in constant
attempts of Terraform to remove the data
without success.

This changes the checks for what data should be sent to Vault,
so arguments will always be set in case their data has changed
(even if it's a change to null).
@github-actions github-actions bot added the size/S label Jan 3, 2022
@AndreasSko
Copy link
Contributor Author

cc @universam1 @dhohengassner @icereed

@benashz benashz added this to the 3.2.0 milestone Jan 5, 2022
@benashz benashz self-requested a review January 5, 2022 22:39
@benashz benashz changed the title 🐛 Arg removal not properly applied for aws_secret_backend_role Arg removal not properly applied for aws_secret_backend_role Jan 5, 2022
benashz
benashz approved these changes Jan 6, 2022
View reviewed changes
Copy link
Contributor

@benashz benashz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM!

Thank you for your contribution to HashiCorp!

@benashz benashz merged commit 3e46de5 into hashicorp:main Jan 6, 2022
@benashz benashz added the bug label Jan 6, 2022
benashz added a commit that referenced this pull request Jan 6, 2022
@benashz
Update changelog for #1277
3102e63
@benashz benashz mentioned this pull request Jan 6, 2022
Merged
@benashz benashz removed the bug label Jan 6, 2022
@AndreasSko AndreasSko deleted the check_for_aws_role_changes branch January 6, 2022 16:36
benashz added a commit that referenced this pull request Jan 6, 2022
@benashz
Update changelog for #1277 (#1283)
b821427
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benashz benashz benashz approved these changes

Assignees
No one assigned
Labels
size/S
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.

aws_secret_backend_role: cannot remove attribute values
2 participants
@AndreasSko @benashz