terraform-provider-vault icon indicating copy to clipboard operation
terraform-provider-vault copied to clipboard

aws_secret_backend_role: fix attribute removal

Open adongy opened this issue 5 years ago • 4 comments

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

There's a change in util (that only affects the aws_secret_backend_role resource). The pathological case treats newlines/spaces as non meaningful, for cases where a template could expand to whitespace only:

resource "" "" {
policy_document = <<EOF
EOF
}

Release note for CHANGELOG:

aws_secret_backend_role: fix attribute removal (role_arns, policy_arns, policy_document, sts_ttl settings)

Output from acceptance testing:

$ make test
==> Checking that code complies with gofmt requirements...
go test -i $(go list ./... |grep -v 'vendor') || exit 1
echo $(go list ./... |grep -v 'vendor') | \
	xargs -t -n4 go test  -timeout=30s -parallel=4
go test -timeout=30s -parallel=4 github.com/terraform-providers/terraform-provider-vault github.com/terraform-providers/terraform-provider-vault/cmd/coverage github.com/terraform-providers/terraform-provider-vault/util github.com/terraform-providers/terraform-provider-vault/vault
?   	github.com/terraform-providers/terraform-provider-vault	[no test files]
?   	github.com/terraform-providers/terraform-provider-vault/cmd/coverage	[no test files]
ok  	github.com/terraform-providers/terraform-provider-vault/util	0.017s
ok  	github.com/terraform-providers/terraform-provider-vault/vault	0.477s

$ make testacc TESTARGS='-run=TestAccAWSSecretBackend'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackend -timeout 120m
?   	github.com/terraform-providers/terraform-provider-vault	[no test files]
?   	github.com/terraform-providers/terraform-provider-vault/cmd/coverage	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-vault/util	(cached) [no tests to run]
=== RUN   TestAccAWSSecretBackendRole_basic
--- PASS: TestAccAWSSecretBackendRole_basic (0.24s)
=== RUN   TestAccAWSSecretBackendRole_import
--- PASS: TestAccAWSSecretBackendRole_import (0.28s)
=== RUN   TestAccAWSSecretBackendRole_nested
--- PASS: TestAccAWSSecretBackendRole_nested (0.28s)
=== RUN   TestAccAWSSecretBackend_basic
--- PASS: TestAccAWSSecretBackend_basic (0.30s)
=== RUN   TestAccAWSSecretBackend_import
--- PASS: TestAccAWSSecretBackend_import (0.11s)
PASS
ok  	github.com/terraform-providers/terraform-provider-vault/vault	1.218s

Thanks!

adongy avatar May 05 '20 15:05 adongy

Another way to implement this, although a bit more convoluted, would be to use the current state to determine which values to send, although it is subject to the same issues: if the default value changes, we won't catch it.

adongy avatar May 12 '20 08:05 adongy

@adongy thank you for this PR! We are facing the same problem you described in #742: We are migrating from using policy_document to policy_arn for existing resources and are now in a weird state where every apply claims to remove the policy_document but in the end fails to. Your work looks like to fix this issue. I'm just wondering what the state of this PR is, as there haven't been any updates for over a year. Were you able to work around it in a different way? Are there other things that are blocking this fix and where help could be beneficial, so the PR can keep moving? 🙂

AndreasSko avatar Dec 31 '21 08:12 AndreasSko

Hello @AndreasSko,

I have changed companies and thus do not require the PR anymore. If other people are interested in taking/rebasing/finishing it, please feel free to do so.

adongy avatar Dec 31 '21 08:12 adongy

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Mar 12 '22 17:03 hashicorp-cla