pulumi / pulumi-vault

A Vault Pulumi resource package, providing multi-language access to HashiCorp Vault
Apache License 2.0
25 stars 5 forks source link

Vault Provider Args `token` should not be mandatory #509

Open arpanrec opened 2 months ago

arpanrec commented 2 months ago

What happened?

Vault Provider Args token should not be mandatory, as we have other authentication methods. in node_modules/@pulumi/vault/provider.d.ts

    /**
     * Token to use to authenticate to Vault.
     */
    token: pulumi.Input<string>;

Example

const vaultSource = new vault.Provider(`vault-${resourceUniqueIdPrefix}`, {
        address: process.env.VAULT_ADDR || '',
        token: 'bug', // not needed but it's marked mandatory in the type definition
        clientAuth: {
            certFile: path.join(vaultCertDir, 'client_certificate.pem'),
            keyFile: path.join(vaultCertDir, 'client_private_key.pem'),
        },
        caCertFile: path.join(vaultCertDir, 'root_ca_certificate.pem'),
        authLogin: {
            path: 'auth/approle/login',
            parameters: {
                role_id: process.env.VAULT_APPROLE_ROLE_ID || '',
                secret_id: process.env.VAULT_APPROLE_SECRET_ID || '',
            },
        },
    });

Output of pulumi about

❯ pulumi about CLI
Version 3.116.0 Go Version go1.22.2 Go Compiler gc

Host
OS arch Version "rolling" Arch x86_64

Backend
Name pulumi.com URL https://app.pulumi.com User Unknown Organizations
Token type personal

Additional context

It is not mandatory in terraform vault provider. https://registry.terraform.io/providers/hashicorp/vault/latest/docs#token

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

guineveresaenger commented 2 months ago

Hey @arpanrec - thank you for reporting this issue. We'll take a look as soon as we can.

guineveresaenger commented 2 months ago

Hi @arpanrec - I took a quick peek upstream and it appears from the schema that token is in fact Required - the documentation is incorrect.

Can you tell us a bit more in which way this presents as a blocker to you? Does declaring token: '' provide a reasonable workaround?

guineveresaenger commented 2 months ago

Note that I opened https://github.com/hashicorp/terraform-provider-vault/pull/2237 upstream.

arpanrec commented 2 months ago

@guineveresaenger , as you can see in the code example above I have done exactly that, token: 'bug', and yes, it's NOT a blocker.

But I have never looked at the upstream source code of terraform-vault-provider. I just saw Terraform docs and I saw it's marked optional.

I can see you have done all the research already https://github.com/hashicorp/terraform-provider-vault/pull/2237 here. Thanks

guineveresaenger commented 2 months ago

@arpanrec - All right, so I've gotten some more context here.

First off, the current workaround for this issue is, as you discovered, passing an empty string to token.

Upstream has made the choice to allow for the schema to say Required but as the field has a DefaultFunc on it (this is the confusing choice), the fallback behavior is always an empty string. They've decided to document this as Optional in the documentation.

On our end, we do need to support this kind of behavior better, so I've filed https://github.com/pulumi/pulumi-terraform-bridge/issues/1978 to follow up.

I'm leaving this issue open and will close when the workaround is no longer necessary. Thanks again for the find!

arpanrec commented 2 months ago

Yes, I agree, I should have checked the source code in Terraform vault provider first, as the conflict is in the source code vs terraform docs. my bad.

Off-Topic: Any way loving pulumi in big terraform to pulumi rewrite in TS. Keep it up.