Skip to content

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented Jun 23, 2023

How to categorize this PR?

/area security usability
/kind enhancement

What this PR does / why we need it:

This PR adds a mechanism for renewing all garden access secrets on a given seed by annotating it with gardener.cloud/operation=renew-garden-access-secrets.
ref #8032 (comment)

Which issue(s) this PR fixes:
Part of #8001

Special notes for your reviewer:

/cc @oliver-goetz @rfranzke

Release note:

`gardener.cloud/operation` annotation was introduced to `seeds`. This includes a verification of its value. Please check your `seeds` for this annotation and remove it if necessary prior to the update.

With this, we can renew the garden access secrets in all namespaces in the next commit without renewing shoot access secrets.

In the future, the shoot/garden controller should also restrict the call to the `shoot` class.
However, the controller in `gardener-resource-manager` is still not configured with the class, hence it is still responsible for all secrets.
This is fine for now since resource-manager and the call to `RenewAccessSecrets` are restricted to the `garden` or shoot namespace, but in the future we should make sure that we restrict it to the `shoot` class to prevent potential future responsibility overlaps.
@gardener-prow gardener-prow bot requested review from oliver-goetz and rfranzke June 23, 2023 10:46
@gardener-prow gardener-prow bot added area/security Security related area/usability Usability related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 23, 2023
Copy link
Member

@oliver-goetz oliver-goetz left a comment

Choose a reason for hiding this comment

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

awesome PR, thanks 🚀

@rfranzke
Copy link
Member

/assign

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.

Awesome PR :)

@timebertt
Copy link
Member Author

@rfranzke @oliver-goetz thanks for your reviews, PTAL :)

@rfranzke
Copy link
Member

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2023
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Jun 26, 2023

LGTM label has been added.

Git tree hash: 5c5b42f9bcc52f98383efe260642654d3b58482b

@oliver-goetz
Copy link
Member

/lgtm
/approve
/retest

@gardener-prow
Copy link
Contributor

gardener-prow bot commented Jun 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oliver-goetz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2023
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Jun 26, 2023

@timebertt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-gardener-apidiff 10c236b link false /test pull-gardener-apidiff

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@gardener-prow gardener-prow bot merged commit c5e46d5 into gardener:master Jun 26, 2023
@timebertt timebertt deleted the renew-garden-access-secrets branch June 26, 2023 08:26
andrerun pushed a commit to andrerun/gardener that referenced this pull request Jul 6, 2023
…ardener#8152)

* Nit: move the `tokenrequestor` controller docs out of the `shoot` controller section

* Add documentation for the new feature

* Validate operation annotation on `Seeds`

* Bump `Seed` generation when the operation annotation is set

* Allow selecting class in `RenewAccessSecrets`

With this, we can renew the garden access secrets in all namespaces in the next commit without renewing shoot access secrets.

In the future, the shoot/garden controller should also restrict the call to the `shoot` class.
However, the controller in `gardener-resource-manager` is still not configured with the class, hence it is still responsible for all secrets.
This is fine for now since resource-manager and the call to `RenewAccessSecrets` are restricted to the `garden` or shoot namespace, but in the future we should make sure that we restrict it to the `shoot` class to prevent potential future responsibility overlaps.

* Renew all garden access secrets on seed when triggered by annotation

* Verify garden access token renewal in e2e tests

* Address review suggestions
nickytd pushed a commit to nickytd/gardener that referenced this pull request Sep 11, 2023
…ardener#8152)

* Nit: move the `tokenrequestor` controller docs out of the `shoot` controller section

* Add documentation for the new feature

* Validate operation annotation on `Seeds`

* Bump `Seed` generation when the operation annotation is set

* Allow selecting class in `RenewAccessSecrets`

With this, we can renew the garden access secrets in all namespaces in the next commit without renewing shoot access secrets.

In the future, the shoot/garden controller should also restrict the call to the `shoot` class.
However, the controller in `gardener-resource-manager` is still not configured with the class, hence it is still responsible for all secrets.
This is fine for now since resource-manager and the call to `RenewAccessSecrets` are restricted to the `garden` or shoot namespace, but in the future we should make sure that we restrict it to the `shoot` class to prevent potential future responsibility overlaps.

* Renew all garden access secrets on seed when triggered by annotation

* Verify garden access token renewal in e2e tests

* Address review suggestions
rfranzke added a commit to rfranzke/gardener that referenced this pull request Nov 30, 2023
gardener-prow bot pushed a commit that referenced this pull request Dec 5, 2023
* Remove MCM legacy CRD deletion

follow-up of #8559, released with `v1.82.0`

* Remove legacy `shoot-node-logging` MR cleanup

follow-up of #8501, released with `v1.80.0`

* Remove MCM legacy resources cleanup in generic `Worker` actuator

follow-up of #8596, released with `v1.82.0`

* Restrict GRM's token requestor to secrets with `class=shoot`

follow-up of #8152, released with `v1.74.0`

* Remove support for deprecated `NetworkPolicy` annotations

follow-up of #7907, released with `v1.71.0`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/security Security related area/usability Usability related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants