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

✨ ec2: Add support for userdata privacy #1490

Merged
merged 1 commit into from
Feb 4, 2020
Merged

✨ ec2: Add support for userdata privacy #1490

merged 1 commit into from
Feb 4, 2020

Conversation

randomvariable
Copy link
Member

@randomvariable randomvariable commented Jan 21, 2020 •
edited

Signed-off-by: Naadir Jeewa jeewan@vmware.com

What this PR does / why we need it:
Re-introduction of #189
Fixes #1387
Fixes #1410
Fixes #1510

Optionally modifies the userdata to make use of a multi-part MIME archive to make cloud-init download userdata from AWS Secrets Manager.

In support of ensuring secrets are deleted as soon as they are not useful, the EC2 state machine has been refined to skip over operations that cannot occur on terminating and terminated instances.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 21, 2020
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 21, 2020
vincepri
vincepri reviewed Jan 21, 2020
View reviewed changes
pkg/cloud/services/ec2/instances.go Outdated Show resolved Hide resolved
pkg/cloud/services/ec2/instances.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/ssm-fetch.py Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/parameter.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/parameter.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/parameter.go Outdated Show resolved Hide resolved
hack/tools/embed/main.go Outdated Show resolved Hide resolved
ncdc
ncdc reviewed Jan 21, 2020
View reviewed changes
api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha3/awsmachine_types.go Outdated Show resolved Hide resolved
pkg/cloud/services/ec2/instances.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/parameter.go Outdated Show resolved Hide resolved
hack/tools/embed/main.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/userdata.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/userdata.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/userdata.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/parameter.go Outdated Show resolved Hide resolved
pkg/cloud/services/ssm/ssm-fetch.py Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 24, 2020
ncdc
ncdc reviewed Jan 24, 2020
View reviewed changes
Copy link
Contributor

@ncdc ncdc left a comment

Choose a reason for hiding this comment

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

Haven't made it quite all the way through, but it's EOW for me, so I wanted to submit what I have. Huge thanks for putting this together!

api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha2/zz_generated.conversion.go Outdated Show resolved Hide resolved
controllers/awsmachine_controller.go Outdated Show resolved Hide resolved
controllers/awsmachine_controller.go Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret-fetch.sh Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret-fetch.sh Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret-fetch.sh Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret-fetch.sh Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret-fetch.sh Outdated Show resolved Hide resolved
vincepri
vincepri reviewed Jan 25, 2020
View reviewed changes
pkg/cloud/services/secretsmanager/cloudinit.go Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/cloudinit.go Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret.go Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/secret.go Outdated Show resolved Hide resolved
api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
pkg/cloud/services/interfaces.go Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/cloudinit.go Outdated Show resolved Hide resolved
pkg/cloud/services/ec2/instances.go Outdated Show resolved Hide resolved
pkg/cloud/services/secretsmanager/cloudinit.go Outdated Show resolved Hide resolved
api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
@ncdc ncdc added this to the v0.5.0 milestone Jan 27, 2020
@randomvariable randomvariable mentioned this pull request Jan 28, 2020
Closed
@randomvariable randomvariable force-pushed the encrypted-userdata branch 4 times, most recently from 877ceac to 4180851 Compare January 28, 2020 22:51
vincepri
vincepri reviewed Jan 28, 2020
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@randomvariable randomvariable force-pushed the encrypted-userdata branch 6 times, most recently from 442ea6e to e02dc3c Compare January 29, 2020 13:30
@randomvariable randomvariable changed the title [wip] ec2: Add support for encrypted userdata ec2: Add support for encrypted userdata Jan 29, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2020
@randomvariable
Copy link
Member Author

randomvariable commented Jan 29, 2020 •
edited

/hold
pending conclusion of backport discussion

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 29, 2020
vincepri
vincepri reviewed Feb 4, 2020
View reviewed changes
api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha2/awsmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha3/awsmachine_types.go Outdated Show resolved Hide resolved
vincepri
vincepri reviewed Feb 4, 2020
View reviewed changes
pkg/cloud/scope/machine.go Outdated Show resolved Hide resolved
@randomvariable randomvariable force-pushed the encrypted-userdata branch 3 times, most recently from e5f795e to 25c208f Compare February 4, 2020 17:42
@randomvariable
Copy link
Member Author

rebasing for new conversion gen

@randomvariable
Copy link
Member Author

rebasing for new conversion gen
done

vincepri
vincepri approved these changes Feb 4, 2020
View reviewed changes
Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/lgtm

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2020
@randomvariable
Copy link
Member Author

argh. some of these converted function names changed

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2020
@randomvariable
Copy link
Member Author

linters are appeased

@vincepri
Copy link
Member

vincepri commented Feb 4, 2020

waiting for green CI

@randomvariable randomvariable changed the title ec2: Add support for userdata privacy ✨ ec2: Add support for userdata privacy Feb 4, 2020
@randomvariable
Copy link
Member Author

done

@vincepri
Copy link
Member

vincepri commented Feb 4, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2020
@k8s-ci-robot k8s-ci-robot merged commit b34e884 into kubernetes-sigs:master Feb 4, 2020
@ncdc
Copy link
Contributor

ncdc commented Feb 4, 2020 •
edited

Need to make sure we record action-required changes to the IAM policy b/c of this PR in the release notes.

@randomvariable randomvariable deleted the encrypted-userdata branch February 4, 2020 18:17
@randomvariable randomvariable mentioned this pull request Feb 4, 2020
Merged
@randomvariable randomvariable mentioned this pull request May 12, 2020
Closed
@randomvariable randomvariable mentioned this pull request Aug 18, 2020
Closed
This was referenced Jan 18, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncdc ncdc ncdc left review comments

@joonas joonas joonas left review comments

@detiber detiber detiber left review comments

@jzhoucliqr jzhoucliqr jzhoucliqr left review comments

@fabriziopandini fabriziopandini fabriziopandini left review comments

@vincepri vincepri vincepri approved these changes

@chuckha chuckha Awaiting requested review from chuckha

@justinsb justinsb Awaiting requested review from justinsb

Assignees

@vincepri vincepri

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v0.5.0
8 participants
@randomvariable @vincepri @k8s-ci-robot @detiber @ncdc @joonas @jzhoucliqr @fabriziopandini