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
Gardener Seed Admission Controller #2066
Conversation
...d-bootstrap/templates/gardener-seed-admission-controller/validatingwebhookconfiguration.yaml
Show resolved
Hide resolved
...d-bootstrap/templates/gardener-seed-admission-controller/validatingwebhookconfiguration.yaml
Show resolved
Hide resolved
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.
Thanks for the PR, Rafael!
Looks pretty good, though I have some comments here and there (mainly regarding the protected CRs/CRDs and the unit tests).
Also wanna give it a test in my local setup tomorrow, will come back with the results after that :)
charts/seed-bootstrap/charts/fluentd-es/templates/fluent-bit-configmap.yaml
Show resolved
Hide resolved
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Show resolved
Hide resolved
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.
Looking good. Will run a more thorough test soon.
charts/seed-bootstrap/templates/gardener-seed-admission-controller/rbac.yaml
Show resolved
Hide resolved
b0c1cd5
to
4c03cc4
Compare
4c03cc4
to
bc0c500
Compare
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Show resolved
Hide resolved
example/seed-admission-controller/10-validatingwebhookconfiguration.yaml
Outdated
Show resolved
Hide resolved
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.
Finished testing now, works completely fine for me.
I have only one more suggestion on how to handle the admission response.
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
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.
Sorry, found one more bug.
...d-bootstrap/templates/gardener-seed-admission-controller/validatingwebhookconfiguration.yaml
Outdated
Show resolved
Hide resolved
bc0c500
to
2b78b49
Compare
Wow, thanks for your thorough and intensive review @tim-ebert, kudos! I'm impressed! I hope I've addressed everything now - can you please check it again? |
You're welcome. Will take another look on Monday :) |
2b78b49
to
9a879af
Compare
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.
Thanks for addressing my comments.
I only have some smaller comments left now.
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
cmd/gardener-seed-admission-controller/app/gardener_seed_admission_controller.go
Outdated
Show resolved
Hide resolved
9a879af
to
ac6f7df
Compare
ac6f7df
to
ea5df84
Compare
ea5df84
to
1a1c326
Compare
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.
Very nice, looks perfect now :)
Gave it a spin in my local setup again, runs as expected!
/approve
Let's wait with the merge of this PR until v1.2 has been cut. |
What this PR does / why we need it:
This PR introduces a new component, the Gardener Seed admission controller. It is deployed by the Gardenlet as part of its seed bootstrapping phase and, consequently, running in every seed cluster. It's main purpose is to serve webhooks (validating or mutating) in order to admit or deny certain requests to the seed's API server.
Concretely, it prevents the undesired deletion of
CustomResourceDefinitions
labeled withgardener.cloud/deletion-protected=true
, and most custom resources of theextensions.gardener.cloud/v1alpha1
API group if they were not previously annotated withconfirmation.gardener.cloud/deletion=true
.Example logs:
Which issue(s) this PR fixes:
Fixes #2053
Special notes for your reviewer:
The PR is split in three commits: The first commit introduces the gardener-seed-admission-controller, the second commit enhances the gardenlet code to a) deploy it as part of the seed bootstrap, and b) confirm the deletion for the extension resources prior to sending
DELETE
calls. The third commit adds a script for local development that adds an image vector overwrite for the Gardener components in thecharts/images.yaml
.PS: It might happen that, when you check out this PR and start the Gardenlet locally, the gardener-seed-admission-controller cannot be deployed in your seeds because of
ImagePullBackOff
. This is because the Gardener CI pipeline has not yet been adapted to automatically build an image for this new component. This can only happen after the PR is merged. You can use the imageeu.gcr.io/gardener-project/gardener/seed-admission-controller:v1.2.0-dev
for testing the admission controller.Release note: