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

Prevent creation of ssh-keypair.old secret on Shoot creation. #5388

Merged
merged 6 commits into from
Mar 7, 2022
Merged

Prevent creation of ssh-keypair.old secret on Shoot creation. #5388

merged 6 commits into from
Mar 7, 2022

Conversation

ary1992
Copy link
Contributor

@ary1992 ary1992 commented Feb 3, 2022

How to categorize this PR?

/area quality
/area auto-scaling
/kind enhancement

What this PR does / why we need it:
Prevent gardenlet from creating ssh-keypair.old Secret on Shoot creation.
Which issue(s) this PR fixes:
Fixes #4527

Special notes for your reviewer:
A new annotation field gardener.cloud/ssh-keypair is added to shoot when ssh-keypair is rotated.
Release note:

NONE

@ary1992 ary1992 requested a review from a team as a code owner February 3, 2022 07:12
@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion area/auto-scaling Auto-scaling (CA/HPA/VPA/HVPA, predominantly control plane, but also otherwise) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/enhancement Enhancement, improvement, extension size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 3, 2022
acumino
acumino reviewed Feb 3, 2022
View reviewed changes
Copy link
Member

@acumino acumino left a comment

Choose a reason for hiding this comment

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

Just curious?
Is it not nice to have just key only something like ShootSSHKeypairRotated, it can be set to true indicating that the SSH keypair for the shoot nodes has been rotated at least once.

@ary1992
Copy link
Contributor Author

ary1992 commented Feb 3, 2022

Just curious?
Is it not nice to have just key only something like ShootSSHKeypairRotated, it can be set to true indicating that the SSH keypair for the shoot nodes has been rotated at least once.

Annotations also acts as key-value pair. Here we have assigned the value rotated to annotation gardener.cloud/ssh-keypair. Moreover , where should the field ShootSSHKeypairRotated be included ?

@acumino
Copy link
Member

acumino commented Feb 3, 2022 •
edited
Loading

Just curious?
Is it not nice to have just key only something like ShootSSHKeypairRotated, it can be set to true indicating that the SSH keypair for the shoot nodes has been rotated at least once.

Annotations also acts as key-value pair. Here we have assigned the value rotated to annotation gardener.cloud/ssh-keypair. Moreover , where should the field ShootSSHKeypairRotated be included ?

ShootSSHKeypairRotated : "true"

@rfranzke rfranzke self-requested a review February 14, 2022 08:12
rfranzke
rfranzke reviewed Feb 22, 2022
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Thanks, this looks much better now!
Should we also delete all the ssh-keypair.old secrets in seed and garden if their content is equal to the ssh-keypair, i.e., if they were never rotated? This would free up some space. OTOH, maybe it's not worth the effort given that once there was at least one SSH key rotation there will always be both ssh-keypair and ssh-keypair.old, so it's just a matter of time until all shoots have both.
cc @ialidzhikov

pkg/operation/botanist/secrets.go Outdated Show resolved Hide resolved
@ary1992
Copy link
Contributor Author

ary1992 commented Feb 25, 2022

Should we also delete all the ssh-keypair.old secrets in seed and garden if their content is equal to the ssh-keypair, i.e., if they were never rotated? This would free up some space.

By this you mean the ssh-keypair.old secrets which were created during the shoot creation process ? I don't think the content of ssh-keypair.old and ssh-keypair created during shoot creation are equal.

rfranzke
rfranzke reviewed Feb 25, 2022
View reviewed changes
pkg/operation/botanist/secrets.go Outdated Show resolved Hide resolved
pkg/operation/botanist/secrets.go Outdated Show resolved Hide resolved
pkg/operation/botanist/secrets.go Outdated Show resolved Hide resolved
@rfranzke rfranzke requested a review from plkokanov March 3, 2022 08:07
rfranzke
rfranzke previously approved these changes Mar 7, 2022
View reviewed changes
@rfranzke rfranzke requested a review from plkokanov March 7, 2022 14:25
plkokanov
plkokanov approved these changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@plkokanov plkokanov left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke merged commit 7f3f29b into gardener:master Mar 7, 2022
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
@ary1992
Prevent creation of ssh-keypair.old secret on Shoot creation. (gard…
3531ee1 
…ener#5388)

* Avoid creation of `ssh-keypair.old` secret on Shoot creation.

* Address PR feedback.

* Move logic for ssh-keypair.old secret creation to `rotateSSHKeypairSecrets` function.

* Handle `IsNotFound` error when `ssh-keypair.old` is not present in shoot namespace in seed.

* Minor modifications

* Address PR feedback from `@plkokanov`
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
@ary1992
Prevent creation of ssh-keypair.old secret on Shoot creation. (gard…
82f7069 
…ener#5388)

* Avoid creation of `ssh-keypair.old` secret on Shoot creation.

* Address PR feedback.

* Move logic for ssh-keypair.old secret creation to `rotateSSHKeypairSecrets` function.

* Handle `IsNotFound` error when `ssh-keypair.old` is not present in shoot namespace in seed.

* Minor modifications

* Address PR feedback from `@plkokanov`
@ary1992 ary1992 deleted the enh/Unused-ssh branch January 23, 2023 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfranzke rfranzke rfranzke left review comments

@plkokanov plkokanov plkokanov approved these changes

@acumino acumino Awaiting requested review from acumino

Assignees
No one assigned
Labels
area/auto-scaling Auto-scaling (CA/HPA/VPA/HVPA, predominantly control plane, but also otherwise) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/api-change API change with impact on API users kind/enhancement Enhancement, improvement, extension size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

gardenlet always creates (useless?) ssh-keypair.old Secret on Shoot creation/reconciliation
8 participants
@ary1992 @acumino @rfranzke @plkokanov @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot