Conversation
On-behalf-of: @SAP krzysztof.zagorski@sap.com
There was a problem hiding this comment.
Pull request overview
Adds cross-cluster reconciliation for Shoot OIDC configuration by labeling referenced Greenhouse AuthenticationConfiguration ConfigMaps and watching them for changes, so matching Shoots are re-enqueued when the auth ConfigMap updates.
Changes:
- Label Greenhouse auth ConfigMaps on first interaction with
AuthConfigMapLabelandCareInstructionLabel. - Add a Greenhouse-cache-backed watch to the Shoot controller to re-enqueue matching Shoots on auth ConfigMap changes.
- Extend tests and update README documentation for the new labeling + watch behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| controller/shoot/shoot_controller.go | Adds Greenhouse ConfigMap watch and re-enqueue mapping for Shoots. |
| controller/shoot/auth.go | Adds CareInstruction ownership labeling to the referenced Greenhouse auth ConfigMap. |
| controller/careinstruction/careinstruction_controller.go | Passes the Greenhouse manager into ShootController so it can watch Greenhouse resources. |
| controller/shoot/shoot_controller_test.go | Starts the Greenhouse manager in tests and adds watch-triggered reconciliation coverage. |
| README.md | Documents auth ConfigMap labeling + watch behavior and updates related references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds cross-cluster watch support so changes to a labeled Greenhouse AuthenticationConfiguration ConfigMap trigger immediate re-reconciliation of affected Shoots, keeping Garden-side OIDC configuration up to date. It also ensures the referenced auth ConfigMap is labeled on first interaction to make the watch predicate work.
Changes:
- Label Greenhouse auth ConfigMaps with
authconfigmap=trueand the owningcareinstruction=<name>on first use (via merge-patch). - Add a Greenhouse-cache-backed watch in the Shoot controller to re-enqueue matching Shoots on auth ConfigMap changes.
- Extend controller tests and update README documentation for the labeling/watch behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| controller/shoot/shoot_controller.go | Adds Greenhouse ConfigMap watch + enqueue mapping for watch-triggered reconciles. |
| controller/shoot/auth.go | Switches to merge-patching labels and adds CareInstruction ownership label logic. |
| controller/careinstruction/careinstruction_controller.go | Passes Greenhouse manager into ShootController to enable cross-cluster watches. |
| controller/shoot/shoot_controller_test.go | Starts Greenhouse manager/cache for tests and adds watch-triggered reconciliation coverage. |
| api/v1alpha1/careinstruction_types.go | Unifies auth ConfigMap label key under shoot-grafter.cloudoperators.dev. |
| README.md | Documents labeling + watch behavior and updates OIDC doc link. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR improves OIDC AuthenticationConfiguration synchronization by labeling the referenced Greenhouse auth ConfigMap on first interaction and adding a Greenhouse-cache-backed watch that re-enqueues matching Shoots when that ConfigMap changes. It also standardizes the auth ConfigMap label key under the shoot-grafter.cloudoperators.dev prefix and updates docs/tests accordingly.
Changes:
- Add labeling of Greenhouse AuthenticationConfiguration ConfigMaps with
auth-configmap=trueand the owningcareinstructionlabel (using a merge patch). - Add a Greenhouse ConfigMap watch (via Greenhouse manager cache) to trigger immediate re-reconciliation of matching Shoots on auth CM updates.
- Update label key constant + README and extend tests to cover labeling and watch-triggered reconciliation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| controller/shoot/shoot_controller.go | Adds cross-cluster ConfigMap watch and re-enqueue mapping for matching Shoots. |
| controller/shoot/auth.go | Switches to merge-patching labels and adds CareInstruction ownership labeling with conflict handling. |
| controller/shoot/shoot_controller_test.go | Starts/syncs Greenhouse manager cache and adds tests for labeling + watch-triggered reconciliation. |
| controller/careinstruction/careinstruction_controller.go | Wires Greenhouse manager into ShootController for cross-cluster watches. |
| api/v1alpha1/careinstruction_types.go | Updates the auth ConfigMap label key constant to the new .dev/auth-configmap value. |
| README.md | Documents new labeling/watch behavior and updates OIDC doc link + field description. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nager context in tests On-behalf-of: @SAP krzysztof.zagorski@sap.com
There was a problem hiding this comment.
Pull request overview
This PR extends the Shoot controller to keep Garden-side OIDC authentication configuration in sync with a referenced Greenhouse AuthenticationConfiguration ConfigMap by (1) labeling the ConfigMap on first interaction and (2) reconciling Shoots when that labeled ConfigMap changes.
Changes:
- Add labeling of the referenced Greenhouse auth ConfigMap with both an “auth CM” label and an owning CareInstruction label.
- Add a cross-cluster watch (via the Greenhouse manager cache) that re-enqueues all matching Shoots when the labeled auth ConfigMap changes.
- Update the auth ConfigMap label key constant, tests, and README documentation accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| controller/shoot/shoot_controller.go | Adds Greenhouse cache-backed ConfigMap watch and re-enqueue mapping for auth ConfigMap changes. |
| controller/shoot/auth.go | Patches Greenhouse auth ConfigMap labels (auth + careinstruction) without overwriting data. |
| controller/careinstruction/careinstruction_controller.go | Passes the Greenhouse manager into the ShootController for cross-cluster watches. |
| api/v1alpha1/careinstruction_types.go | Renames the auth ConfigMap label key to the .dev/auth-configmap form. |
| controller/shoot/shoot_controller_test.go | Starts/syncs the Greenhouse manager cache in tests and adds watch-triggered reconciliation coverage. |
| README.md | Documents labeling + watch behavior and updates the referenced Greenhouse docs link. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
| // GreenhouseMgr is passed so the ShootController can watch Greenhouse cluster resources (e.g. auth CMs). | ||
| sc := &shoot.ShootController{ | ||
| GreenhouseClient: r.Client, | ||
| GreenhouseMgr: r.Manager, |
There was a problem hiding this comment.
I am not super convinced that watching resources on two different Clusters is the way to go here.
I would opt to actually watch the CM changes in the CareInstruction controller for separation of concerns.
That would involve:
- Adding a watch on the auth CM
- Maintaining in-memory status of CM for shoot-controller restart
Looping in @abhijith-darshan for his opinion
This PR adds two related behaviours:
1. Auth CM labeling on first interaction When
authenticationConfigMapNameis configured, the shoot controller now labels the referenced Greenhouse ConfigMap with:shoot-grafter.cloudoperators.dev/auth-configmap: "true"— marks it as an auth CMshoot-grafter.cloudoperators.dev/careinstruction: <ci-name>— associates it with the owning CareInstruction2. Watch-triggered reconciliation The shoot controller now watches labeled auth ConfigMaps on the Greenhouse cluster using
source.Kindwith the Greenhouse manager's cache. When an auth CM changes, all matching Shoots are immediately re-enqueued, keeping the Garden-side OIDC configuration in sync.Added tests and updated docs.
The authentication ConfigMap label key is changed from
shoot-grafter.cloudoperators/authconfigmaptoshoot-grafter.cloudoperators.dev/auth-configmap. The new label will be added automatically on shoot reconciliation.