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

Fixing Deployment Secrets - Parallel Values #320

Closed
wants to merge 1 commit into from

Conversation

IaroslavTitov
Copy link
Contributor

@IaroslavTitov IaroslavTitov commented Jun 14, 2024

Note: this is an alternative approach for PR - #325

Summary

Testing

  • Tested pulumi up, refresh, import and up from previous version of the provider (for unchanged DS inputs, migrating to this new way of saving will require refresh and then up)

Example TS program (Sadly can't use Dotnet, due to bug with maps):

const settings = new service.DeploymentSettings("deployment_settings", {
  organization: "IaroslavTitov",
  project: "PulumiDotnet",
  stack: "SdkTest5",
  operationContext: {
    environmentVariables: {
      TEST_VAR: "fooa",
      SECRET_VAR: config.requireSecret("my_secret"),
    }
  },
  sourceContext: {
    git: {
        repoUrl: "https://github.com/pulumi/deploy-demos.git",
        branch: "refs/heads/main",
        repoDir: "pulumi-programs/simple-resource",
        gitAuth: {
            sshAuth: {
                sshPrivateKey: "privatekey",
                password: secret,
            }
        }
    }
}
});

Secret resource values end up split into cipher and plaintext like this:
    "sshPrivateKeyCipher": "AAABAJrXK3j7+jgXLJI9Q30JnAFDxek6yUyYudh3h2SU4/dQKyBl",
    "sshPrivateKey": {
      "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
      "ciphertext": "AAABAKlREY4tsjd1bbWc8JXPaABRqG6BeVxfUkhO1ob70Q=="
    }
twin values for each secret. Environment variables ciphers are stored in a separate variable `environmentVariablesCipher`

Passwords and sshKey are forced into twin secrets, Environment Variables are optionally twin secrets, everything else uses normal Pulumi workdlows, because they are not secret in Pulumi Service.

Import of the above code generates successfully with dummy values for secrets:

const ds1 = new pulumiservice.DeploymentSettings("ds1", {
operationContext: {
environmentVariables: {
SECRET_VAR: pulumi.secret(""),
TEST_VAR: "fooa",
},
},
organization: "IaroslavTitov",
project: "PulumiDotnet",
sourceContext: {
git: {
branch: "refs/heads/main",
gitAuth: {
sshAuth: {
password: pulumi.secret(""),
sshPrivateKey: pulumi.secret(""),
},
},
repoDir: "pulumi-programs/simple-resource",
repoUrl: "https://github.com/pulumi/deploy-demos.git",
},
},
stack: "SdkTest5",
}, {
protect: true,
});

@IaroslavTitov IaroslavTitov force-pushed the iaro/secrets branch 6 times, most recently from f583aaf to 8c2c865 Compare June 14, 2024 23:02
@IaroslavTitov IaroslavTitov marked this pull request as ready for review June 14, 2024 23:09
Comment on lines +107 to +111
type SecretValue struct {
Value string // Plaintext if Secret is false; ciphertext otherwise.
Secret bool
}

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this return both Value and Ciphertext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, Pulumi Service only returns either plaintext or ciphertext, with the Secret bool indicating which one. But since we opted not to have the ability to return plaintext on these secret deployment settings, it only ever returns ciphertext.

This whole struct tree is taken from sdk, I unfrotunately had to copy it in to override the marshaling/unmarshaling logic

provider/cmd/pulumi-resource-pulumiservice/schema.json Outdated Show resolved Hide resolved
@IaroslavTitov IaroslavTitov force-pushed the iaro/secrets branch 4 times, most recently from 426457c to eab8682 Compare June 17, 2024 21:02
@IaroslavTitov
Copy link
Contributor Author

Updated the code.
This way it's not a breaking change, which is well worth the extra bit of complexity, I just didn't see this way until today.

The env vars are now stored as map of string to object, inside which ciphertext and plaintext are packed. Import now needs a separate flag for serialization into Inputs, separate from serialization into Properties, but it's a pretty small change.

@IaroslavTitov IaroslavTitov force-pushed the iaro/secrets branch 3 times, most recently from d361a4a to 157b476 Compare June 17, 2024 22:01
@IaroslavTitov
Copy link
Contributor Author

Ran into an issue with python tests, filed #322 to address later, looks like another bug with providers

@@ -0,0 +1,250 @@
// Code generated by pulumi-language-go DO NOT EDIT.
Copy link
Member

Choose a reason for hiding this comment

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

This diff is surprising in this PR. Did we forget to include this earlier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, the go files only didn't generate for Environment resources, Stoo pinged me about it. Looking into it, a while ago I updated .gitignore and it's covering too much, which caused this. I'll have to address the gitignore issue, but figured should include the missed files asap

Copy link
Member

Choose a reason for hiding this comment

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

Can we pull this out into a separate PR and get that merged and released? I'd rather not hold that up on this PR.

@IaroslavTitov IaroslavTitov changed the title Fixing Deployment Secrets Fixing Deployment Secrets - Parallel Values Jun 20, 2024
@IaroslavTitov IaroslavTitov force-pushed the iaro/secrets branch 2 times, most recently from 75a63bd to da6b3bc Compare June 21, 2024 23:07
IaroslavTitov added a commit that referenced this pull request Jun 26, 2024
### Note: there is an alternative implementation of this PR -
#320

### Summary

- Added logic to save ciphertext into the output properties for secret
values, allowing comparison on refresh of just ciphertext, fixing
#123
- Import now works as well, including code generation (with dummy values
for secrets)
- Migrated to new PUT API and updated client to actually return
DeploymentSettings

### Testing
- Tested pulumi up, refresh, import and up from previous version of the
provider (for unchanged DS inputs, migrating to this new way of saving
will require refresh and then up)

Example TS program (Sadly can't use Dotnet, due to bug with maps):
```
const settings = new service.DeploymentSettings("deployment_settings", {
  organization: "IaroslavTitov",
  project: "PulumiDotnet",
  stack: "SdkTest5",
  operationContext: {
    environmentVariables: {
      TEST_VAR: "fooa",
      SECRET_VAR: config.requireSecret("my_secret"),
    }
  },
  sourceContext: {
    git: {
        repoUrl: "https://github.com/pulumi/deploy-demos.git",
        branch: "refs/heads/main",
        repoDir: "pulumi-programs/simple-resource",
        gitAuth: {
            sshAuth: {
                sshPrivateKey: "privatekey",
                password: secret,
            }
        }
    }
}
});
```

Secret resource values end up with just cipher, while plaintext is
stored in inputs:
```
      "sshAuth": {
        "password": "AAABAD6/2Nroj62qORoHOLofFOkRhdUNwCAYeC86nABU/G4AO5I7Fw==",
        "sshPrivateKey": "AAABABqRIQ1bZbvU/hrlpX1Rh9sj9OCyArjG0SUILPQmb0KSCFIrz6bK"
      }
```

Passwords and sshKey are forced into twin secrets, Environment Variables
are optionally twin secrets, everything else uses normal Pulumi
workflows, because they are not secret in Pulumi Service.

Import of the above code generates successfully with dummy values for
secrets:
```
const ds1 = new pulumiservice.DeploymentSettings("ds1", {
    operationContext: {
        environmentVariables: {
            SECRET_VAR: pulumi.secret("<REPLACE WITH ACTUAL SECRET VALUE>"),
            TEST_VAR: "fooa",
        },
    },
    organization: "IaroslavTitov",
    project: "PulumiDotnet",
    sourceContext: {
        git: {
            branch: "refs/heads/main",
            gitAuth: {
                sshAuth: {
                    password: pulumi.secret("<REPLACE WITH ACTUAL SECRET VALUE>"),
                    sshPrivateKey: pulumi.secret("<REPLACE WITH ACTUAL SECRET VALUE>"),
                },
            },
            repoDir: "pulumi-programs/simple-resource",
            repoUrl: "https://github.com/pulumi/deploy-demos.git",
        },
    },
    stack: "SdkTest5",
}, {
    protect: true,
});
```
@komalali
Copy link
Member

Can we close this out @IaroslavTitov ?

@IaroslavTitov
Copy link
Contributor Author

Closing as this was just an alternate version

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.

2 participants