provider config documentation: Token is Required in schema

[guineveresaenger]

Description

Fixes provider config documentation to state token as Required.

The provider schema marks token as Required but it is currently marked as Optional in the docs.

Checklist

• [ ] Added CHANGELOG entry (only for user-facing changes)
• [ ] Acceptance tests where run against all supported Vault Versions This is a miniscule docs change so I did not set up tests.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

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

[fairclothjm]

@guineveresaenger You are correct that the field is marked as required in the source. However, I am not sure it is correct to mark the field as required in the documentation. This is because the token can be set by the environment variable. So it is not required to be set in the config.

[benashz]

Hi @guineveresaenger thanks for the PR. It looks like the current documentation is correct; the token parameter is optional. The Vault provider supports numerous auth_login* directives that can be used to get a token on behalf of Terraform. See: https://github.com/hashicorp/terraform-provider-vault/blob/789136e1bb70a40cb40ff1b763ef97e55a3df0f2/website/docs/index.html.markdown?plain=1#L110-L111 - If you feel like the token docs could be improved we'd be happy to accept a PR for that.

Ben

[guineveresaenger]

@benashz - if the docs are correct, then in that case, would a PR to fix the source be welcome?

[benashz]

@benashz - if the docs are correct, then in that case, would a PR to fix the source be welcome?

Sure, are you thinking of changing https://github.com/hashicorp/terraform-provider-vault/blob/89132d1648005d2234bfb2653a0f8386e5619447/internal/provider/provider.go#L82-L84 ?

Currently, it works because of the DefaultFunc. Removing that bit results in:

=== RUN   TestAccAuthLoginProviderConfigure
    provider_test.go:116: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: Missing required argument

          on terraform_plugin_test.tf line 1, in provider "vault":
           1: provider "vault" {}

        The argument "token" is required, but no definition was found.
--- FAIL: TestAccAuthLoginProviderConfigure (0.26s)

FAIL

Process finished with the exit code 1

[guineveresaenger]

Other way 'round - I find it really confusing that the schema marks this attribute as Required, but in practice, the DefaultFunc is applied when a token is not supplied. Shouldn't L83 be Required: false then?

[fairclothjm]

@guineveresaenger I agree that it is confusing.

For one thing, the Schema.Required doc states that it cannot be used with DefaultFunc. However, this is contradicted by the docs for DefaultFunc which state that, for legacy reasons, it can be used with Required. This allows the user to be prompted for the token as input if it is not set in the config or the environment. I believe this may have been the intended behavior for the Vault Provider. All the being said, we are not using the "prompt for input" feature because we are not returning nil in our DefaultFunc if the token is unset in the config or environment. We are returning an empty string.

Now, if we set Required to false, then we get the error token: One of optional, required, or computed must be set. The token is not Computed so we can't set that to true. But does it make sense to set Optional to true? The token isn't optional because if we don't provide it then we get the error: Error: failed to lookup token, err=Error making API request.

[guineveresaenger]

@fairclothjm - thank you for the detailed context here! I had never seen a TF provider make this particular choice, so I've learned something.

I'm going to close this PR. Thanks again for all the replies!