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

Getting a secret from an imported environment returns [unknown] #347

Open
pierskarsenbarg opened this issue Jul 1, 2024 · 5 comments
Open
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec
Milestone

Comments

@pierskarsenbarg
Copy link
Member

pierskarsenbarg commented Jul 1, 2024

What happened?

When trying to retrieve a config value from one environment that was imported through another, the value returned was [unknown]

Example

(Commands below are using the Pulumi CLI, but the one that returns the [unknown] value is the same with the ESC CLI.)

Follow steps:

  1. Set up demo-base environment: pulumi env init demo-base
  2. Set up demo-dev environment: pulumi env init demo-dev
  3. Add the following to the demo-dev environment:
imports:
    - demo-base
  1. Add secret to demo-base environment: pulumi env set demo-base pulumiConfig.myBasePassword "my-base-password" --secret
  2. Add secret to demo-dev environment: pulumi env set demo-dev pulumiConfig.myDevPassword "my-dev-password" --secret
  3. Get base secret from base environment: pulumi env get demo-base pulumiConfig.myBasePassword --show-secrets --value json
  4. Get dev secret from dev environment: pulumi env get demo-dev pulumiConfig.myDevPassword --show-secrets --value json
  5. Try to get base secret from dev environment: pulumi env get demo-dev pulumiConfig.myBasePassword --show-secrets --value json

(Will return [unknown])

if you run pulumi env open demo-dev that works correctly and the following code works as expected:

import * as pulumi from "@pulumi/pulumi";

const config = new pulumi.Config();

export const basePw = config.get("myBasePassword");
export const devPw = config.get("myDevPassword");

when Pulumi.dev.yaml is as follows:

environment:
  - demo-dev

This is what my demo-base environment looks like:

values:
  pulumiConfig:
    myBasePassword:
      fn::secret:
        ciphertext: ZXNjeAAAAAEAAAEA28KPa3Jj1chaQtbF+Hahccp22+0BMvymrx0ta5PzEX3/yKVZCwJ044qt4CSHhK6h

this is what my demo-dev environment looks like:

imports:
  - demo-base
values:
  pulumiConfig:
    myDevPassword:
      fn::secret:
        ciphertext: ZXNjeAAAAAEAAAEAHuH2JF7Gz4ad/e3ZTfR63nN6AA2G+Llrktj2HDXAvfia57zbhR0XTaQqd+oetes=

(don't forget to delete the environments when you're done: pulumi env rm demo-dev -y && pulumi env rm demo-base -y)

Output of pulumi about

CLI
Version      3.121.0
Go Version   go1.22.4
Go Compiler  gc

Plugins
KIND      NAME    VERSION
language  nodejs  unknown

Host
OS       darwin
Version  14.5
Arch     arm64

This project is written in nodejs: executable='/Users/piers/.nvm/versions/node/v20.11.1/bin/node' version='v20.11.1'

Current Stack: pierskarsenbarg/esc-secrets-issue/dev

TYPE                 URN
pulumi:pulumi:Stack  urn:pulumi:dev::esc-secrets-issue::pulumi:pulumi:Stack::esc-secrets-issue-dev


Found no pending operations associated with dev

Backend
Name           pulumi.com
URL            https://app.pulumi.com/pierskarsenbarg
User           pierskarsenbarg
Organizations  pierskarsenbarg, karsenbarg, team-ce, gitlab-test-piers, demo
Token type     personal

Dependencies:
NAME            VERSION
@pulumi/pulumi  3.121.0
@types/node     18.19.39
typescript      5.5.2

Pulumi locates its logs in /var/folders/x8/cdd9j87s607fwpy0q62mfmmw0000gn/T/ by default

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

@pierskarsenbarg pierskarsenbarg added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Jul 1, 2024
@davemyler
Copy link

This was reported on the basis of the simplest thing I could get to go wrong. It goes wrong in other situations though.

  • Using config.requireSecretObject("x") when the imported secret is within x. This reveals x,myBasePassword as a string with value "[secret]"
  • Setting an environment variable from the secret in the base config and then accessing it where it's imported does not set the environment variable.

The key part of this is it was all working Friday night when I ran pulumi down to get rid of my test infrastructure. Come Monday morning, pulumi up suddenly has auth errors as my password have vanished.

@komalali komalali removed the needs-triage Needs attention from the triage team label Jul 1, 2024
@komalali komalali added this to the 0.107 milestone Jul 1, 2024
@pgavlin
Copy link
Member

pgavlin commented Jul 16, 2024

Okay, I have some explanations for what's going on in the original report:

env get <env name> is implemented as two operations: one to get the environment definition for <env name> (i.e. the YAML), and one to analyze the YAML and return the analyzed values. show-secrets decrypts the secrets prior to analysis, but the analysis itself does not decrypt secrets, instead leaving them as [unknown]. So the fact that the secrets are showing up in the analyzed values at all is something of a side-effect, and results in unexpected behavior. We should fix ESC so that we can supply show-secrets to the analysis pass.

@davemyler I'd love separate reports or more details on the other issues you mention, though. I'm not sure what the concrete issues are there, and more information would be helpful.

@davemyler
Copy link

There were quite a few things going wrong when storing secrets in ESC and then pulling them into a Pulumi stack using an import. I've stopped using ESC for secrets for the time being as it was no longer working - so I'm not in a position to record or report on what things were failing.

What I can say with certainty however is that this problem was introduced by a change sometime between 12:30 UTC 28th June and 09:30 UTC 1st July. I had been deploying and removing stacks for weeks using ESC prior to that, but on the Monday (1st July) morning it was all broken in various ways.

@davemyler
Copy link

Because of this I suspect this all stems from a single underlying issue, so hopefully all will work again once this fix is in place. If any other issues remain afterwards I'll report separately.

pgavlin added a commit that referenced this issue Jul 17, 2024
These changes allow clients to request that CheckEnvironment decrypt
static secrets rather than treating their contents as unknown. This
allows clients to more reliably implement `showSecrets` by supplying
a decrypter and setting the `showSecrets` flag to `true`. To acheive
equivalent behavior without these changes, the client must decrypt
secrets in environment definitions prior to calling `CheckEnvironment`
and as part of `LoadEnviroment`.

Part of #347.
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes allow clients to request that CheckEnvironment decrypt
static secrets rather than treating their contents as unknown. This
allows clients to more reliably implement `showSecrets` by supplying
a decrypter and setting the `showSecrets` flag to `true`. To acheive
equivalent behavior without these changes, the client must decrypt
secrets in environment definitions prior to calling `CheckEnvironment`
and as part of `LoadEnviroment`.

Part of #347.
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes allow clients to request that CheckEnvironment decrypt
static secrets rather than treating their contents as unknown. This
allows clients to more reliably implement `showSecrets` by supplying
a decrypter and setting the `showSecrets` flag to `true`. To acheive
equivalent behavior without these changes, the client must decrypt
secrets in environment definitions prior to calling `CheckEnvironment`
and as part of `LoadEnviroment`.

Part of #347.
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes plumb support for showing _all_ static secrets through
`esc env get` (rather than only those secrets immediately present in
the environment being read directly; i.e. including secrets from
imported environments).

Note that this support is latent, and the fix will not be user-visible
until supporting code is deployed to the ESC API.

Part of #347.
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes allow clients to request that CheckEnvironment decrypt
static secrets rather than treating their contents as unknown. This
allows clients to more reliably implement `showSecrets` by supplying
a decrypter and setting the `showSecrets` flag to `true`. To acheive
equivalent behavior without these changes, the client must decrypt
secrets in environment definitions prior to calling `CheckEnvironment`
and as part of `LoadEnviroment`.

Part of #347.
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes plumb support for showing _all_ static secrets through
`esc env get` (rather than only those secrets immediately present in
the environment being read directly; i.e. including secrets from
imported environments).

Note that this support is latent, and the fix will not be user-visible
until supporting code is deployed to the ESC API.

Part of #347.
@dschaller dschaller assigned pgavlin and unassigned dschaller Jul 17, 2024
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes plumb support for showing _all_ static secrets through
`esc env get` (rather than only those secrets immediately present in
the environment being read directly; i.e. including secrets from
imported environments).

Note that this support is latent, and the fix will not be user-visible
until supporting code is deployed to the ESC API.

Part of #347.
pgavlin added a commit that referenced this issue Jul 17, 2024
These changes plumb support for showing _all_ static secrets through
`esc env get` (rather than only those secrets immediately present in
the environment being read directly; i.e. including secrets from
imported environments).

Note that this support is latent, and the fix will not be user-visible
until supporting code is deployed to the ESC API.

Part of #347.
@komalali komalali modified the milestones: 0.107, 0.108 Jul 25, 2024
@komalali komalali modified the milestones: 0.108, 0.109 Aug 15, 2024
@pgavlin
Copy link
Member

pgavlin commented Aug 20, 2024

Note: we need to do a CLI release and pull the released code into Pulumi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

5 participants