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 validation error with unknown values #753

Merged

Conversation

bendrucker
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

This PR resolves a bug with vault_jwt_auth_backend, where the validation implemented via CustomDiff that asserts "exactly one of oidc_discovery_url, jwks_url or jwt_validation_pubkeys should be provided" fails when one of these values is set in configuration but not known until apply time.

It handles this by first checking NewValueKnown(key) and returning early if a value is unknown.

This will happen any time these values are sourced from another resource that has not yet been created. We encountered this when pairing an okta_auth_server with Vault, but the effect would be the same for any other provider.

Closes #718

Release note for CHANGELOG:

`vault_jwt_auth_backend`: Fix plan error when `oidc_discovery_url`, `jwks_url`, or `jwt_validation_pubkeys` is set to a value that is not known until apply time

Output from acceptance testing:

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

@ghost ghost added the size/S label May 14, 2020
@bendrucker
Copy link
Contributor Author

Very open to less hacky ways to test this if that's a concern! The "correct" way I can think of to do this:

  • Create a second provider in the tests, call it "test" or something. Copy/append to testAccProviders where needed.
  • Create a "test_resource" resource or similar
  • In its schema, add two fields (both map(string)), input and outputs. Inputs is required and user-specifiable, outputs are computed and just a copy of the inputs.

If you're interested in that I'm happy to put it in a package somewhere, for now I've just gone with a resource where the name becomes the id after Create() as a simple but somewhat obscure hack.

@tyrannosaurus-becks
Copy link
Contributor

Hi! Thanks for this PR!

Question - I don't completely understand the use case here. Would simply using a depends_on parameter solve the issue where the desired value hasn't been created yet?

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this May 21, 2020
@bendrucker
Copy link
Contributor Author

Hi and thanks for responding! I'm 95% sure depends_on wouldn't suffice even as a workaround. As far as I understand, depends_on is only implemented during applies. From the docs:

Explicitly specifying a dependency is only necessary when a resource relies on some other resource's behavior but doesn't access any of that resource's data in its arguments.

The vault_jwt_auth_backend is not depending on the behavior of another resource, it is referring to its attributes directly. Terraform would wait to create the vault_jwt_auth_backend until the other resource has been created such that all the attributes of the auth backend are fully known. By default, the provider would handle this dependency on an unknown value without issue:

+ oidc_discovery_url = (known after apply)

The bug comes at plan time when the CustomizeDiff function tries to implement "at least one attribute is required" logic. For comparison, if you had a Required: true field in the schema and that were set to an unknown value, Terraform would accept that and render as (known after apply) as shown above.

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks 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 the explanation!

@tyrannosaurus-becks tyrannosaurus-becks merged commit e156f96 into hashicorp:master May 21, 2020
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
…#753)

* jwt_auth_backend: fix validation error with unknown values

* remove coalesce from test, prevented bug from being triggered
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.

oidc_discovery_url fails when dependencies have not been resolved beforehand
2 participants