-
Notifications
You must be signed in to change notification settings - Fork 6
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
Made secret values in Deployment Settings actually work, save and refresh from source of truth #259
Conversation
…resh from source of truth.
ev[string(k)] = apitype.SecretValue{Secret: false, Value: v.StringValue()} | ||
} | ||
// This hardcodes all env vars created via provider as secret. Schema change would be needed for optional secrecy | ||
ev[string(k)] = apitype.SecretValue{Secret: true, Value: v.StringValue()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change necessary? Previously if the value passed in was a pulumi secret we would set it as a secret. Otherwise it would not be marked secret. Looks like we changed this to mark everything as secret, but it's not clear why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was not strictly necessary, but the way things were there was no way to make a secret env var. (Correct me if I'm wrong, env var map was just a map of string to string).
So I opted for secret always vs non-secret always. As I mentioned in description, it would be great to come back and change the schema and actually allow both secret and non-secret env vars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think you're mistaken. With the current implementation, if you mark something as secret the pulumi way (pulumi.Secret or config.requireSecret) it will set the the env var as secret. It's not in the schema but we do already respect secretness. Recommend reading this doc and trying it out with this change reverted. https://www.pulumi.com/docs/concepts/secrets/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test has some examples: https://github.com/pulumi/pulumi-pulumiservice/blob/main/examples/ts-deployment-settings/index.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tried it out, Output<String>
is black magic 🤯
I will need to adjust parsing in toOperationContext
, because adding secrets turns it into below struct, but other than that it should work I think.
{
struct_value: {
fields:{
key:"environmentVariables"
value: {
struct_value:{
fields:{
key:"4dabf18193072939515e22adb298388d"
value:{
string_value:"1b47061264138c4ac30d75fd1eb44270"
}
}
fields:{
key:"value"
value:{
struct_value:{
fields:{
key:"IaroEnvVarKey"
value:{
string_value:"[secret]"
}
}
}
}
}
}
}
}
}
}
I will figure out the parsing and update PR, likely next week.
PR checks are failing, so I made #261 to fix the wf |
Also this PR is blocked until this PR is in |
### Summary - The main build and test workflow has been failing for my PRs: - #259 - #260 - As far as I understood the issue, some dependency now leaves a folder called `go` with this in it: ``` go/pkg/mod/cache/download/github.com/pulumi/pulumi/pkg/v3/@v: list v3.112.0.info v3.112.0.mod ``` I just set to ignore it in .gitignore, is there a better solution maybe? I figured it was not worth tracking down where it comes from, since this looks like some package metadata, lmk if we care to find out what leaves that behind. Also: - Adding '.mono` to gitignore, dotnet compilation keeps generating it - Adding logic to not install not needed languages, which speeds up tests a little
Closing this outdated PR, finally implemented this fix in #320 |
Summary
Issues
Testing
Results in values in the resource explorer like so:
but refresh now shows no change UNLESS there was actual change.