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

OIDC Access Token stored in state even if using env variable - results in expired credentials when destroying after expiry #1759

Closed
MitchellGerdisch opened this issue Mar 1, 2024 · 6 comments · Fixed by #1814
Assignees
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed

Comments

@MitchellGerdisch
Copy link

What happened?

What happened?

I have an ESC environment set up for GCP OIDC named gcp-access

values:
  gcp:
    projectName: myProject
    login:
      fn::open::gcp-login:
        project: 1234456789
        oidc:
          workloadPoolId: myPoolId
          providerId: pulumi-cloud-pequod-oidc
          serviceAccount: [email protected]
          tokenLifetime: "1h"
  environmentVariables:
    GOOGLE_PROJECT: ${gcp.login.project}
    GOOGLE_OAUTH_ACCESS_TOKEN: ${gcp.login.accessToken}

I have a project with a Pulumi.dev.yaml file that looks like this:

environment:
  - gcp-access

I run pulumi up for a gcp program (pulumi new gcp-typescript is a good example to use).

Running pulumi stack export or looking at the provider in the Pulumi Cloud UI will show the accessToken is stored in state.

This causes problems when later, after the temporary token has expired, trying to destroy the stack will fail due to expired credentials.

Example

See above

Output of pulumi about

CLI
Version 3.107.0
Go Version go1.22.0
Go Compiler gc

Plugins
NAME VERSION
gcp 7.11.2
nodejs unknown

Host
OS darwin
Version 13.6.3
Arch x86_64

This project is written in nodejs

Current Stack: dev

TYPE URN
pulumi:pulumi:Stack urn:pulumi:dev::gcp-simple-test::pulumi:pulumi:Stack::gcp-simple-test-dev
pulumi:providers:gcp urn:pulumi:dev::gcp-simple-test::pulumi:providers:gcp::default_7_11_2
gcp:storage/bucket:Bucket urn:pulumi:dev::gcp-simple-test::gcp:storage/bucket:Bucket::my-bucket2

Found no pending operations associated with dev

Backend
Name pulumi.com

Dependencies:
NAME VERSION
@pulumi/gcp v7.11.2
@pulumi/pulumi 3.107.0
@types/node 18.19.21

Additional context

No response

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).

@MitchellGerdisch MitchellGerdisch added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Mar 1, 2024
@iwahbe iwahbe self-assigned this Mar 1, 2024
@iwahbe iwahbe added needs-triage Needs attention from the triage team p1 A bug severe enough to be the next item assigned to an engineer and removed needs-triage Needs attention from the triage team labels Mar 1, 2024
@iwahbe iwahbe assigned VenelinMartinov and unassigned iwahbe Mar 11, 2024
@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Mar 11, 2024

This looks like it was introduced in #1691 - commenting out that config seems to fix the issue.

Confirmed that this regressed in 7.11 - the issues is not present in 7.9 (7.10 does not exist)

Not clear why the config is being saved from the env var in #1691

VenelinMartinov added a commit that referenced this issue Mar 11, 2024
fixes #1759

Reverts #1715 and
#1691.

The TF providers was already picking up credentials from the env var
before #1691. We have no reason
to store these in the state.

Note that this might break users who depend on the the stored
credentials for authentication. I think this is intended and they can
set up an env var or explicitly configure the provider to store the
credentials.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Mar 11, 2024
@sylver
Copy link

sylver commented Apr 24, 2024

@VenelinMartinov I had the exact same problem today, the issue is still happening with the latest version of the provider.

From what I understand (sorry if I'm wrong) your revert PR (#1814) does not seem to fix the problem and I think actually (re)introduce a few problems:

Please see my thread on slack with @komalali : https://pulumi-community.slack.com/archives/C0602S4P4T1/p1713952731427519

@VenelinMartinov
Copy link
Contributor

Hi @sylver, sorry you are having trouble with this. I'd really appreciate a repro here - it's very hard to take any action without repro steps.

I don't believe #1814 should have caused a regression as we supported env var tokens and credentials before #1691 - 1691 was not necessary to begin with.

Looking at the slack thread, it looks like you are setting the credentials as config - can you try removing the config and just using the env variables? Thanks!

@VenelinMartinov
Copy link
Contributor

@sylver Have a look at the example here for how to use dynamic credentials with ESC and GCP.

@sylver
Copy link

sylver commented Apr 25, 2024

Hey @VenelinMartinov, thanks for coming back to me.

Looking at the slack thread, it looks like you are setting the credentials as config - can you try removing the config and just using the env variables? Thanks!

As stated in the slack thread here https://pulumi-community.slack.com/archives/C0602S4P4T1/p1713974080632999?thread_ts=1713952731.427519&cid=C0602S4P4T1

Using

values:
  environmentVariables:
    CLOUDSDK_AUTH_ACCESS_TOKEN: ${gcp.login.accessToken}
    GOOGLE_OAUTH_ACCESS_TOKEN: ${gcp.login.accessToken}
    GOOGLE_PROJECT: ${gcp.login.project}
  gcp:
    login:
      fn::open::gcp-login:
        oidc:
          providerId: pulumi-oidc-provider
          serviceAccount: [email protected]
          workloadPoolId: pulumi-oidc-identity-pool
        project: 12345

(without the pulumiConfig values)

returns

    error: Missing required configuration variable 'gcp:accessToken'
        please set a value using the command `pulumi config set gcp:accessToken <value>`

I'd really appreciate a repro here - it's very hard to take any action without repro steps.

Sure, I understand, not sure what should I provide you with though.

  • I think you should be able to reproduce the error I get from not setting the pulumiConfig and only using environment variables, else that would make it a local issue and I have no idea what could create such discrepancy on my end
  • About the accessToken stored in cleartext, that's also an easy one, set a custom gcp provider
    const provider = new gcp.Provider('gcp-oidc-provider', {
      accessToken: gcpAccessToken,
      project: gcpProject,
      region: gcpRegion,
    });
    once having a successful pulumi up, explore the state and see by yourself the value being not encrypted (compared to the default provider behavior)
  • About the main issue, that's a tricky one, an edge case once you understand it, but still dangerous if that happens. to "force" it to happen :
    • within a project with a few resources (best with dependencies), and ofc the oidc provider setup, enforce a short lifetime (tokenLifetime in the oidc args (https://www.pulumi.com/docs/esc/providers/gcp-login/#gcploginoidc))
    • do a successful pulumi up creating all resources
    • based on the token lifetime you set, time properly a pulumi destroy so that the token expires in the middle of the resources destruction
    • now you have a failed destroy and an unhealthy dependency graph in your stack
    • you should not be able to do a pulumi up successful again, thus not being able to refresh the token, so you're stuck both in up or destroy
      (I'll try to make it happens again in a repro, but that's not an easy one)

@VenelinMartinov
Copy link
Contributor

hey @sylver, I've tried the ESC environment and it seemed to work. I suspect we are doing something different. It could be something in the pulumi program which triggers this behaviour. Maybe an explicit config.require?

Can I please ask you to open a new issue with the following:

  1. minimal pulumi program + esc environment
  2. output of pulumi about in that program.

Such that the Missing required configuration variable 'gcp:accessToken' error pops up? Very much appreciated 🙏

As for the plaintext access token, I've raised #1956 - we'll prioritise that one and investigate as soon as possible.

@VenelinMartinov VenelinMartinov added the impact/regression Something that used to work, but is now broken label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants