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

jwt_auth_backend: fix oidc_client_secret storage #803

Merged
merged 2 commits into from
Nov 6, 2020

Conversation

coreypobrien
Copy link
Contributor

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 #449

The oidc_client_secret is sensitive so it will not be in the response from Vault.

Our options are to a) always assume it must be updated (current) or b) always assume it matches what we last set it to (this change). This assumes it always matches what we last set it to so that HasChange isn't always true.

This does intentionally miss the edge case where the oidc_client_secret is updated without using Terraform. Since we cannot know the current state of oidc_client_secret, Terraform will show no changes are required even if the actual value in Vault does not match the current value in state.

Release note for CHANGELOG:

Store `oidc_client_secret` value correctly for `jwt_auth_backend` to fix bug where `oidc_client_secret` always appears to require an update. 

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccJWTAuthBackend_OIDC'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccJWTAuthBackend_OIDC -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   TestAccJWTAuthBackend_OIDC
--- PASS: TestAccJWTAuthBackend_OIDC (0.84s)
PASS
ok  	github.com/terraform-providers/terraform-provider-vault/vault	1.186s

@ghost ghost added the size/XS label Jun 25, 2020
@coreypobrien
Copy link
Contributor Author

That failure seems to be a flaky test right? Looks like it also failed on master a few builds ago: https://app.circleci.com/pipelines/github/terraform-providers/terraform-provider-vault/1067/workflows/0131be17-4682-4e77-939f-0ec24948d676/jobs/1063

@coreypobrien
Copy link
Contributor Author

@catsby Is there a step I'm missing to get this assigned for review?

@catsby catsby added this to the vNext milestone Oct 8, 2020
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

This looks great, thanks! Sorry for the delay

@catsby catsby merged commit 088dbf2 into hashicorp:master Nov 6, 2020
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vault_jwt_auth_backend: attribute oidc_client_secret always appears to be changing
2 participants