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.

Sign up for GitHub

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

Jump to bottom

Automatically mark Secret data as Pulumi secrets #1577

Merged
merged 7 commits into from
May 25, 2021
Merged

Automatically mark Secret data as Pulumi secrets #1577

merged 7 commits into from
May 25, 2021

Conversation

lblackstone
Copy link
Member

@lblackstone lblackstone commented May 13, 2021 •
edited
Loading

Proposed changes

To avoid inadvertently leaking sensitive data into the Pulumi
state for Kubernetes v1/Secret resources, this change manually
marks the "stringData" and "data" fields as secret. The output
fields were already marked as secret.

This combination will also prevent sensitive data that appears
in raw YAML from the YAML and Helm SDKs from appearing
in the state.

Related issues (optional)

Fix #999
Fix #1353

@lblackstone lblackstone requested a review from viveklak May 13, 2021 22:27
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

viveklak
viveklak reviewed May 13, 2021
View reviewed changes
sdk/nodejs/yaml/yaml.ts Outdated Show resolved Hide resolved
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

1 similar comment
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone lblackstone force-pushed the lblackstone/yaml-secrets branch from 49a08ef to 409725f Compare May 18, 2021 21:00
@lblackstone lblackstone changed the title Mask data from YAML Secrets Automatically mark Secret data as Pulumi secrets May 18, 2021
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@viveklak
Copy link
Contributor

This seems important enough for us to have a test case for each sdk?

@lblackstone
Automatically mark Secret data as Pulumi secrets
46c8c95 
To avoid inadvertently leaking sensitive data into the Pulumi
state for Kubernetes v1/Secret resources, this change manually
marks the "stringData" and "data" fields as secret. The output
fields were already marked as secret.

This combination will also prevent sensitive data that appears
in raw YAML from the YAML and Helm SDKs from appearing
in the state.
@lblackstone
Python
21b638e
@lblackstone
Go
3dfa9ea
@lblackstone lblackstone force-pushed the lblackstone/yaml-secrets branch from b9e9569 to 1e2ffc7 Compare May 19, 2021 21:29
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone lblackstone force-pushed the lblackstone/yaml-secrets branch from 1e2ffc7 to 99fa44e Compare May 20, 2021 20:20
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone lblackstone marked this pull request as ready for review May 21, 2021 21:28
@lblackstone
Copy link
Member Author

@viveklak Added tests and finished the .NET implementation

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@viveklak
Copy link
Contributor

Looks like there might be some test failures? Looks good otherwise!

@lblackstone lblackstone force-pushed the lblackstone/yaml-secrets branch from 3f6836c to 80eaf5b Compare May 21, 2021 22:28
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lukehoban
Copy link
Contributor

Overriding the generated files with these point-in-time snapshots feels like it introduces non-trivial maintainability debt - and a lot of ways we could accidentally not pick up other codegen and/or functionality changes in the future.

Could we just improve the code generators to support generating the right thing here? (A secret annotation on inputs)?

@lblackstone lblackstone force-pushed the lblackstone/yaml-secrets branch from 80eaf5b to 7c60e18 Compare May 21, 2021 22:56
@lblackstone
Copy link
Member Author

Could we just improve the code generators to support generating the right thing here?

Most likely, but I wanted to make sure everything was working as expected in the k8s provider first. My intention is to follow up with the schema changes in the near term. Since all of these tests will already be in place, it should be easier to make sure the codegen is working as expected.

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

1 similar comment
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone
Copy link
Member Author

I started working on the code generators to see how difficult it would be to generalize this, and am about halfway done with the required changes now. My plan for this PR is:

  1. Finish the codegen changes, and manually update the template files to match the expected output
  2. Merge this PR with the TODO's for updating to codegen
  3. Remove the templates + overrides when we bump the p/p dependency after the next release

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone
Copy link
Member Author

Codegen changes are here: pulumi/pulumi#7128

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone lblackstone force-pushed the lblackstone/yaml-secrets branch from 89fbc49 to 706a448 Compare May 24, 2021 23:03
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone lblackstone merged commit 651a7b5 into master May 25, 2021
@pulumi-bot pulumi-bot deleted the lblackstone/yaml-secrets branch May 25, 2021 00:12
@lblackstone lblackstone mentioned this pull request May 26, 2021
Merged
@renovate renovate bot mentioned this pull request May 26, 2021
Open
1 task
@renovate renovate bot mentioned this pull request Jun 6, 2021
Open
1 task
@mikhailshilkov mikhailshilkov mentioned this pull request Apr 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukehoban lukehoban lukehoban left review comments

@viveklak viveklak viveklak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@lblackstone @viveklak @lukehoban