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

Adding Environment Resource #271

Merged
merged 2 commits into from
May 21, 2024
Merged

Adding Environment Resource #271

merged 2 commits into from
May 21, 2024

Conversation

IaroslavTitov
Copy link
Contributor

@IaroslavTitov IaroslavTitov commented May 10, 2024

Summary

  • Adding Environment resource
  • Adding dependency on ESC client
  • Fixes 255

Testing

Tested in dev stack using below Pulumi Program (Dotnet SDK), confirmed that things like imports, opening environment work as expected.

    String yaml = """
    imports:
      - dev-stacks
    values:
      aws:
        secrets:
          fn::open::aws-secrets:
            region: us-west-2
            login: ${aws.creds}
            get:
              allSecrets:
                secretId: iaro-dev-stack/pulumi-service
      secrets:
        fn::fromJSON: ${aws.secrets.allSecrets}
      pulumiConfig:
        gitHubOAuthID: ${secrets.gitHubOAuthID}
        gitHubOAuthSecret: ${secrets.gitHubOAuthSecret}
    """;

    var environ = new Environment(
        "Iaro's environment",
        new EnvironmentArgs {
            Organization = "IaroslavTitov",
            Name = "IaroEnv",
            Yaml = new StringAsset(yaml)
        }
    );

@IaroslavTitov IaroslavTitov force-pushed the iaro/env branch 4 times, most recently from f964e07 to 8219f71 Compare May 10, 2024 21:44
@IaroslavTitov IaroslavTitov marked this pull request as ready for review May 13, 2024 18:37
Comment on lines +1234 to +1282
"yaml": {
"description": "Environment's yaml file.",
"$ref": "pulumi.json#/Asset"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should make absolutely certain that assets work with output properties. Not 100% sure about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed it works with output using Random and below snippet of code:

var random = new RandomString("rand", new RandomStringArgs
    {
        Length = 5,
        Special = false,
    });

    Output<AssetOrArchive> asset = random.Result.Apply(res => {
        String yaml = """
            imports:
              - dev-stacks
            values:
              aws:
                secrets:
                  fn::open::aws-secrets:
                    region: us-west-2
                    login: ${aws.creds}
                    get:
                      allSecrets:
                        secretId: iaro-dev-stack/pulumi-service
              secrets:
                fn::fromJSON: ${aws.secrets.allSecrets}
              pulumiConfig:
                gitHubOAuthID: ${secrets.gitHubOAuthID}
                gitHubOAuthSecret: ${secrets.gitHubOAuthSecret}
                randomString: 
        """ + res;
        AssetOrArchive asset = new StringAsset(yaml);
        return asset;
        }
    );

    var environ = new Pulumi.PulumiService.Environment(
        "Iaro's environment",
        new EnvironmentArgs {
            Organization = "IaroslavTitov",
            Name = "IaroEnv",
            Yaml = asset
        }
    );

When I go to see the environment in console it has a random blurb zbhrh and the same in Random's resource properties.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I should clarify--I meant that I'm not sure that the output property Environment.Yaml will work with assets. I'd try passing the Yaml output of one environment to the Yaml input of another.

Copy link
Contributor Author

@IaroslavTitov IaroslavTitov May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh, gotcha, didn't understand properly
Confirmed that this works, creating 2 environments with identical yaml:

var environ = new Pulumi.PulumiService.Environment(
        "Iaro's environment",
        new EnvironmentArgs {
            Organization = "IaroslavTitov",
            Name = "IaroEnv",
            Yaml = asset
        }
    );

    var environ2 = new Pulumi.PulumiService.Environment(
        "Iaro's environment 2",
        new EnvironmentArgs {
            Organization = "IaroslavTitov",
            Name = "IaroEnv2",
            Yaml = environ.Yaml.Apply(yaml => yaml)
        }
    );

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW @IaroslavTitov you shouldn't have to do this:

Yaml = environ.Yaml.Apply(yaml => yaml)

It can just be:

Yaml = environ.Yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, makes sense, got stuck in the Apply mindset

provider/pkg/provider/environment.go Outdated Show resolved Hide resolved
provider/pkg/provider/environment.go Outdated Show resolved Hide resolved
@pgavlin
Copy link
Member

pgavlin commented May 17, 2024

High-level question:

If my environment definition contains secrets, are those stored in plaintext unless the entire environment definition is marked secret? For example, in the following program, will foo's value be stored in plaintext?

const env = new Environment("env", { yaml: `{ values: { foo: { fn::secret: hunter2 } } }` });

@IaroslavTitov
Copy link
Contributor Author

Tried this, shows up as plaintext in the Resources tab. (In Environments, shows up as ciphertext)

Unfortunately, I don't think we can avoid storing it as plaintext, in order for import to work, this is the same issue as with Deployment Settings. We can add some frontend logic to *** them out maybe? Still obv wouldn't actually hide the secrets, just visually.

@pgavlin
Copy link
Member

pgavlin commented May 17, 2024

So users can manually mark the environment def as secret in their Pulumi program:

const env = new Environment("env", { yaml: pulumi.secret(`{ values: { foo: { fn::secret: hunter2 } } }`) });

That will encrypt the YAML in the statefile. We might also be able to do this automatically as part of Check, but I'm not certain. Check might be able to wrap the input YAML in a Secret prior to returning the inputs to the engine. I think that's worth a look.

Unfortunately, I don't think we can avoid storing it as plaintext, in order for import to work, this is the same issue as with Deployment Settings

Yeah we need to chat more about our approach to diffing with secrets. I think that we have several options here, but all of them will involve tradeoffs.

@IaroslavTitov
Copy link
Contributor Author

IaroslavTitov commented May 17, 2024

Check might be able to wrap the input YAML in a Secret prior

Ooh, I did not think of that, I think that shouldn't be hard, I'll look into it and update the PR.

we need to chat more about our approach to diffing with secrets

Yep, that's the purpose of that doc, to get a conversation going

@IaroslavTitov IaroslavTitov force-pushed the iaro/env branch 2 times, most recently from ac2134f to 473b0bf Compare May 20, 2024 19:35
@IaroslavTitov
Copy link
Contributor Author

Updated to force yaml to always be a secret property.

I then went down the rabbit hole trying to solve noisy diff when user has extra spaces in the yaml, which ESC then removes and causes diff. Sadly, found no way of eliminating this issue with secrets, as Diff receives only hash of a secret Asset, not full text and this can only be compared exactly, and not partially. It is a rare chance anyone will hit that issue though (I totally accidentally stumbled into it), so I think it's fine.

@komalali
Copy link
Member

@IaroslavTitov can you resolve the conflicts please?

@IaroslavTitov
Copy link
Contributor Author

@IaroslavTitov can you resolve the conflicts please?

Done!

@IaroslavTitov IaroslavTitov merged commit 3bdfc31 into main May 21, 2024
13 checks passed
@IaroslavTitov IaroslavTitov deleted the iaro/env branch May 21, 2024 19:20
IaroslavTitov added a commit that referenced this pull request May 22, 2024
# Note: 
This PR should only be merged in after rebasing on top of [this
PR](#271) that
introduces Environment resource

### Summary

- Fixes [280](#280)
- Adding examples for C#, TypeScript, Go, Python and Yaml

### Testing
- workflow runs all these tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ESC Environment Resource to the Pulumi Cloud Provider
3 participants