From d251e6fa2a0d46ad5a9f73d5abfd87946101f840 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Fri, 24 Apr 2026 11:23:17 +0200 Subject: [PATCH 01/20] feat: label auth CMs on first interaction and reconcile Shoots on Greenhouse auth CM changes On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .gitignore | 1 + .../careinstruction_controller.go | 4 +- controller/shoot/auth.go | 13 ++++- controller/shoot/shoot_controller.go | 50 +++++++++++++++++-- 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index a615880d..efa593e2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .vscode +/bin/ /build/ diff --git a/controller/careinstruction/careinstruction_controller.go b/controller/careinstruction/careinstruction_controller.go index e5696055..e7e8dada 100644 --- a/controller/careinstruction/careinstruction_controller.go +++ b/controller/careinstruction/careinstruction_controller.go @@ -288,9 +288,11 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn } // Register the ShootController with the garden manager - // Note: EventRecorder is obtained from the Greenhouse manager to emit events on the Greenhouse cluster + // Note: EventRecorder is obtained from the Greenhouse manager to emit events on the Greenhouse cluster. + // GreenhouseMgr is passed so the ShootController can watch Greenhouse cluster resources (e.g. auth CMs). sc := &shoot.ShootController{ GreenhouseClient: r.Client, + GreenhouseMgr: r.Manager, GardenClient: gardenClient, Logger: r.WithValues("careInstruction", careInstruction.Name), Name: shoot.GenerateName(careInstruction.Name), diff --git a/controller/shoot/auth.go b/controller/shoot/auth.go index f2d25393..8ef2fa10 100644 --- a/controller/shoot/auth.go +++ b/controller/shoot/auth.go @@ -34,14 +34,23 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot r.CareInstruction.Spec.AuthenticationConfigMapName, err) } - // Add the auth ConfigMap label if it doesn't exist + // Add labels if missing: AuthConfigMapLabel marks CMs as auth config maps, + // CareInstructionLabel associates the CM with the owning CareInstruction. if greenhouseAuthConfigMap.Labels == nil { greenhouseAuthConfigMap.Labels = make(map[string]string) } + labelsNeedUpdate := false if _, hasLabel := greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel]; !hasLabel { greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel] = "true" + labelsNeedUpdate = true + } + if val, hasLabel := greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel]; !hasLabel || val != r.CareInstruction.Name { + greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] = r.CareInstruction.Name + labelsNeedUpdate = true + } + if labelsNeedUpdate { if err := r.GreenhouseClient.Update(ctx, &greenhouseAuthConfigMap); err != nil { - r.Info("failed to add auth ConfigMap label", "configMap", greenhouseAuthConfigMap.Name, "error", err) + r.Info("failed to add labels to auth ConfigMap", "configMap", greenhouseAuthConfigMap.Name, "error", err) // Don't fail the reconciliation for this, just log it } } diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index bd071a49..6dfd1163 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -26,7 +26,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" ) const ( @@ -36,6 +39,7 @@ const ( type ShootController struct { GreenhouseClient client.Client + GreenhouseMgr ctrl.Manager // GreenhouseMgr provides the Greenhouse cluster cache for watches GardenClient client.Client logr.Logger Name string @@ -75,11 +79,49 @@ func (r *ShootController) SetupWithManager(mgr ctrl.Manager) error { r.Error(nil, "EventRecorder is not set for ShootController", "name", r.Name) } - // Setup the shoot controller with the manager - return ctrl.NewControllerManagedBy(mgr). + b := ctrl.NewControllerManagedBy(mgr). Named(r.Name). - For(&gardenerv1beta1.Shoot{}, builder.WithPredicates(predicates...)). - Complete(r) + For(&gardenerv1beta1.Shoot{}, builder.WithPredicates(predicates...)) + + // Watch AuthenticationConfiguration ConfigMaps on the Greenhouse cluster. + // When they change, all Shoots managed by this CareInstruction are re-enqueued + // so OIDC config stays in sync with the Greenhouse source. + // The predicate restricts events to CMs that carry both the auth-configmap marker + // and the CareInstruction ownership label matching this controller instance. + if r.CareInstruction.Spec.AuthenticationConfigMapName != "" && r.GreenhouseMgr != nil { + authCMPredicate := predicate.NewTypedPredicateFuncs(func(cm *corev1.ConfigMap) bool { + labels := cm.GetLabels() + return labels[v1alpha1.AuthConfigMapLabel] == "true" && + labels[v1alpha1.CareInstructionLabel] == r.CareInstruction.Name + }) + b = b.WatchesRawSource(source.Kind( + r.GreenhouseMgr.GetCache(), + &corev1.ConfigMap{}, + handler.TypedEnqueueRequestsFromMapFunc(r.enqueueShootsForAuthConfigMap), + authCMPredicate, + )) + } + + // Setup the shoot controller with the manager + return b.Complete(r) +} + +func (r *ShootController) enqueueShootsForAuthConfigMap(ctx context.Context, _ *corev1.ConfigMap) []reconcile.Request { + shoots, err := r.CareInstruction.ListShoots(ctx, r.GardenClient) + if err != nil { + r.Info("failed to list shoots for auth ConfigMap change", "error", err) + return nil + } + requests := make([]ctrl.Request, 0, len(shoots.Items)) + for _, shoot := range shoots.Items { + requests = append(requests, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: shoot.Namespace, + Name: shoot.Name, + }, + }) + } + return requests } func (r *ShootController) newCELPredicate() predicate.Predicate { From 3748d84b4da9c01ce556e91c796753a16a089094 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Fri, 24 Apr 2026 14:21:21 +0200 Subject: [PATCH 02/20] add integration tests for auth CM labeling and Greenhouse CM watch-triggered reconciliation On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller_test.go | 200 ++++++++++++++++++++-- 1 file changed, 190 insertions(+), 10 deletions(-) diff --git a/controller/shoot/shoot_controller_test.go b/controller/shoot/shoot_controller_test.go index 61378eac..f1036f1a 100644 --- a/controller/shoot/shoot_controller_test.go +++ b/controller/shoot/shoot_controller_test.go @@ -30,9 +30,10 @@ import ( ) var ( - careInstruction *v1alpha1.CareInstruction - mgrCtx context.Context - mgrCancel context.CancelFunc + careInstruction *v1alpha1.CareInstruction + mgrCtx context.Context + mgrCancel context.CancelFunc + greenhouseMgrCancel context.CancelFunc ) var _ = Describe("Shoot Controller", func() { JustBeforeEach(func() { @@ -73,10 +74,13 @@ var _ = Describe("Shoot Controller", func() { }) Expect(err).NotTo(HaveOccurred(), "there must be no error creating the greenhouse manager") - // Create ShootController with EventRecorder from Greenhouse manager + // Create ShootController with EventRecorder and GreenhouseMgr from the Greenhouse manager. + // GreenhouseMgr is required so the ShootController can watch Greenhouse auth CMs and + // re-enqueue Shoots when they change. Expect(err).NotTo(HaveOccurred(), "there must be no error creating the manager") Expect((&shoot.ShootController{ GreenhouseClient: test.K8sClient, + GreenhouseMgr: greenhouseMgr, // Provide Greenhouse manager for cross-cluster CM watch GardenClient: test.GardenK8sClient, Logger: ctrl.Log.WithName("controllers").WithName("ShootController"), Name: "ShootController", @@ -88,7 +92,16 @@ var _ = Describe("Shoot Controller", func() { Expect(careInstructionWebhook.SetupWebhookWithManager(mgr)).To(Succeed(), "there must be no error setting up the webhook with the manager") mgrCtx, mgrCancel = context.WithCancel(test.Ctx) - // start the manager + + // start the Greenhouse manager so its cache is populated for the auth CM watch + ghCtx, ghCancel := context.WithCancel(test.Ctx) + greenhouseMgrCancel = ghCancel + go func() { + defer GinkgoRecover() + Expect(greenhouseMgr.Start(ghCtx)).To(Succeed(), "there must be no error starting the greenhouse manager") + }() + + // start the garden manager go func() { defer GinkgoRecover() Expect(mgr.Start(mgrCtx)).To(Succeed(), "there must be no error starting the manager") @@ -191,8 +204,12 @@ var _ = Describe("Shoot Controller", func() { return len(events.Items) == 0 }).Should(BeTrue(), "should eventually not find Event resources") - // stop the manager + // stop both managers mgrCancel() + if greenhouseMgrCancel != nil { + greenhouseMgrCancel() + greenhouseMgrCancel = nil + } }) @@ -1734,7 +1751,7 @@ jwt: } Expect(test.GardenK8sClient.Create(test.Ctx, cm)).To(Succeed(), "should create CA ConfigMap") - // Eventually verify the label was added by the controller + // Eventually verify both labels were added by the controller Eventually(func(g Gomega) bool { var updatedConfigMap corev1.ConfigMap err := test.K8sClient.Get(test.Ctx, client.ObjectKey{ @@ -1744,10 +1761,173 @@ jwt: if err != nil { return false } - // Verify the label was added - g.Expect(updatedConfigMap.Labels).To(HaveKeyWithValue(v1alpha1.AuthConfigMapLabel, "true")) + // Verify AuthConfigMapLabel was added to enable the watch predicate + g.Expect(updatedConfigMap.Labels).To(HaveKeyWithValue(v1alpha1.AuthConfigMapLabel, "true"), + "controller should add auth-configmap label so the watch predicate can match") + // Verify CareInstructionLabel was added to associate the CM with its owning CareInstruction + g.Expect(updatedConfigMap.Labels).To(HaveKeyWithValue(v1alpha1.CareInstructionLabel, careInstruction.Name), + "controller should add careinstruction label to identify the owning CareInstruction") return true - }).Should(BeTrue(), "controller should add auth ConfigMap label when not initially present") + }).Should(BeTrue(), "controller should add both auth and careinstruction labels when not initially present") + }) + }) + + When("testing OIDC configuration watch triggering reconciliation", func() { + var greenhouseAuthConfigMap *corev1.ConfigMap + + BeforeEach(func() { + careInstruction = &v1alpha1.CareInstruction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-careinstruction-watch", + Namespace: "default", + }, + Spec: v1alpha1.CareInstructionSpec{ + ShootSelector: &v1alpha1.ShootSelector{ + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "test": "watch", + }, + }, + }, + AuthenticationConfigMapName: "greenhouse-auth-config-watch", + }, + } + + // Create the Greenhouse AuthenticationConfiguration ConfigMap with initial OIDC config. + // The watch predicate requires both AuthConfigMapLabel and CareInstructionLabel. + // The controller will add CareInstructionLabel on first reconciliation; we pre-set + // AuthConfigMapLabel here so the CM is already identifiable as an auth CM. + greenhouseAuthConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "greenhouse-auth-config-watch", + Namespace: "default", + Labels: map[string]string{ + v1alpha1.AuthConfigMapLabel: "true", + }, + }, + Data: map[string]string{ + "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://greenhouse-watch.test.example.com + audiences: + - audience-v1 + claimMappings: + username: + claim: sub + prefix: 'greenhouse-v1:' +`, + }, + } + Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthConfigMap)).To(Succeed(), "should create Greenhouse auth ConfigMap") + }) + + It("should re-reconcile shoots when Greenhouse auth CM is updated", func() { + // Create a shoot that matches the selector + shoot := &gardenerv1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot-watch", + Namespace: "default", + Labels: map[string]string{ + "test": "watch", + }, + }, + } + Expect(test.GardenK8sClient.Create(test.Ctx, shoot)).To(Succeed(), "should create Shoot resource") + + shoot.Status = gardenerv1beta1.ShootStatus{ + AdvertisedAddresses: []gardenerv1beta1.ShootAdvertisedAddress{ + { + Name: "external", + URL: "https://api-server.test-shoot-watch.example.com", + }, + }, + } + Expect(test.GardenK8sClient.Status().Update(test.Ctx, shoot)).To(Succeed(), "should update Shoot status") + + // Create CA ConfigMap + caCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot-watch.ca-cluster", + Namespace: "default", + }, + Data: map[string]string{"ca.crt": "test-ca-data"}, + } + Expect(test.GardenK8sClient.Create(test.Ctx, caCM)).To(Succeed(), "should create CA ConfigMap") + + gardenAuthCMName := "test-careinstruction-watch-greenhouse-auth" + + // Step 1: wait for the initial Garden auth CM to be created with the v1 audience + Eventually(func(g Gomega) { + authCM := &corev1.ConfigMap{} + g.Expect(test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ + Name: gardenAuthCMName, + Namespace: "default", + }, authCM)).To(Succeed(), "should find Garden auth ConfigMap") + + var authConfig apiserverv1beta1.AuthenticationConfiguration + g.Expect(yaml.Unmarshal([]byte(authCM.Data["config.yaml"]), &authConfig)).To(Succeed()) + g.Expect(authConfig.JWT).To(HaveLen(1)) + g.Expect(authConfig.JWT[0].Issuer.URL).To(Equal("https://greenhouse-watch.test.example.com")) + g.Expect(authConfig.JWT[0].Issuer.Audiences).To(ConsistOf("audience-v1")) + }).Should(Succeed(), "initial Garden auth CM should contain v1 audience") + + // Step 2: wait for the controller to label the Greenhouse auth CM. + // This adds CareInstructionLabel, enabling the watch predicate to match future updates. + Eventually(func(g Gomega) { + var ghCM corev1.ConfigMap + g.Expect(test.K8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "greenhouse-auth-config-watch", + Namespace: "default", + }, &ghCM)).To(Succeed()) + g.Expect(ghCM.Labels).To(HaveKeyWithValue(v1alpha1.CareInstructionLabel, careInstruction.Name), + "controller should have added CareInstructionLabel to Greenhouse auth CM") + }).Should(Succeed(), "Greenhouse auth CM should be labeled with CareInstructionLabel before proceeding") + + // Step 3: update the Greenhouse auth CM — same issuer URL, but new audience and prefix. + // Keeping the same URL ensures mergeAuthenticationConfigurations replaces the existing + // entry in-place rather than appending a second issuer. The watch (source.Kind on the + // Greenhouse cache) fires because the CM has both required labels and its content changed. + Expect(test.K8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "greenhouse-auth-config-watch", + Namespace: "default", + }, greenhouseAuthConfigMap)).To(Succeed(), "should fetch latest Greenhouse auth CM") + greenhouseAuthConfigMap.Data["config.yaml"] = `apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://greenhouse-watch.test.example.com + audiences: + - audience-v2 + claimMappings: + username: + claim: sub + prefix: 'greenhouse-v2:' +` + Expect(test.K8sClient.Update(test.Ctx, greenhouseAuthConfigMap)).To(Succeed(), + "should update Greenhouse auth CM to trigger watch-based reconciliation") + + // Step 4: the watch fires -> shoots are re-enqueued -> controller reconciles -> + // Garden auth CM is updated in-place to reflect the new audience (v2). + // The issuer count stays at 1 because the URL matches and the entry is replaced. + Eventually(func(g Gomega) { + authCM := &corev1.ConfigMap{} + g.Expect(test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ + Name: gardenAuthCMName, + Namespace: "default", + }, authCM)).To(Succeed()) + + var authConfig apiserverv1beta1.AuthenticationConfiguration + g.Expect(yaml.Unmarshal([]byte(authCM.Data["config.yaml"]), &authConfig)).To(Succeed()) + g.Expect(authConfig.JWT).To(HaveLen(1), "should still have exactly one issuer (replaced in-place)") + g.Expect(authConfig.JWT[0].Issuer.URL).To(Equal("https://greenhouse-watch.test.example.com")) + // audience-v2 proves the Garden CM was re-reconciled from the updated Greenhouse CM + g.Expect(authConfig.JWT[0].Issuer.Audiences).To(ConsistOf("audience-v2"), + "Garden auth CM should have updated audience after Greenhouse CM change triggers watch") + g.Expect(authConfig.JWT[0].ClaimMappings.Username.Prefix).NotTo(BeNil()) + g.Expect(*authConfig.JWT[0].ClaimMappings.Username.Prefix).To(Equal("greenhouse-v2:")) + }).Should(Succeed(), "Garden auth CM should be re-reconciled to v2 config after Greenhouse CM change triggers watch") }) }) From 3a213d69eb07beb4e194fda530329f59b66f9b09 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Fri, 24 Apr 2026 14:24:21 +0200 Subject: [PATCH 03/20] add note on auth CM labeling and watch-triggered shoot reconciliation On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4888a970..f6fdf700 100644 --- a/README.md +++ b/README.md @@ -145,9 +145,11 @@ For each CareInstruction, a dedicated Shoot controller is dynamically created an - Extracts cluster connection details (API server URL, CA certificate) - Creates or updates corresponding Secret resources with OIDC configuration - Generates Greenhouse Cluster resources with appropriate labels -- Optionally configures OIDC authentication on Shoot clusters for Greenhouse access. Also see respective [Greenhouse docs](https://cloudoperators.github.io/greenhouse/docs/user-guides/cluster/oidc_connectivity/) and [Gardener docs](https://gardener.cloud/docs/guides/administer-shoots/oidc-login/#configure-the-shoot-cluster) +- Optionally configures OIDC authentication on Shoot clusters for Greenhouse access. Also see respective [Greenhouse docs](https://cloudoperators.github.io/greenhouse/docs/user-guides/cluster/oidc-login/) and [Gardener docs](https://gardener.cloud/docs/guides/administer-shoots/oidc-login/#configure-the-shoot-cluster) - Optionally configures RBAC on the Shoot cluster for Greenhouse access +> **Auth ConfigMap labeling & watch**: When `authenticationConfigMapName` is set, the shoot controller labels the referenced Greenhouse ConfigMap with `shoot-grafter.cloudoperators.dev/auth-configmap: "true"` and `shoot-grafter.cloudoperators.dev/careinstruction: ` on first interaction (creation or update). The controller also watches these labeled ConfigMaps on the Greenhouse cluster; any change to the auth ConfigMap automatically re-enqueues all matching Shoots so the Garden-side OIDC configuration stays in sync without waiting for the next Shoot event. + ## Custom Resource: CareInstruction A `CareInstruction` defines the configuration for onboarding Shoots from a specific Garden cluster. @@ -208,7 +210,7 @@ spec: | `shootSelector.expression` | string | No | CEL expression for filtering shoots by status or other fields (max 1024 chars). The shoot object is available as `object` | | `propagateLabels` | []string | No | List of label keys to copy from Shoot to Greenhouse Cluster | | `additionalLabels` | map[string]string | No | Additional labels to add to all created Greenhouse Clusters | -| `authenticationConfigMapName` | string | No | Name of ConfigMap in Greenhouse cluster containing AuthenticationConfiguration [(config.yaml with apiserver.config.k8s.io/v1beta1 content)](https://gardener.cloud/docs/guides/administer-shoots/oidc-login/#configure-the-shoot-cluster)| +| `authenticationConfigMapName` | string | No | Name of ConfigMap in Greenhouse cluster containing AuthenticationConfiguration [(config.yaml with apiserver.config.k8s.io/v1beta1 content)](https://gardener.cloud/docs/guides/administer-shoots/oidc-login/#configure-the-shoot-cluster). The ConfigMap is labeled by the shoot controller on first interaction and watched for changes to trigger automatic re-reconciliation of Shoots. | | `enableRBAC` | bool | No | When false, skips automatic RBAC setup on Shoot clusters (default: true‚) | *Note: Either `gardenClusterName` or `gardenClusterKubeConfigSecretName` must be provided (priority: kubeconfig secret > cluster name) From f9ac1296d249f781f2775a8c08a463912a77c97e Mon Sep 17 00:00:00 2001 From: Krzysztof Zagorski Date: Fri, 24 Apr 2026 14:17:28 +0200 Subject: [PATCH 04/20] feat: trigger shoot reconciliation after OIDC updates (#44) * feat: trigger Shoot reconciliation on OIDC ConfigMap updates On-behalf-of: @SAP krzysztof.zagorski@sap.com * simplify the conditions for triggering shoot reconciliation On-behalf-of: @SAP krzysztof.zagorski@sap.com * update docs On-behalf-of: @SAP krzysztof.zagorski@sap.com * add bin to gitignore, remove invalid G118 from gosec excludes, regenerate files On-behalf-of: @SAP krzysztof.zagorski@sap.com * revert golangci config due to mismatch between CI and local version On-behalf-of: @SAP krzysztof.zagorski@sap.com * update go to 1.26.0 On-behalf-of: @SAP krzysztof.zagorski@sap.com * update project with go-makefile-maker On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .github/renovate.json | 2 +- .github/workflows/checks.yaml | 4 +- .github/workflows/ci.yaml | 10 +- .github/workflows/codeql.yaml | 2 +- .../workflows/container-registry-ghcr.yaml | 10 +- .license-scan-overrides.jsonl | 1 + Makefile | 2 +- README.md | 13 + api/v1alpha1/zz_generated.deepcopy.go | 2 - controller/shoot/auth.go | 18 ++ controller/shoot/auth_test.go | 17 +- controller/shoot/shoot_controller_test.go | 268 ++++++++++++++++++ go.mod | 2 +- shell.nix | 2 +- 14 files changed, 323 insertions(+), 30 deletions(-) diff --git a/.github/renovate.json b/.github/renovate.json index 3e7e3311..c804c1a3 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -11,7 +11,7 @@ ], "commitMessageAction": "Renovate: Update", "constraints": { - "go": "1.25" + "go": "1.26" }, "dependencyDashboardOSVVulnerabilitySummary": "all", "osvVulnerabilityAlerts": true, diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 0d69c582..cce04ad3 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -29,13 +29,13 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.8 + go-version: 1.26.2 - name: Run golangci-lint uses: golangci/golangci-lint-action@v9 with: version: latest - name: Delete pre-installed shellcheck - run: sudo rm -f $(which shellcheck) + run: sudo rm -f "$(which shellcheck)" - name: Run shellcheck run: make run-shellcheck - name: Dependency Licenses Review diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3a6300c0..2f027c66 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -32,12 +32,12 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.8 + go-version: 1.26.2 - name: Build all binaries run: make build-all code_coverage: name: Code coverage report - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository needs: - test runs-on: ubuntu-latest @@ -45,7 +45,7 @@ jobs: - name: Check out code uses: actions/checkout@v6 - name: Post coverage report - uses: fgrosse/go-coverage-report@v1.2.0 + uses: fgrosse/go-coverage-report@v1.3.0 with: coverage-artifact-name: code-coverage coverage-file-name: cover.out @@ -65,11 +65,11 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.8 + go-version: 1.26.2 - name: Run tests and generate coverage report run: make test-with-envtest - name: Archive code coverage results - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@v7 with: name: code-coverage path: build/cover.out diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml index c04019ec..b0bb5d38 100644 --- a/.github/workflows/codeql.yaml +++ b/.github/workflows/codeql.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.8 + go-version: 1.26.2 - name: Initialize CodeQL uses: github/codeql-action/init@v4 with: diff --git a/.github/workflows/container-registry-ghcr.yaml b/.github/workflows/container-registry-ghcr.yaml index 55152eba..bc5f92a2 100644 --- a/.github/workflows/container-registry-ghcr.yaml +++ b/.github/workflows/container-registry-ghcr.yaml @@ -25,14 +25,14 @@ jobs: - name: Check out code uses: actions/checkout@v6 - name: Log in to the Container registry - uses: docker/login-action@v3 + uses: docker/login-action@v4 with: password: ${{ secrets.GITHUB_TOKEN }} registry: ghcr.io username: ${{ github.actor }} - name: Extract metadata (tags, labels) for Docker id: meta - uses: docker/metadata-action@v5 + uses: docker/metadata-action@v6 with: images: ghcr.io/${{ github.repository }} tags: | @@ -45,11 +45,11 @@ jobs: # https://github.com/docker/metadata-action#typesha type=sha,format=long - name: Set up QEMU - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@v4 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@v4 - name: Build and push Docker image - uses: docker/build-push-action@v6 + uses: docker/build-push-action@v7 with: context: . labels: ${{ steps.meta.outputs.labels }} diff --git a/.license-scan-overrides.jsonl b/.license-scan-overrides.jsonl index 2e279c10..faa62bcb 100644 --- a/.license-scan-overrides.jsonl +++ b/.license-scan-overrides.jsonl @@ -6,6 +6,7 @@ {"name": "github.com/mattn/go-localereader", "licenceType": "MIT"} {"name": "github.com/miekg/dns", "licenceType": "BSD-3-Clause"} {"name": "github.com/pashagolub/pgxmock/v4", "licenceType": "BSD-3-Clause"} +{"name": "github.com/pashagolub/pgxmock/v5", "licenceType": "BSD-3-Clause"} {"name": "github.com/spdx/tools-golang", "licenceTextOverrideFile": "vendor/github.com/spdx/tools-golang/LICENSE.code"} {"name": "github.com/xeipuuv/gojsonpointer", "licenceType": "Apache-2.0"} {"name": "github.com/xeipuuv/gojsonreference", "licenceType": "Apache-2.0"} diff --git a/Makefile b/Makefile index 52cd1c74..66945885 100644 --- a/Makefile +++ b/Makefile @@ -145,7 +145,7 @@ license-headers: FORCE install-addlicense install-reuse @printf "\e[1;36m>> addlicense (for license headers on source code files)\e[0m\n" @printf "%s\0" $(patsubst $(shell awk '$$1 == "module" {print $$2}' go.mod)%,.%/*.go,$(shell go list ./...)) | $(XARGS) -0 -I{} bash -c 'year="$$(grep 'Copyright' {} | head -n1 | grep -E -o '"'"'[0-9]{4}(-[0-9]{4})?'"'"')"; if [[ -z "$$year" ]]; then year=$$(date +%Y); fi; gawk -i inplace '"'"'{if (display) {print} else {!/^\/\*/ && !/^\*/}}; {if (!display && $$0 ~ /^(package |$$)/) {display=1} else { }}'"'"' {}; addlicense -c "SAP SE or an SAP affiliate company and Greenhouse contributors" -s=only -y "$$year" -- {}; $(SED) -i '"'"'1s+// Copyright +// SPDX-FileCopyrightText: +'"'"' {}; ' @printf "\e[1;36m>> reuse annotate (for license headers on other files)\e[0m\n" - @reuse lint -j | jq -r '.non_compliant.missing_licensing_info[]' | grep -vw vendor | $(XARGS) reuse annotate -c 'SAP SE or an SAP affiliate company and Greenhouse contributors' -l Apache-2.0 --skip-unrecognised + @reuse lint -j | jq -r '.non_compliant.missing_licensing_info[]' | sed '/\/d' | $(XARGS) reuse annotate -c 'SAP SE or an SAP affiliate company and Greenhouse contributors' -l Apache-2.0 --skip-unrecognised @printf "\e[1;36m>> reuse download --all\e[0m\n" @reuse download --all @printf "\e[1;35mPlease review the changes. If *.license files were generated, consider instructing go-makefile-maker to add overrides to REUSE.toml instead.\e[0m\n" diff --git a/README.md b/README.md index f6fdf700..6da15b07 100644 --- a/README.md +++ b/README.md @@ -387,6 +387,19 @@ data: prefix: "greenhouse:" ``` +### OIDC Configuration Updates and Shoot Reconciliation + +When `spec.authenticationConfigMapName` is configured in a CareInstruction, shoot-grafter automatically manages OIDC authentication on Shoots: + +1. **Initial Setup**: When a Shoot is first onboarded, shoot-grafter creates an AuthenticationConfiguration ConfigMap in the Garden cluster and updates the Shoot's spec to reference it. + +2. **Configuration Updates**: When the Greenhouse AuthenticationConfiguration ConfigMap is updated with new OIDC settings: + - shoot-grafter merges the updated configuration with any existing Garden cluster configuration + - If the Shoot spec already references the correct ConfigMap (no spec change needed), shoot-grafter automatically triggers a Shoot reconciliation by annotating it with `gardener.cloud/operation: reconcile` to apply the changes immediately without waiting for the Shoot's maintenance window + - See the [Gardener documentation on immediate reconciliation](https://gardener.cloud/docs/gardener/shoot-operations/shoot_operations/#immediate-reconciliation) for more details + +**Note**: If the Shoot spec needs to be updated (e.g., when the ConfigMap reference changes), the spec update itself triggers reconciliation automatically, so no additional annotation is needed. + ## Debugging Shoot Reconciliation shoot-grafter emits Kubernetes events to help you monitor and debug the Shoot onboarding process. Events are associated with the CareInstruction resource. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 857882ac..7effe7ad 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -2,8 +2,6 @@ // // SPDX-License-Identifier: Apache-2.0 -//go:build !ignore_autogenerated - // Code generated by controller-gen. DO NOT EDIT. package v1alpha1 diff --git a/controller/shoot/auth.go b/controller/shoot/auth.go index 8ef2fa10..aa4c6114 100644 --- a/controller/shoot/auth.go +++ b/controller/shoot/auth.go @@ -139,6 +139,24 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot return fmt.Errorf("failed to update Shoot spec with OIDC authentication ConfigMap reference: %w", err) } r.Info("Updated Shoot spec with OIDC configuration", "shoot", shoot.Name, "configMap", configMapName) + return nil // Spec change triggers reconciliation automatically + } + + // At this point, Shoot spec doesn't need updates (ConfigMapName reference already exists) + // Trigger Shoot reconciliation if ConfigMap content was updated + // Reference: https://gardener.cloud/docs/gardener/shoot-operations/shoot_operations/#immediate-reconciliation + if configMapResult == controllerutil.OperationResultUpdated { + if shoot.Annotations == nil { + shoot.Annotations = make(map[string]string) + } + shoot.Annotations["gardener.cloud/operation"] = "reconcile" + + if err := r.GardenClient.Update(ctx, shoot); err != nil { + return fmt.Errorf("failed to annotate Shoot for reconciliation: %w", err) + } + r.Info("Annotated Shoot for reconciliation due to ConfigMap content update", + "shoot", shoot.Name, + "configMap", configMapName) } return nil diff --git a/controller/shoot/auth_test.go b/controller/shoot/auth_test.go index 14539a8e..9f9f8ce8 100644 --- a/controller/shoot/auth_test.go +++ b/controller/shoot/auth_test.go @@ -93,7 +93,7 @@ var _ = Describe("Auth", func() { ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: stringPtr("greenhouse:"), + Prefix: new("greenhouse:"), }, }, }, @@ -132,7 +132,7 @@ jwt: ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: stringPtr("greenhouse:"), + Prefix: new("greenhouse:"), }, }, }, @@ -182,7 +182,7 @@ jwt: ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: stringPtr("greenhouse:"), + Prefix: new("greenhouse:"), }, }, }, @@ -237,7 +237,7 @@ jwt: ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: stringPtr("greenhouse:"), + Prefix: new("greenhouse:"), }, }, }, @@ -299,7 +299,7 @@ jwt: ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: stringPtr("greenhouse:"), + Prefix: new("greenhouse:"), }, }, }, @@ -311,7 +311,7 @@ jwt: ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "email", - Prefix: stringPtr("other:"), + Prefix: new("other:"), }, }, }, @@ -381,8 +381,3 @@ jwt: }) }) }) - -// stringPtr returns a pointer to the given string -func stringPtr(s string) *string { - return &s -} diff --git a/controller/shoot/shoot_controller_test.go b/controller/shoot/shoot_controller_test.go index f1036f1a..83025008 100644 --- a/controller/shoot/shoot_controller_test.go +++ b/controller/shoot/shoot_controller_test.go @@ -2102,4 +2102,272 @@ jwt: }, "3s", "500ms").Should(Succeed()) }) }) + + When("testing OIDC authentication configuration with Shoot annotation", func() { + BeforeEach(func() { + careInstruction = &v1alpha1.CareInstruction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-careinstruction-oidc", + Namespace: "default", + }, + Spec: v1alpha1.CareInstructionSpec{ + ShootSelector: &v1alpha1.ShootSelector{ + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "oidc-test": "true", + }, + }, + }, + AuthenticationConfigMapName: "greenhouse-oidc-config", + }, + } + }) + + It("should NOT annotate Shoot when setting up OIDC for the first time", func() { + // Create Greenhouse auth ConfigMap + greenhouseAuthCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "greenhouse-oidc-config", + Namespace: "default", + }, + Data: map[string]string{ + "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://greenhouse.example.com + audiences: + - greenhouse + claimMappings: + username: + claim: sub + prefix: 'greenhouse:' +`, + }, + } + Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthCM)).To(Succeed(), "should create Greenhouse auth ConfigMap") + + // Create a shoot without any OIDC configuration + shoot := &gardenerv1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot-oidc-first-time", + Namespace: "default", + Labels: map[string]string{ + "oidc-test": "true", + }, + }, + } + Expect(test.GardenK8sClient.Create(test.Ctx, shoot)).To(Succeed(), "should create Shoot resource") + + shoot.Status = gardenerv1beta1.ShootStatus{ + AdvertisedAddresses: []gardenerv1beta1.ShootAdvertisedAddress{ + { + Name: "external", + URL: "https://api-server.test-shoot-oidc-first-time.example.com", + }, + }, + } + Expect(test.GardenK8sClient.Status().Update(test.Ctx, shoot)).To(Succeed(), "should update Shoot status") + + // Create CA ConfigMap + caCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot-oidc-first-time.ca-cluster", + Namespace: "default", + }, + Data: map[string]string{ + "ca.crt": "test-ca-data", + }, + } + Expect(test.GardenK8sClient.Create(test.Ctx, caCM)).To(Succeed(), "should create CA ConfigMap") + + // Eventually verify the Shoot spec was updated with ConfigMap reference + // but NO reconcile annotation was added (spec change triggers reconciliation) + Eventually(func(g Gomega) bool { + updatedShoot := &gardenerv1beta1.Shoot{} + err := test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "test-shoot-oidc-first-time", + Namespace: "default", + }, updatedShoot) + g.Expect(err).NotTo(HaveOccurred(), "should get updated Shoot") + + // Verify Shoot spec was updated with ConfigMap reference + if updatedShoot.Spec.Kubernetes.KubeAPIServer == nil || + updatedShoot.Spec.Kubernetes.KubeAPIServer.StructuredAuthentication == nil || + updatedShoot.Spec.Kubernetes.KubeAPIServer.StructuredAuthentication.ConfigMapName == "" { + return false + } + + // Verify NO reconcile annotation was added (spec change is enough) + if updatedShoot.Annotations != nil { + _, hasReconcileAnnotation := updatedShoot.Annotations["gardener.cloud/operation"] + g.Expect(hasReconcileAnnotation).To(BeFalse(), "should NOT have reconcile annotation for first-time setup") + } + + return true + }).Should(BeTrue(), "should eventually update Shoot spec without reconcile annotation") + }) + + It("should annotate Shoot with gardener.cloud/operation=reconcile when ConfigMap content changes", func() { + // Create Greenhouse auth ConfigMap + greenhouseAuthCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "greenhouse-oidc-config", + Namespace: "default", + }, + Data: map[string]string{ + "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://greenhouse.example.com + audiences: + - greenhouse + claimMappings: + username: + claim: sub + prefix: 'greenhouse:' +`, + }, + } + Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthCM)).To(Succeed(), "should create Greenhouse auth ConfigMap") + + // Create a shoot that ALREADY has the OIDC ConfigMap reference + shoot := &gardenerv1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot-oidc-update", + Namespace: "default", + Labels: map[string]string{ + "oidc-test": "true", + }, + }, + Spec: gardenerv1beta1.ShootSpec{ + Kubernetes: gardenerv1beta1.Kubernetes{ + KubeAPIServer: &gardenerv1beta1.KubeAPIServerConfig{ + StructuredAuthentication: &gardenerv1beta1.StructuredAuthentication{ + ConfigMapName: "test-careinstruction-oidc-greenhouse-auth", + }, + }, + }, + }, + } + Expect(test.GardenK8sClient.Create(test.Ctx, shoot)).To(Succeed(), "should create Shoot resource") + + shoot.Status = gardenerv1beta1.ShootStatus{ + AdvertisedAddresses: []gardenerv1beta1.ShootAdvertisedAddress{ + { + Name: "external", + URL: "https://api-server.test-shoot-oidc-update.example.com", + }, + }, + } + Expect(test.GardenK8sClient.Status().Update(test.Ctx, shoot)).To(Succeed(), "should update Shoot status") + + // Create CA ConfigMap + caCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot-oidc-update.ca-cluster", + Namespace: "default", + }, + Data: map[string]string{ + "ca.crt": "test-ca-data", + }, + } + Expect(test.GardenK8sClient.Create(test.Ctx, caCM)).To(Succeed(), "should create CA ConfigMap") + + // Create the OIDC ConfigMap in Garden cluster with initial content + gardenOIDCCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-careinstruction-oidc-greenhouse-auth", + Namespace: "default", + }, + Data: map[string]string{ + "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://greenhouse.example.com + audiences: + - old-audience + claimMappings: + username: + claim: email +`, + }, + } + Expect(test.GardenK8sClient.Create(test.Ctx, gardenOIDCCM)).To(Succeed(), "should create Garden OIDC ConfigMap") + + // Wait a moment for initial reconciliation + Eventually(func(g Gomega) bool { + secret := &corev1.Secret{} + err := test.K8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "test-shoot-oidc-update", + Namespace: "default", + }, secret) + return err == nil + }).Should(BeTrue(), "should eventually create secret") + + // Now update the Greenhouse ConfigMap (simulating content change) + Eventually(func(g Gomega) error { + cm := &corev1.ConfigMap{} + err := test.K8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "greenhouse-oidc-config", + Namespace: "default", + }, cm) + if err != nil { + return err + } + cm.Data["config.yaml"] = `apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://greenhouse.example.com + audiences: + - new-audience + claimMappings: + username: + claim: sub + prefix: 'greenhouse-updated:' +` + return test.K8sClient.Update(test.Ctx, cm) + }).Should(Succeed(), "should update Greenhouse auth ConfigMap") + + // Trigger reconciliation by updating shoot label + Eventually(func(g Gomega) error { + s := &gardenerv1beta1.Shoot{} + err := test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "test-shoot-oidc-update", + Namespace: "default", + }, s) + if err != nil { + return err + } + if s.Labels == nil { + s.Labels = make(map[string]string) + } + s.Labels["trigger-reconcile"] = "true" + return test.GardenK8sClient.Update(test.Ctx, s) + }).Should(Succeed(), "should update Shoot to trigger reconciliation") + + // Eventually verify the Shoot has the reconcile annotation + Eventually(func(g Gomega) bool { + updatedShoot := &gardenerv1beta1.Shoot{} + err := test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ + Name: "test-shoot-oidc-update", + Namespace: "default", + }, updatedShoot) + g.Expect(err).NotTo(HaveOccurred(), "should get updated Shoot") + + // Verify reconcile annotation was added + if updatedShoot.Annotations == nil { + return false + } + reconcileOp, hasReconcileAnnotation := updatedShoot.Annotations["gardener.cloud/operation"] + g.Expect(hasReconcileAnnotation).To(BeTrue(), "should have reconcile annotation for ConfigMap content change") + g.Expect(reconcileOp).To(Equal("reconcile"), "should have correct reconcile annotation value") + + return true + }).Should(BeTrue(), "should eventually add reconcile annotation to Shoot") + }) + }) }) diff --git a/go.mod b/go.mod index 6506fa74..e8d83f14 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module shoot-grafter -go 1.25.0 +go 1.26 // Synced from greenhouse v0.9.0 go.mod replace ( diff --git a/shell.nix b/shell.nix index a536c168..a75e89c2 100644 --- a/shell.nix +++ b/shell.nix @@ -9,7 +9,7 @@ mkShell { nativeBuildInputs = [ addlicense go-licence-detector - go_1_25 + go_1_26 gotools # goimports renovate reuse From 9f5e844e39cab3a08add1f397bcf3444c9320c1f Mon Sep 17 00:00:00 2001 From: Mikolaj Kucinski Date: Fri, 24 Apr 2026 16:55:47 +0200 Subject: [PATCH 05/20] chore: bump Go base image to 1.26.0 to match go.mod (#47) * chore: bump Go base image to 1.26.0 to match go.mod On-behalf-of: @SAP * chore: make typos binary cleanup idempotent On-behalf-of: @SAP Signed-off-by: Zaggy21 --- .github/workflows/checks.yaml | 2 +- Dockerfile | 2 +- Makefile.maker.yaml | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index cce04ad3..ef8d9c3d 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -45,7 +45,7 @@ jobs: env: CLICOLOR: "1" - name: Delete typos binary - run: rm typos + run: rm -f typos - name: Check if source code files have license header run: make check-addlicense - name: REUSE Compliance Check diff --git a/Dockerfile b/Dockerfile index 3dcafcab..0538833d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 # Build the manager binary -FROM --platform=${BUILDPLATFORM:-linux/amd64} golang:1.25.1 AS builder +FROM --platform=${BUILDPLATFORM:-linux/amd64} golang:1.26.0 AS builder ARG TARGETOS ARG TARGETARCH diff --git a/Makefile.maker.yaml b/Makefile.maker.yaml index 0009e4d2..2d904add 100644 --- a/Makefile.maker.yaml +++ b/Makefile.maker.yaml @@ -6,6 +6,7 @@ # NOTE: After running go-makefile-maker, manually apply these changes: # 1. Add 'branches: [main]' to container-registry-ghcr.yaml for main branch builds # 2. Change 'make build/cover.out' to 'make test-with-envtest' in ci.yaml for envtest support +# 3. Change 'rm typos' to 'rm -f typos' in checks.yaml metadata: url: https://github.com/cloudoperators/shoot-grafter From 62c40a930cd8da2f630381cb13b861e46085d220 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Mon, 27 Apr 2026 16:27:11 +0200 Subject: [PATCH 06/20] wait for the greenhouse cache before making assertions On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/controller/shoot/shoot_controller_test.go b/controller/shoot/shoot_controller_test.go index 83025008..7160ca0c 100644 --- a/controller/shoot/shoot_controller_test.go +++ b/controller/shoot/shoot_controller_test.go @@ -101,6 +101,12 @@ var _ = Describe("Shoot Controller", func() { Expect(greenhouseMgr.Start(ghCtx)).To(Succeed(), "there must be no error starting the greenhouse manager") }() + // Wait for the Greenhouse cache to be fully synced before proceeding. + // This ensures the source.Kind watch on the Greenhouse cache is established + // before any test assertions that depend on watch-triggered reconciliation. + Expect(greenhouseMgr.GetCache().WaitForCacheSync(ghCtx)).To(BeTrue(), + "the greenhouse manager cache must be synced before running watch-dependent assertions") + // start the garden manager go func() { defer GinkgoRecover() From b6f9e84c14d89cf4f517c2bc03fd1e80fa0b32b9 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Mon, 27 Apr 2026 17:04:01 +0200 Subject: [PATCH 07/20] unify the prefix for labels to be shoot-grafter.cloudoperators.dev On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- README.md | 2 +- api/v1alpha1/careinstruction_types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6da15b07..97945abe 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ For each CareInstruction, a dedicated Shoot controller is dynamically created an - Optionally configures OIDC authentication on Shoot clusters for Greenhouse access. Also see respective [Greenhouse docs](https://cloudoperators.github.io/greenhouse/docs/user-guides/cluster/oidc-login/) and [Gardener docs](https://gardener.cloud/docs/guides/administer-shoots/oidc-login/#configure-the-shoot-cluster) - Optionally configures RBAC on the Shoot cluster for Greenhouse access -> **Auth ConfigMap labeling & watch**: When `authenticationConfigMapName` is set, the shoot controller labels the referenced Greenhouse ConfigMap with `shoot-grafter.cloudoperators.dev/auth-configmap: "true"` and `shoot-grafter.cloudoperators.dev/careinstruction: ` on first interaction (creation or update). The controller also watches these labeled ConfigMaps on the Greenhouse cluster; any change to the auth ConfigMap automatically re-enqueues all matching Shoots so the Garden-side OIDC configuration stays in sync without waiting for the next Shoot event. +> **Auth ConfigMap labeling & watch**: When `authenticationConfigMapName` is set, the shoot controller labels the referenced Greenhouse ConfigMap with `shoot-grafter.cloudoperators.dev/authconfigmap: "true"` and `shoot-grafter.cloudoperators.dev/careinstruction: ` on first interaction (creation or update). The controller also watches these labeled ConfigMaps on the Greenhouse cluster; any change to the auth ConfigMap automatically re-enqueues all matching Shoots so the Garden-side OIDC configuration stays in sync without waiting for the next Shoot event. ## Custom Resource: CareInstruction diff --git a/api/v1alpha1/careinstruction_types.go b/api/v1alpha1/careinstruction_types.go index 4d4eae40..8bd380da 100644 --- a/api/v1alpha1/careinstruction_types.go +++ b/api/v1alpha1/careinstruction_types.go @@ -36,7 +36,7 @@ const ( CareInstructionLabel = "shoot-grafter.cloudoperators.dev/careinstruction" // AuthConfigMapLabel is the label used to identify AuthenticationConfiguration ConfigMaps - AuthConfigMapLabel = "shoot-grafter.cloudoperators/authconfigmap" + AuthConfigMapLabel = "shoot-grafter.cloudoperators.dev/authconfigmap" // ShootStatusOnboarded indicates the shoot has been onboarded as a Greenhouse Cluster. ShootStatusOnboarded = "Onboarded" From b519f20be0ea842ca19b87c2c1313c339a8c4925 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Mon, 27 Apr 2026 17:18:29 +0200 Subject: [PATCH 08/20] patch the labels on auth configmap instead of updating On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/auth.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/controller/shoot/auth.go b/controller/shoot/auth.go index aa4c6114..9ffaef86 100644 --- a/controller/shoot/auth.go +++ b/controller/shoot/auth.go @@ -36,6 +36,8 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot // Add labels if missing: AuthConfigMapLabel marks CMs as auth config maps, // CareInstructionLabel associates the CM with the owning CareInstruction. + // Take a snapshot before any mutations so the patch only touches metadata.labels. + base := greenhouseAuthConfigMap.DeepCopy() if greenhouseAuthConfigMap.Labels == nil { greenhouseAuthConfigMap.Labels = make(map[string]string) } @@ -49,8 +51,10 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot labelsNeedUpdate = true } if labelsNeedUpdate { - if err := r.GreenhouseClient.Update(ctx, &greenhouseAuthConfigMap); err != nil { - r.Info("failed to add labels to auth ConfigMap", "configMap", greenhouseAuthConfigMap.Name, "error", err) + // Use a merge patch so only metadata.labels is sent to the API server. + // This avoids overwriting concurrent changes to config.yaml or other fields. + if err := r.GreenhouseClient.Patch(ctx, &greenhouseAuthConfigMap, client.MergeFrom(base)); err != nil { + r.Info("failed to patch labels on auth ConfigMap", "configMap", greenhouseAuthConfigMap.Name, "error", err) // Don't fail the reconciliation for this, just log it } } From 543fa364a97d2753a5b3a317131b3ed807ba8695 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 28 Apr 2026 10:55:57 +0200 Subject: [PATCH 09/20] fix: skip auth CM relabel when already owned by another CareInstruction On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/auth.go | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/controller/shoot/auth.go b/controller/shoot/auth.go index 9ffaef86..ad3c8324 100644 --- a/controller/shoot/auth.go +++ b/controller/shoot/auth.go @@ -34,22 +34,33 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot r.CareInstruction.Spec.AuthenticationConfigMapName, err) } - // Add labels if missing: AuthConfigMapLabel marks CMs as auth config maps, - // CareInstructionLabel associates the CM with the owning CareInstruction. + // Label the ConfigMap so the watch predicate can identify it and associate it with this CareInstruction. // Take a snapshot before any mutations so the patch only touches metadata.labels. base := greenhouseAuthConfigMap.DeepCopy() if greenhouseAuthConfigMap.Labels == nil { greenhouseAuthConfigMap.Labels = make(map[string]string) } labelsNeedUpdate := false - if _, hasLabel := greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel]; !hasLabel { - greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel] = "true" - labelsNeedUpdate = true - } - if val, hasLabel := greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel]; !hasLabel || val != r.CareInstruction.Name { - greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] = r.CareInstruction.Name - labelsNeedUpdate = true + + // If the CM is already owned by a different CareInstruction, skip relabelling to + // avoid breaking its watch predicate. OIDC merging still proceeds normally. + existingOwner, hasCILabel := greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] + if hasCILabel && existingOwner != r.CareInstruction.Name { + r.Info("auth ConfigMap is already owned by another CareInstruction; skipping relabel to avoid breaking its watch", + "configMap", greenhouseAuthConfigMap.Name, + "existingOwner", existingOwner, + "thisCareInstruction", r.CareInstruction.Name) + } else { + if _, hasAuthLabel := greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel]; !hasAuthLabel { + greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel] = "true" + labelsNeedUpdate = true + } + if !hasCILabel { + greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] = r.CareInstruction.Name + labelsNeedUpdate = true + } } + if labelsNeedUpdate { // Use a merge patch so only metadata.labels is sent to the API server. // This avoids overwriting concurrent changes to config.yaml or other fields. From b97b5380435c0539e0ae3a21858ccf9ce11c3a00 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 28 Apr 2026 14:15:04 +0200 Subject: [PATCH 10/20] fix suggestions On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller.go | 4 ++-- controller/shoot/shoot_controller_test.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index 6dfd1163..8638f01f 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -112,9 +112,9 @@ func (r *ShootController) enqueueShootsForAuthConfigMap(ctx context.Context, _ * r.Info("failed to list shoots for auth ConfigMap change", "error", err) return nil } - requests := make([]ctrl.Request, 0, len(shoots.Items)) + requests := make([]reconcile.Request, 0, len(shoots.Items)) for _, shoot := range shoots.Items { - requests = append(requests, ctrl.Request{ + requests = append(requests, reconcile.Request{ NamespacedName: client.ObjectKey{ Namespace: shoot.Namespace, Name: shoot.Name, diff --git a/controller/shoot/shoot_controller_test.go b/controller/shoot/shoot_controller_test.go index 7160ca0c..ef5aa3f8 100644 --- a/controller/shoot/shoot_controller_test.go +++ b/controller/shoot/shoot_controller_test.go @@ -77,7 +77,6 @@ var _ = Describe("Shoot Controller", func() { // Create ShootController with EventRecorder and GreenhouseMgr from the Greenhouse manager. // GreenhouseMgr is required so the ShootController can watch Greenhouse auth CMs and // re-enqueue Shoots when they change. - Expect(err).NotTo(HaveOccurred(), "there must be no error creating the manager") Expect((&shoot.ShootController{ GreenhouseClient: test.K8sClient, GreenhouseMgr: greenhouseMgr, // Provide Greenhouse manager for cross-cluster CM watch From af259b5ac88e328026581175218e9d1b421525fe Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 28 Apr 2026 15:29:24 +0200 Subject: [PATCH 11/20] filter shoots not matching CEL for watch On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index 8638f01f..291aeb26 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -114,6 +114,9 @@ func (r *ShootController) enqueueShootsForAuthConfigMap(ctx context.Context, _ * } requests := make([]reconcile.Request, 0, len(shoots.Items)) for _, shoot := range shoots.Items { + if !r.matchesCEL(&shoot) { + continue + } requests = append(requests, reconcile.Request{ NamespacedName: client.ObjectKey{ Namespace: shoot.Namespace, From 9b637cf6408fcaa5e0ad7a8636b94183639c967e Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 28 Apr 2026 15:40:45 +0200 Subject: [PATCH 12/20] change the AuthConfigMapLabelKey to shoot-grafter.cloudoperators.dev/auth-configmap On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- README.md | 2 +- api/v1alpha1/careinstruction_types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 97945abe..6da15b07 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ For each CareInstruction, a dedicated Shoot controller is dynamically created an - Optionally configures OIDC authentication on Shoot clusters for Greenhouse access. Also see respective [Greenhouse docs](https://cloudoperators.github.io/greenhouse/docs/user-guides/cluster/oidc-login/) and [Gardener docs](https://gardener.cloud/docs/guides/administer-shoots/oidc-login/#configure-the-shoot-cluster) - Optionally configures RBAC on the Shoot cluster for Greenhouse access -> **Auth ConfigMap labeling & watch**: When `authenticationConfigMapName` is set, the shoot controller labels the referenced Greenhouse ConfigMap with `shoot-grafter.cloudoperators.dev/authconfigmap: "true"` and `shoot-grafter.cloudoperators.dev/careinstruction: ` on first interaction (creation or update). The controller also watches these labeled ConfigMaps on the Greenhouse cluster; any change to the auth ConfigMap automatically re-enqueues all matching Shoots so the Garden-side OIDC configuration stays in sync without waiting for the next Shoot event. +> **Auth ConfigMap labeling & watch**: When `authenticationConfigMapName` is set, the shoot controller labels the referenced Greenhouse ConfigMap with `shoot-grafter.cloudoperators.dev/auth-configmap: "true"` and `shoot-grafter.cloudoperators.dev/careinstruction: ` on first interaction (creation or update). The controller also watches these labeled ConfigMaps on the Greenhouse cluster; any change to the auth ConfigMap automatically re-enqueues all matching Shoots so the Garden-side OIDC configuration stays in sync without waiting for the next Shoot event. ## Custom Resource: CareInstruction diff --git a/api/v1alpha1/careinstruction_types.go b/api/v1alpha1/careinstruction_types.go index 8bd380da..80338820 100644 --- a/api/v1alpha1/careinstruction_types.go +++ b/api/v1alpha1/careinstruction_types.go @@ -36,7 +36,7 @@ const ( CareInstructionLabel = "shoot-grafter.cloudoperators.dev/careinstruction" // AuthConfigMapLabel is the label used to identify AuthenticationConfiguration ConfigMaps - AuthConfigMapLabel = "shoot-grafter.cloudoperators.dev/authconfigmap" + AuthConfigMapLabel = "shoot-grafter.cloudoperators.dev/auth-configmap" // ShootStatusOnboarded indicates the shoot has been onboarded as a Greenhouse Cluster. ShootStatusOnboarded = "Onboarded" From 13cf18b35b0af86c146e45ced9027c00d3f3e1c3 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 29 Apr 2026 10:50:35 +0200 Subject: [PATCH 13/20] restrict watch to specific configmaps, add a timeout to greenhouse manager context in tests On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller.go | 14 ++++++-------- controller/shoot/shoot_controller_test.go | 9 +++++---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index 291aeb26..726a8082 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -83,16 +83,14 @@ func (r *ShootController) SetupWithManager(mgr ctrl.Manager) error { Named(r.Name). For(&gardenerv1beta1.Shoot{}, builder.WithPredicates(predicates...)) - // Watch AuthenticationConfiguration ConfigMaps on the Greenhouse cluster. - // When they change, all Shoots managed by this CareInstruction are re-enqueued - // so OIDC config stays in sync with the Greenhouse source. - // The predicate restricts events to CMs that carry both the auth-configmap marker - // and the CareInstruction ownership label matching this controller instance. + // Watch the auth ConfigMap on the Greenhouse cluster; re-enqueue all Shoots when it changes + // so the Garden-side OIDC config stays in sync. if r.CareInstruction.Spec.AuthenticationConfigMapName != "" && r.GreenhouseMgr != nil { authCMPredicate := predicate.NewTypedPredicateFuncs(func(cm *corev1.ConfigMap) bool { - labels := cm.GetLabels() - return labels[v1alpha1.AuthConfigMapLabel] == "true" && - labels[v1alpha1.CareInstructionLabel] == r.CareInstruction.Name + return cm.GetName() == r.CareInstruction.Spec.AuthenticationConfigMapName && + cm.GetNamespace() == r.CareInstruction.GetNamespace() && + cm.GetLabels()[v1alpha1.AuthConfigMapLabel] == "true" && + cm.GetLabels()[v1alpha1.CareInstructionLabel] == r.CareInstruction.Name }) b = b.WatchesRawSource(source.Kind( r.GreenhouseMgr.GetCache(), diff --git a/controller/shoot/shoot_controller_test.go b/controller/shoot/shoot_controller_test.go index ef5aa3f8..cf56f38a 100644 --- a/controller/shoot/shoot_controller_test.go +++ b/controller/shoot/shoot_controller_test.go @@ -6,6 +6,7 @@ package shoot_test import ( "context" "encoding/base64" + "time" "shoot-grafter/api/v1alpha1" "shoot-grafter/controller/shoot" @@ -100,10 +101,10 @@ var _ = Describe("Shoot Controller", func() { Expect(greenhouseMgr.Start(ghCtx)).To(Succeed(), "there must be no error starting the greenhouse manager") }() - // Wait for the Greenhouse cache to be fully synced before proceeding. - // This ensures the source.Kind watch on the Greenhouse cache is established - // before any test assertions that depend on watch-triggered reconciliation. - Expect(greenhouseMgr.GetCache().WaitForCacheSync(ghCtx)).To(BeTrue(), + // Wait for the Greenhouse cache to sync before proceeding so the auth CM watch is established. + syncCtx, syncCancel := context.WithTimeout(ghCtx, 30*time.Second) + defer syncCancel() + Expect(greenhouseMgr.GetCache().WaitForCacheSync(syncCtx)).To(BeTrue(), "the greenhouse manager cache must be synced before running watch-dependent assertions") // start the garden manager From 4b70d23172aff5eab48bef0f59f417dba6a3c704 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 29 Apr 2026 15:24:29 +0200 Subject: [PATCH 14/20] change predicate to fire only when Data in config map changed On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index 726a8082..2fff696e 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -86,12 +86,23 @@ func (r *ShootController) SetupWithManager(mgr ctrl.Manager) error { // Watch the auth ConfigMap on the Greenhouse cluster; re-enqueue all Shoots when it changes // so the Garden-side OIDC config stays in sync. if r.CareInstruction.Spec.AuthenticationConfigMapName != "" && r.GreenhouseMgr != nil { - authCMPredicate := predicate.NewTypedPredicateFuncs(func(cm *corev1.ConfigMap) bool { - return cm.GetName() == r.CareInstruction.Spec.AuthenticationConfigMapName && - cm.GetNamespace() == r.CareInstruction.GetNamespace() && - cm.GetLabels()[v1alpha1.AuthConfigMapLabel] == "true" && - cm.GetLabels()[v1alpha1.CareInstructionLabel] == r.CareInstruction.Name - }) + authCMPredicate := predicate.TypedFuncs[*corev1.ConfigMap]{ + CreateFunc: func(_ event.TypedCreateEvent[*corev1.ConfigMap]) bool { return false }, + UpdateFunc: func(e event.TypedUpdateEvent[*corev1.ConfigMap]) bool { + old, new := e.ObjectOld, e.ObjectNew + if new.GetName() != r.CareInstruction.Spec.AuthenticationConfigMapName || + new.GetNamespace() != r.CareInstruction.GetNamespace() { + return false + } + if new.GetLabels()[v1alpha1.AuthConfigMapLabel] != "true" || + new.GetLabels()[v1alpha1.CareInstructionLabel] != r.CareInstruction.Name { + return false + } + // Only fire when Data changed. + return !maps.Equal(old.Data, new.Data) + }, + DeleteFunc: func(_ event.TypedDeleteEvent[*corev1.ConfigMap]) bool { return false }, + } b = b.WatchesRawSource(source.Kind( r.GreenhouseMgr.GetCache(), &corev1.ConfigMap{}, From c9f9369f50216d62ce03d659e4f9b4c93a6d496f Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 29 Apr 2026 15:31:38 +0200 Subject: [PATCH 15/20] fix lint error On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/shoot/shoot_controller.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index 2fff696e..f74865ed 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -89,17 +89,16 @@ func (r *ShootController) SetupWithManager(mgr ctrl.Manager) error { authCMPredicate := predicate.TypedFuncs[*corev1.ConfigMap]{ CreateFunc: func(_ event.TypedCreateEvent[*corev1.ConfigMap]) bool { return false }, UpdateFunc: func(e event.TypedUpdateEvent[*corev1.ConfigMap]) bool { - old, new := e.ObjectOld, e.ObjectNew - if new.GetName() != r.CareInstruction.Spec.AuthenticationConfigMapName || - new.GetNamespace() != r.CareInstruction.GetNamespace() { + oldCM, newCM := e.ObjectOld, e.ObjectNew + if newCM.GetName() != r.CareInstruction.Spec.AuthenticationConfigMapName || + newCM.GetNamespace() != r.CareInstruction.GetNamespace() { return false } - if new.GetLabels()[v1alpha1.AuthConfigMapLabel] != "true" || - new.GetLabels()[v1alpha1.CareInstructionLabel] != r.CareInstruction.Name { + if newCM.GetLabels()[v1alpha1.AuthConfigMapLabel] != "true" || + newCM.GetLabels()[v1alpha1.CareInstructionLabel] != r.CareInstruction.Name { return false } - // Only fire when Data changed. - return !maps.Equal(old.Data, new.Data) + return !maps.Equal(oldCM.Data, newCM.Data) }, DeleteFunc: func(_ event.TypedDeleteEvent[*corev1.ConfigMap]) bool { return false }, } From 1807100707495c5a70404dbfc9d95ca344e1fcfe Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 12 May 2026 10:41:32 +0200 Subject: [PATCH 16/20] feat: move auth ConfigMap watch from ShootController to CareInstruction controller, adapt tests On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .../careinstruction_controller.go | 100 ++++++- controller/shoot/auth.go | 85 +++--- controller/shoot/shoot_controller.go | 66 +---- controller/shoot/shoot_controller_test.go | 278 +++--------------- 4 files changed, 183 insertions(+), 346 deletions(-) diff --git a/controller/careinstruction/careinstruction_controller.go b/controller/careinstruction/careinstruction_controller.go index e7e8dada..cf84cc21 100644 --- a/controller/careinstruction/careinstruction_controller.go +++ b/controller/careinstruction/careinstruction_controller.go @@ -6,6 +6,7 @@ package careinstruction import ( "context" "errors" + "maps" "reflect" "sync" @@ -18,9 +19,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/config" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/predicate" greenhouseapis "github.com/cloudoperators/greenhouse/api" greenhousemetav1alpha1 "github.com/cloudoperators/greenhouse/api/meta/v1alpha1" @@ -52,6 +55,7 @@ type garden struct { careInstructionSpec *v1alpha1.CareInstructionSpec // The CareInstruction object for the garden cluster cancelFunc context.CancelFunc // Cancel function to stop the manager stopChan chan bool // Channel to know if the manager is stopped + authConfigMapData map[string]string // In-memory cache of the latest auth ConfigMap Data } type careInstructionContextKey struct{} @@ -64,11 +68,35 @@ type careInstructionContextKey struct{} // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch;delete func (r *CareInstructionReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Auth ConfigMap predicate: only re-enqueue the CareInstruction when ConfigMap Data changes. + // Label-only patches (e.g. from our own MergeFrom calls in auth.go) are ignored. + authCMDataChangedPredicate := predicate.Funcs{ + CreateFunc: func(_ event.CreateEvent) bool { return false }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldCM, ok1 := e.ObjectOld.(*corev1.ConfigMap) + newCM, ok2 := e.ObjectNew.(*corev1.ConfigMap) + if !ok1 || !ok2 { + return false + } + return !maps.Equal(oldCM.Data, newCM.Data) + }, + DeleteFunc: func(_ event.DeleteEvent) bool { return false }, + } + // Setup the controller with the manager return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.CareInstruction{}). Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.enqueueCareInstructionForGardenCluster), builder.WithPredicates(clientutil.PredicateFilterBySecretTypes(greenhouseapis.SecretTypeKubeConfig, greenhouseapis.SecretTypeOIDCConfig))). Watches(&greenhousev1alpha1.Cluster{}, handler.EnqueueRequestsFromMapFunc(r.enqueueCareInstructionForCreatedClusters), builder.WithPredicates(clientutil.PredicateHasLabel(v1alpha1.CareInstructionLabel))). + // Watch auth ConfigMaps on the Greenhouse cluster. When their Data changes, re-enqueue the owning + // CareInstruction so reconcileManager detects the change and restarts the ShootController with + // the updated CM data. + Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.enqueueCareInstructionForAuthConfigMap), + builder.WithPredicates( + clientutil.PredicateHasLabel(v1alpha1.AuthConfigMapLabel), + authCMDataChangedPredicate, + ), + ). Complete(r) } @@ -172,6 +200,7 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn gardenClient: nil, careInstructionSpec: &careInstruction.Spec, cancelFunc: nil, + authConfigMapData: nil, } } r.gardensMu.Unlock() @@ -197,6 +226,12 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn ), ) + // Fetch the current auth ConfigMap data so we can detect changes and pass it to the ShootController. + currentAuthCMData, authCMErr := r.fetchAuthConfigMapData(ctx, &careInstruction) + if authCMErr != nil && !apierrors.IsNotFound(authCMErr) { + r.Info("failed to fetch auth ConfigMap data, will proceed without it", "error", authCMErr) + } + // Now we check the following to see if we need to recreate and restart the manager (with read lock): r.gardensMu.RLock() garden := r.gardens[gardenKey] @@ -221,9 +256,11 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn channelOpen = true } } + // 6. If the auth ConfigMap data has changed, the ShootController must be restarted with the new data + authConfigMapDataChanged := !maps.Equal(garden.authConfigMapData, currentAuthCMData) r.gardensMu.RUnlock() - if mgrExists && shootControllerStarted && !gardenConfigChanged && !careInstructionSpecChanged && channelExists && channelOpen { + if mgrExists && shootControllerStarted && !gardenConfigChanged && !careInstructionSpecChanged && channelExists && channelOpen && !authConfigMapDataChanged { r.Info("Manager is running, garden cluster config & careInstruction.Spec is unchanged, skipping client and manager recreation", "careInstruction", careInstruction.Name) return nil } @@ -241,6 +278,8 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn reason = "stop channel is missing" case !channelOpen: reason = "manager stop channel is closed" + case authConfigMapDataChanged: + reason = "auth ConfigMap data has changed" default: reason = "unknown reason" } @@ -287,17 +326,17 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn return err } - // Register the ShootController with the garden manager + // Register the ShootController with the garden manager. // Note: EventRecorder is obtained from the Greenhouse manager to emit events on the Greenhouse cluster. - // GreenhouseMgr is passed so the ShootController can watch Greenhouse cluster resources (e.g. auth CMs). + // AuthConfigMapData is passed in-memory from the CareInstruction controller, which owns the auth CM watch. sc := &shoot.ShootController{ - GreenhouseClient: r.Client, - GreenhouseMgr: r.Manager, - GardenClient: gardenClient, - Logger: r.WithValues("careInstruction", careInstruction.Name), - Name: shoot.GenerateName(careInstruction.Name), - CareInstruction: careInstruction.DeepCopy(), - EventRecorder: r.GetEventRecorderFor(shoot.GenerateName(careInstruction.Name)), + GreenhouseClient: r.Client, + GardenClient: gardenClient, + Logger: r.WithValues("careInstruction", careInstruction.Name), + Name: shoot.GenerateName(careInstruction.Name), + CareInstruction: careInstruction.DeepCopy(), + EventRecorder: r.GetEventRecorderFor(shoot.GenerateName(careInstruction.Name)), + AuthConfigMapData: currentAuthCMData, } if err := sc.SetupWithManager(shootControllerMgr); err != nil { return err @@ -315,6 +354,7 @@ func (r *CareInstructionReconciler) reconcileManager(ctx context.Context, careIn r.gardens[gardenKey].cancelFunc = cancel r.gardens[gardenKey].stopChan = make(chan bool) r.gardens[gardenKey].careInstructionSpec = &careInstruction.Spec + r.gardens[gardenKey].authConfigMapData = currentAuthCMData stopChan := r.gardens[gardenKey].stopChan r.gardensMu.Unlock() @@ -589,3 +629,43 @@ func (r *CareInstructionReconciler) enqueueCareInstructionForCreatedClusters(_ c }, } } + +// enqueueCareInstructionForAuthConfigMap enqueues the CareInstruction that references the changed auth ConfigMap. +// The CareInstructionLabel on the ConfigMap identifies the owning CareInstruction. +func (r *CareInstructionReconciler) enqueueCareInstructionForAuthConfigMap(_ context.Context, obj client.Object) []ctrl.Request { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + return nil + } + + careInstructionName, exists := cm.Labels[v1alpha1.CareInstructionLabel] + if !exists { + return nil + } + + r.Info("Enqueuing CareInstruction for auth ConfigMap change", "configMap", cm.Name, "careInstruction", careInstructionName) + return []ctrl.Request{ + { + NamespacedName: client.ObjectKey{ + Name: careInstructionName, + Namespace: cm.Namespace, + }, + }, + } +} + +// fetchAuthConfigMapData fetches the current Data of the auth ConfigMap referenced by the CareInstruction. +// Returns nil (no error) when no auth ConfigMap is configured or the CM does not exist yet. +func (r *CareInstructionReconciler) fetchAuthConfigMapData(ctx context.Context, careInstruction *v1alpha1.CareInstruction) (map[string]string, error) { + if careInstruction.Spec.AuthenticationConfigMapName == "" { + return nil, nil + } + var cm corev1.ConfigMap + if err := r.Get(ctx, client.ObjectKey{ + Namespace: careInstruction.Namespace, + Name: careInstruction.Spec.AuthenticationConfigMapName, + }, &cm); err != nil { + return nil, err + } + return cm.Data, nil +} diff --git a/controller/shoot/auth.go b/controller/shoot/auth.go index ad3c8324..366bafdf 100644 --- a/controller/shoot/auth.go +++ b/controller/shoot/auth.go @@ -20,65 +20,60 @@ import ( ) // configureOIDCAuthentication configures OIDC authentication for the Shoot by: -// 1. Fetching the AuthenticationConfiguration ConfigMap from Greenhouse cluster +// 1. Reading the AuthenticationConfiguration from the in-memory auth ConfigMap data (provided by the CareInstruction controller) // 2. Merging it with any existing configuration on the Garden cluster // 3. Updating the Shoot spec to reference the merged configuration func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot *gardenerv1beta1.Shoot) error { - // Fetch the AuthenticationConfiguration ConfigMap from Greenhouse cluster + // Use the in-memory auth ConfigMap data provided by the CareInstruction controller. + // This avoids a cross-cluster watch; the CareInstruction controller is responsible for + // fetching and caching the data, and restarts this ShootController when it changes. + if r.AuthConfigMapData == nil || r.AuthConfigMapData["config.yaml"] == "" { + return fmt.Errorf("AuthenticationConfiguration ConfigMap %s does not contain config.yaml (data not available)", + r.CareInstruction.Spec.AuthenticationConfigMapName) + } + + // Label the Greenhouse auth ConfigMap so the CareInstruction controller's watch predicate can + // identify it and associate it with this CareInstruction. + // We fetch the live CM to get the current metadata, then patch only the labels. var greenhouseAuthConfigMap corev1.ConfigMap if err := r.GreenhouseClient.Get(ctx, client.ObjectKey{ Namespace: r.CareInstruction.Namespace, Name: r.CareInstruction.Spec.AuthenticationConfigMapName, - }, &greenhouseAuthConfigMap); err != nil { - return fmt.Errorf("failed to fetch AuthenticationConfiguration ConfigMap %s from Greenhouse cluster: %w", - r.CareInstruction.Spec.AuthenticationConfigMapName, err) - } - - // Label the ConfigMap so the watch predicate can identify it and associate it with this CareInstruction. - // Take a snapshot before any mutations so the patch only touches metadata.labels. - base := greenhouseAuthConfigMap.DeepCopy() - if greenhouseAuthConfigMap.Labels == nil { - greenhouseAuthConfigMap.Labels = make(map[string]string) - } - labelsNeedUpdate := false - - // If the CM is already owned by a different CareInstruction, skip relabelling to - // avoid breaking its watch predicate. OIDC merging still proceeds normally. - existingOwner, hasCILabel := greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] - if hasCILabel && existingOwner != r.CareInstruction.Name { - r.Info("auth ConfigMap is already owned by another CareInstruction; skipping relabel to avoid breaking its watch", - "configMap", greenhouseAuthConfigMap.Name, - "existingOwner", existingOwner, - "thisCareInstruction", r.CareInstruction.Name) - } else { - if _, hasAuthLabel := greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel]; !hasAuthLabel { - greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel] = "true" - labelsNeedUpdate = true + }, &greenhouseAuthConfigMap); err == nil { + base := greenhouseAuthConfigMap.DeepCopy() + if greenhouseAuthConfigMap.Labels == nil { + greenhouseAuthConfigMap.Labels = make(map[string]string) } - if !hasCILabel { - greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] = r.CareInstruction.Name - labelsNeedUpdate = true + labelsNeedUpdate := false + + // If the CM is already owned by a different CareInstruction, skip relabelling. + existingOwner, hasCILabel := greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] + if hasCILabel && existingOwner != r.CareInstruction.Name { + r.Info("auth ConfigMap is already owned by another CareInstruction; skipping relabel", + "configMap", greenhouseAuthConfigMap.Name, + "existingOwner", existingOwner, + "thisCareInstruction", r.CareInstruction.Name) + } else { + if _, hasAuthLabel := greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel]; !hasAuthLabel { + greenhouseAuthConfigMap.Labels[v1alpha1.AuthConfigMapLabel] = "true" + labelsNeedUpdate = true + } + if !hasCILabel { + greenhouseAuthConfigMap.Labels[v1alpha1.CareInstructionLabel] = r.CareInstruction.Name + labelsNeedUpdate = true + } } - } - if labelsNeedUpdate { - // Use a merge patch so only metadata.labels is sent to the API server. - // This avoids overwriting concurrent changes to config.yaml or other fields. - if err := r.GreenhouseClient.Patch(ctx, &greenhouseAuthConfigMap, client.MergeFrom(base)); err != nil { - r.Info("failed to patch labels on auth ConfigMap", "configMap", greenhouseAuthConfigMap.Name, "error", err) - // Don't fail the reconciliation for this, just log it + if labelsNeedUpdate { + if patchErr := r.GreenhouseClient.Patch(ctx, &greenhouseAuthConfigMap, client.MergeFrom(base)); patchErr != nil { + r.Info("failed to patch labels on auth ConfigMap", "configMap", greenhouseAuthConfigMap.Name, "error", patchErr) + } } } - // Verify the ConfigMap contains config.yaml - if greenhouseAuthConfigMap.Data == nil || greenhouseAuthConfigMap.Data["config.yaml"] == "" { - return fmt.Errorf("AuthenticationConfiguration ConfigMap %s does not contain config.yaml", - r.CareInstruction.Spec.AuthenticationConfigMapName) - } - - // Parse the Greenhouse authentication configuration + // Parse the Greenhouse authentication configuration from in-memory data var greenhouseAuthConfig apiserverv1beta1.AuthenticationConfiguration - if err := yaml.Unmarshal([]byte(greenhouseAuthConfigMap.Data["config.yaml"]), &greenhouseAuthConfig); err != nil { + if err := yaml.Unmarshal([]byte(r.AuthConfigMapData["config.yaml"]), &greenhouseAuthConfig); err != nil { return fmt.Errorf("failed to parse Greenhouse AuthenticationConfiguration: %w", err) } diff --git a/controller/shoot/shoot_controller.go b/controller/shoot/shoot_controller.go index f74865ed..b88ed4d8 100644 --- a/controller/shoot/shoot_controller.go +++ b/controller/shoot/shoot_controller.go @@ -26,10 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" ) const ( @@ -38,9 +35,9 @@ const ( ) type ShootController struct { - GreenhouseClient client.Client - GreenhouseMgr ctrl.Manager // GreenhouseMgr provides the Greenhouse cluster cache for watches - GardenClient client.Client + GreenhouseClient client.Client + GardenClient client.Client + AuthConfigMapData map[string]string // In-memory auth ConfigMap Data provided by the CareInstruction controller logr.Logger Name string CareInstruction *v1alpha1.CareInstruction @@ -79,60 +76,11 @@ func (r *ShootController) SetupWithManager(mgr ctrl.Manager) error { r.Error(nil, "EventRecorder is not set for ShootController", "name", r.Name) } - b := ctrl.NewControllerManagedBy(mgr). - Named(r.Name). - For(&gardenerv1beta1.Shoot{}, builder.WithPredicates(predicates...)) - - // Watch the auth ConfigMap on the Greenhouse cluster; re-enqueue all Shoots when it changes - // so the Garden-side OIDC config stays in sync. - if r.CareInstruction.Spec.AuthenticationConfigMapName != "" && r.GreenhouseMgr != nil { - authCMPredicate := predicate.TypedFuncs[*corev1.ConfigMap]{ - CreateFunc: func(_ event.TypedCreateEvent[*corev1.ConfigMap]) bool { return false }, - UpdateFunc: func(e event.TypedUpdateEvent[*corev1.ConfigMap]) bool { - oldCM, newCM := e.ObjectOld, e.ObjectNew - if newCM.GetName() != r.CareInstruction.Spec.AuthenticationConfigMapName || - newCM.GetNamespace() != r.CareInstruction.GetNamespace() { - return false - } - if newCM.GetLabels()[v1alpha1.AuthConfigMapLabel] != "true" || - newCM.GetLabels()[v1alpha1.CareInstructionLabel] != r.CareInstruction.Name { - return false - } - return !maps.Equal(oldCM.Data, newCM.Data) - }, - DeleteFunc: func(_ event.TypedDeleteEvent[*corev1.ConfigMap]) bool { return false }, - } - b = b.WatchesRawSource(source.Kind( - r.GreenhouseMgr.GetCache(), - &corev1.ConfigMap{}, - handler.TypedEnqueueRequestsFromMapFunc(r.enqueueShootsForAuthConfigMap), - authCMPredicate, - )) - } - // Setup the shoot controller with the manager - return b.Complete(r) -} - -func (r *ShootController) enqueueShootsForAuthConfigMap(ctx context.Context, _ *corev1.ConfigMap) []reconcile.Request { - shoots, err := r.CareInstruction.ListShoots(ctx, r.GardenClient) - if err != nil { - r.Info("failed to list shoots for auth ConfigMap change", "error", err) - return nil - } - requests := make([]reconcile.Request, 0, len(shoots.Items)) - for _, shoot := range shoots.Items { - if !r.matchesCEL(&shoot) { - continue - } - requests = append(requests, reconcile.Request{ - NamespacedName: client.ObjectKey{ - Namespace: shoot.Namespace, - Name: shoot.Name, - }, - }) - } - return requests + return ctrl.NewControllerManagedBy(mgr). + Named(r.Name). + For(&gardenerv1beta1.Shoot{}, builder.WithPredicates(predicates...)). + Complete(r) } func (r *ShootController) newCELPredicate() predicate.Predicate { diff --git a/controller/shoot/shoot_controller_test.go b/controller/shoot/shoot_controller_test.go index cf56f38a..994eb85e 100644 --- a/controller/shoot/shoot_controller_test.go +++ b/controller/shoot/shoot_controller_test.go @@ -6,7 +6,6 @@ package shoot_test import ( "context" "encoding/base64" - "time" "shoot-grafter/api/v1alpha1" "shoot-grafter/controller/shoot" @@ -31,11 +30,11 @@ import ( ) var ( - careInstruction *v1alpha1.CareInstruction - mgrCtx context.Context - mgrCancel context.CancelFunc - greenhouseMgrCancel context.CancelFunc + careInstruction *v1alpha1.CareInstruction + mgrCtx context.Context + mgrCancel context.CancelFunc ) + var _ = Describe("Shoot Controller", func() { JustBeforeEach(func() { // register controllers in JustBeforeEach, as they depend on the CareInstruction. @@ -65,27 +64,40 @@ var _ = Describe("Shoot Controller", func() { }) Expect(err).NotTo(HaveOccurred(), "there must be no error creating the garden manager") - // Create a manager for the Greenhouse cluster (where events should be emitted) + // Set up the ShootController with the garden manager. + // If the CareInstruction references an auth ConfigMap, fetch its data so configureOIDCAuthentication + // can use it (in production this is passed in-memory by the CareInstruction controller). + var authConfigMapData map[string]string + if careInstruction.Spec.AuthenticationConfigMapName != "" { + var authCM corev1.ConfigMap + if err := test.K8sClient.Get(test.Ctx, client.ObjectKey{ + Namespace: careInstruction.Namespace, + Name: careInstruction.Spec.AuthenticationConfigMapName, + }, &authCM); err == nil { + authConfigMapData = authCM.Data + } + } + + // Build an event recorder backed by the Greenhouse cluster so events are emitted there. + // The garden manager's recorder would emit to the Garden cluster, but tests verify events + // on the Greenhouse cluster (test.K8sClient). greenhouseMgr, err := ctrl.NewManager(test.Cfg, ctrl.Options{ Scheme: scheme.Scheme, Metrics: server.Options{ - BindAddress: "0", // Disable metrics + BindAddress: "0", }, LeaderElection: false, }) - Expect(err).NotTo(HaveOccurred(), "there must be no error creating the greenhouse manager") + Expect(err).NotTo(HaveOccurred(), "there must be no error creating the greenhouse manager for event recording") - // Create ShootController with EventRecorder and GreenhouseMgr from the Greenhouse manager. - // GreenhouseMgr is required so the ShootController can watch Greenhouse auth CMs and - // re-enqueue Shoots when they change. Expect((&shoot.ShootController{ - GreenhouseClient: test.K8sClient, - GreenhouseMgr: greenhouseMgr, // Provide Greenhouse manager for cross-cluster CM watch - GardenClient: test.GardenK8sClient, - Logger: ctrl.Log.WithName("controllers").WithName("ShootController"), - Name: "ShootController", - CareInstruction: careInstruction, - EventRecorder: greenhouseMgr.GetEventRecorderFor("ShootController"), // Get EventRecorder from Greenhouse manager + GreenhouseClient: test.K8sClient, + GardenClient: test.GardenK8sClient, + Logger: ctrl.Log.WithName("controllers").WithName("ShootController"), + Name: "ShootController", + CareInstruction: careInstruction, + EventRecorder: greenhouseMgr.GetEventRecorderFor("ShootController"), + AuthConfigMapData: authConfigMapData, }).SetupWithManager(mgr)).To(Succeed(), "there must be no error setting up the controller with the manager") careInstructionWebhook := &webhookv1alpha1.CareInstructionWebhook{} @@ -93,21 +105,11 @@ var _ = Describe("Shoot Controller", func() { mgrCtx, mgrCancel = context.WithCancel(test.Ctx) - // start the Greenhouse manager so its cache is populated for the auth CM watch - ghCtx, ghCancel := context.WithCancel(test.Ctx) - greenhouseMgrCancel = ghCancel + // start both managers go func() { defer GinkgoRecover() - Expect(greenhouseMgr.Start(ghCtx)).To(Succeed(), "there must be no error starting the greenhouse manager") + Expect(greenhouseMgr.Start(mgrCtx)).To(Succeed()) }() - - // Wait for the Greenhouse cache to sync before proceeding so the auth CM watch is established. - syncCtx, syncCancel := context.WithTimeout(ghCtx, 30*time.Second) - defer syncCancel() - Expect(greenhouseMgr.GetCache().WaitForCacheSync(syncCtx)).To(BeTrue(), - "the greenhouse manager cache must be synced before running watch-dependent assertions") - - // start the garden manager go func() { defer GinkgoRecover() Expect(mgr.Start(mgrCtx)).To(Succeed(), "there must be no error starting the manager") @@ -176,24 +178,20 @@ var _ = Describe("Shoot Controller", func() { return len(configMaps.Items) == 0 // Only the garden cluster ConfigMap should remain }).Should(BeTrue(), "should eventually not find ConfigMap resources") - // Clean up auth ConfigMaps in Greenhouse cluster using label selector - greenhouseAuthConfigMaps := &corev1.ConfigMapList{} - Expect(test.K8sClient.List(test.Ctx, greenhouseAuthConfigMaps, client.MatchingLabels{ - v1alpha1.AuthConfigMapLabel: "true", - })).To(Succeed(), "should list auth ConfigMaps in Greenhouse cluster") - for _, configMap := range greenhouseAuthConfigMaps.Items { - Expect(client.IgnoreNotFound(test.K8sClient.Delete(test.Ctx, &configMap))).To(Succeed(), "should delete auth ConfigMap resource") + // Clean up all ConfigMaps created during tests in the Greenhouse cluster + greenhouseConfigMaps := &corev1.ConfigMapList{} + Expect(test.K8sClient.List(test.Ctx, greenhouseConfigMaps, client.InNamespace("default"))).To(Succeed(), "should list ConfigMaps in Greenhouse cluster") + for _, configMap := range greenhouseConfigMaps.Items { + Expect(client.IgnoreNotFound(test.K8sClient.Delete(test.Ctx, &configMap))).To(Succeed(), "should delete ConfigMap resource") } Eventually(func(g Gomega) bool { - greenhouseAuthConfigMaps := &corev1.ConfigMapList{} - err := test.K8sClient.List(test.Ctx, greenhouseAuthConfigMaps, client.MatchingLabels{ - v1alpha1.AuthConfigMapLabel: "true", - }) + greenhouseConfigMaps := &corev1.ConfigMapList{} + err := test.K8sClient.List(test.Ctx, greenhouseConfigMaps, client.InNamespace("default")) if err != nil { return false } - return len(greenhouseAuthConfigMaps.Items) == 0 - }).Should(BeTrue(), "should eventually not find auth ConfigMap resources in Greenhouse cluster") + return len(greenhouseConfigMaps.Items) == 0 + }).Should(BeTrue(), "should eventually not find ConfigMap resources in Greenhouse cluster") // Clean up any Events created during the tests events := &corev1.EventList{} @@ -210,12 +208,7 @@ var _ = Describe("Shoot Controller", func() { return len(events.Items) == 0 }).Should(BeTrue(), "should eventually not find Event resources") - // stop both managers mgrCancel() - if greenhouseMgrCancel != nil { - greenhouseMgrCancel() - greenhouseMgrCancel = nil - } }) @@ -1778,165 +1771,6 @@ jwt: }) }) - When("testing OIDC configuration watch triggering reconciliation", func() { - var greenhouseAuthConfigMap *corev1.ConfigMap - - BeforeEach(func() { - careInstruction = &v1alpha1.CareInstruction{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-careinstruction-watch", - Namespace: "default", - }, - Spec: v1alpha1.CareInstructionSpec{ - ShootSelector: &v1alpha1.ShootSelector{ - LabelSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "test": "watch", - }, - }, - }, - AuthenticationConfigMapName: "greenhouse-auth-config-watch", - }, - } - - // Create the Greenhouse AuthenticationConfiguration ConfigMap with initial OIDC config. - // The watch predicate requires both AuthConfigMapLabel and CareInstructionLabel. - // The controller will add CareInstructionLabel on first reconciliation; we pre-set - // AuthConfigMapLabel here so the CM is already identifiable as an auth CM. - greenhouseAuthConfigMap = &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "greenhouse-auth-config-watch", - Namespace: "default", - Labels: map[string]string{ - v1alpha1.AuthConfigMapLabel: "true", - }, - }, - Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 -kind: AuthenticationConfiguration -jwt: -- issuer: - url: https://greenhouse-watch.test.example.com - audiences: - - audience-v1 - claimMappings: - username: - claim: sub - prefix: 'greenhouse-v1:' -`, - }, - } - Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthConfigMap)).To(Succeed(), "should create Greenhouse auth ConfigMap") - }) - - It("should re-reconcile shoots when Greenhouse auth CM is updated", func() { - // Create a shoot that matches the selector - shoot := &gardenerv1beta1.Shoot{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-shoot-watch", - Namespace: "default", - Labels: map[string]string{ - "test": "watch", - }, - }, - } - Expect(test.GardenK8sClient.Create(test.Ctx, shoot)).To(Succeed(), "should create Shoot resource") - - shoot.Status = gardenerv1beta1.ShootStatus{ - AdvertisedAddresses: []gardenerv1beta1.ShootAdvertisedAddress{ - { - Name: "external", - URL: "https://api-server.test-shoot-watch.example.com", - }, - }, - } - Expect(test.GardenK8sClient.Status().Update(test.Ctx, shoot)).To(Succeed(), "should update Shoot status") - - // Create CA ConfigMap - caCM := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-shoot-watch.ca-cluster", - Namespace: "default", - }, - Data: map[string]string{"ca.crt": "test-ca-data"}, - } - Expect(test.GardenK8sClient.Create(test.Ctx, caCM)).To(Succeed(), "should create CA ConfigMap") - - gardenAuthCMName := "test-careinstruction-watch-greenhouse-auth" - - // Step 1: wait for the initial Garden auth CM to be created with the v1 audience - Eventually(func(g Gomega) { - authCM := &corev1.ConfigMap{} - g.Expect(test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ - Name: gardenAuthCMName, - Namespace: "default", - }, authCM)).To(Succeed(), "should find Garden auth ConfigMap") - - var authConfig apiserverv1beta1.AuthenticationConfiguration - g.Expect(yaml.Unmarshal([]byte(authCM.Data["config.yaml"]), &authConfig)).To(Succeed()) - g.Expect(authConfig.JWT).To(HaveLen(1)) - g.Expect(authConfig.JWT[0].Issuer.URL).To(Equal("https://greenhouse-watch.test.example.com")) - g.Expect(authConfig.JWT[0].Issuer.Audiences).To(ConsistOf("audience-v1")) - }).Should(Succeed(), "initial Garden auth CM should contain v1 audience") - - // Step 2: wait for the controller to label the Greenhouse auth CM. - // This adds CareInstructionLabel, enabling the watch predicate to match future updates. - Eventually(func(g Gomega) { - var ghCM corev1.ConfigMap - g.Expect(test.K8sClient.Get(test.Ctx, client.ObjectKey{ - Name: "greenhouse-auth-config-watch", - Namespace: "default", - }, &ghCM)).To(Succeed()) - g.Expect(ghCM.Labels).To(HaveKeyWithValue(v1alpha1.CareInstructionLabel, careInstruction.Name), - "controller should have added CareInstructionLabel to Greenhouse auth CM") - }).Should(Succeed(), "Greenhouse auth CM should be labeled with CareInstructionLabel before proceeding") - - // Step 3: update the Greenhouse auth CM — same issuer URL, but new audience and prefix. - // Keeping the same URL ensures mergeAuthenticationConfigurations replaces the existing - // entry in-place rather than appending a second issuer. The watch (source.Kind on the - // Greenhouse cache) fires because the CM has both required labels and its content changed. - Expect(test.K8sClient.Get(test.Ctx, client.ObjectKey{ - Name: "greenhouse-auth-config-watch", - Namespace: "default", - }, greenhouseAuthConfigMap)).To(Succeed(), "should fetch latest Greenhouse auth CM") - greenhouseAuthConfigMap.Data["config.yaml"] = `apiVersion: apiserver.config.k8s.io/v1beta1 -kind: AuthenticationConfiguration -jwt: -- issuer: - url: https://greenhouse-watch.test.example.com - audiences: - - audience-v2 - claimMappings: - username: - claim: sub - prefix: 'greenhouse-v2:' -` - Expect(test.K8sClient.Update(test.Ctx, greenhouseAuthConfigMap)).To(Succeed(), - "should update Greenhouse auth CM to trigger watch-based reconciliation") - - // Step 4: the watch fires -> shoots are re-enqueued -> controller reconciles -> - // Garden auth CM is updated in-place to reflect the new audience (v2). - // The issuer count stays at 1 because the URL matches and the entry is replaced. - Eventually(func(g Gomega) { - authCM := &corev1.ConfigMap{} - g.Expect(test.GardenK8sClient.Get(test.Ctx, client.ObjectKey{ - Name: gardenAuthCMName, - Namespace: "default", - }, authCM)).To(Succeed()) - - var authConfig apiserverv1beta1.AuthenticationConfiguration - g.Expect(yaml.Unmarshal([]byte(authCM.Data["config.yaml"]), &authConfig)).To(Succeed()) - g.Expect(authConfig.JWT).To(HaveLen(1), "should still have exactly one issuer (replaced in-place)") - g.Expect(authConfig.JWT[0].Issuer.URL).To(Equal("https://greenhouse-watch.test.example.com")) - // audience-v2 proves the Garden CM was re-reconciled from the updated Greenhouse CM - g.Expect(authConfig.JWT[0].Issuer.Audiences).To(ConsistOf("audience-v2"), - "Garden auth CM should have updated audience after Greenhouse CM change triggers watch") - g.Expect(authConfig.JWT[0].ClaimMappings.Username.Prefix).NotTo(BeNil()) - g.Expect(*authConfig.JWT[0].ClaimMappings.Username.Prefix).To(Equal("greenhouse-v2:")) - }).Should(Succeed(), "Garden auth CM should be re-reconciled to v2 config after Greenhouse CM change triggers watch") - }) - }) - When("using ShootSelector.Expression with CEL expression", func() { BeforeEach(func() { careInstruction = &v1alpha1.CareInstruction{ @@ -2127,10 +1961,9 @@ jwt: AuthenticationConfigMapName: "greenhouse-oidc-config", }, } - }) - It("should NOT annotate Shoot when setting up OIDC for the first time", func() { - // Create Greenhouse auth ConfigMap + // Create Greenhouse auth ConfigMap in BeforeEach so JustBeforeEach can fetch its data + // for AuthConfigMapData (which is passed in-memory to the ShootController). greenhouseAuthCM := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "greenhouse-oidc-config", @@ -2152,7 +1985,9 @@ jwt: }, } Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthCM)).To(Succeed(), "should create Greenhouse auth ConfigMap") + }) + It("should NOT annotate Shoot when setting up OIDC for the first time", func() { // Create a shoot without any OIDC configuration shoot := &gardenerv1beta1.Shoot{ ObjectMeta: metav1.ObjectMeta{ @@ -2215,28 +2050,7 @@ jwt: }) It("should annotate Shoot with gardener.cloud/operation=reconcile when ConfigMap content changes", func() { - // Create Greenhouse auth ConfigMap - greenhouseAuthCM := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "greenhouse-oidc-config", - Namespace: "default", - }, - Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 -kind: AuthenticationConfiguration -jwt: -- issuer: - url: https://greenhouse.example.com - audiences: - - greenhouse - claimMappings: - username: - claim: sub - prefix: 'greenhouse:' -`, - }, - } - Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthCM)).To(Succeed(), "should create Greenhouse auth ConfigMap") + // Note: the Greenhouse auth ConfigMap was created in BeforeEach. // Create a shoot that ALREADY has the OIDC ConfigMap reference shoot := &gardenerv1beta1.Shoot{ From 6d1192a35a1a8cba17d981d68e2a49898290fa8f Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 12 May 2026 11:57:06 +0200 Subject: [PATCH 17/20] fix goconst issues On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/careinstruction/metrics.go | 36 ++++++++++++++------------- controller/shoot/auth.go | 14 ++++++----- controller/shoot/auth_test.go | 28 ++++++++++----------- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/controller/careinstruction/metrics.go b/controller/careinstruction/metrics.go index 91627987..9079353f 100644 --- a/controller/careinstruction/metrics.go +++ b/controller/careinstruction/metrics.go @@ -10,34 +10,36 @@ import ( "shoot-grafter/api/v1alpha1" ) +const labelCareInstruction = "care_instruction" + var ( TotalTargetShootsGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_total_target_shoots", Help: "Total number of shoots matching the CareInstruction label selector", }, - []string{"care_instruction", "namespace", "garden_namespace"}, + []string{labelCareInstruction, "namespace", "garden_namespace"}, ) CreatedClustersGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_created_clusters", Help: "Number of clusters created by the CareInstruction", }, - []string{"care_instruction", "namespace", "garden_namespace"}, + []string{labelCareInstruction, "namespace", "garden_namespace"}, ) FailedClustersGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_failed_clusters", Help: "Number of clusters failed to be created by the CareInstruction", }, - []string{"care_instruction", "namespace", "garden_namespace"}, + []string{labelCareInstruction, "namespace", "garden_namespace"}, ) ShootOnboardedGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_shoot_onboarded", Help: "Is shoot onboarded by the CareInstruction", }, - []string{"care_instruction", "namespace", "garden_namespace", "shoot_name"}, + []string{labelCareInstruction, "namespace", "garden_namespace", "shoot_name"}, ) ) @@ -59,9 +61,9 @@ func UpdateCareInstructionMetrics(careInstruction *v1alpha1.CareInstruction) { func updateTotalTargetShootsMetric(careInstruction *v1alpha1.CareInstruction) { metricLabels := prometheus.Labels{ - "care_instruction": careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, + labelCareInstruction: careInstruction.Name, + "namespace": careInstruction.Namespace, + "garden_namespace": careInstruction.Spec.GardenNamespace, } totalTargetShoots := careInstruction.Status.TotalTargetShoots TotalTargetShootsGauge.With(metricLabels).Set(float64(totalTargetShoots)) @@ -69,9 +71,9 @@ func updateTotalTargetShootsMetric(careInstruction *v1alpha1.CareInstruction) { func updateCreatedClustersMetric(careInstruction *v1alpha1.CareInstruction) { metricLabels := prometheus.Labels{ - "care_instruction": careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, + labelCareInstruction: careInstruction.Name, + "namespace": careInstruction.Namespace, + "garden_namespace": careInstruction.Spec.GardenNamespace, } createdCount := careInstruction.Status.CreatedClusters CreatedClustersGauge.With(metricLabels).Set(float64(createdCount)) @@ -79,9 +81,9 @@ func updateCreatedClustersMetric(careInstruction *v1alpha1.CareInstruction) { func updateFailedClustersMetric(careInstruction *v1alpha1.CareInstruction) { metricLabels := prometheus.Labels{ - "care_instruction": careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, + labelCareInstruction: careInstruction.Name, + "namespace": careInstruction.Namespace, + "garden_namespace": careInstruction.Spec.GardenNamespace, } failedCount := careInstruction.Status.FailedClusters FailedClustersGauge.With(metricLabels).Set(float64(failedCount)) @@ -90,10 +92,10 @@ func updateFailedClustersMetric(careInstruction *v1alpha1.CareInstruction) { func updateOnboardedShootsMetrics(careInstruction *v1alpha1.CareInstruction) { for _, ss := range careInstruction.Status.Shoots { metricLabels := prometheus.Labels{ - "care_instruction": careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, - "shoot_name": ss.Name, + labelCareInstruction: careInstruction.Name, + "namespace": careInstruction.Namespace, + "garden_namespace": careInstruction.Spec.GardenNamespace, + "shoot_name": ss.Name, } if ss.Status == v1alpha1.ShootStatusOnboarded { ShootOnboardedGauge.With(metricLabels).Set(float64(1)) diff --git a/controller/shoot/auth.go b/controller/shoot/auth.go index 366bafdf..2e2e2d1c 100644 --- a/controller/shoot/auth.go +++ b/controller/shoot/auth.go @@ -19,6 +19,8 @@ import ( "sigs.k8s.io/yaml" ) +const authConfigKey = "config.yaml" + // configureOIDCAuthentication configures OIDC authentication for the Shoot by: // 1. Reading the AuthenticationConfiguration from the in-memory auth ConfigMap data (provided by the CareInstruction controller) // 2. Merging it with any existing configuration on the Garden cluster @@ -27,7 +29,7 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot // Use the in-memory auth ConfigMap data provided by the CareInstruction controller. // This avoids a cross-cluster watch; the CareInstruction controller is responsible for // fetching and caching the data, and restarts this ShootController when it changes. - if r.AuthConfigMapData == nil || r.AuthConfigMapData["config.yaml"] == "" { + if r.AuthConfigMapData == nil || r.AuthConfigMapData[authConfigKey] == "" { return fmt.Errorf("AuthenticationConfiguration ConfigMap %s does not contain config.yaml (data not available)", r.CareInstruction.Spec.AuthenticationConfigMapName) } @@ -73,7 +75,7 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot // Parse the Greenhouse authentication configuration from in-memory data var greenhouseAuthConfig apiserverv1beta1.AuthenticationConfiguration - if err := yaml.Unmarshal([]byte(r.AuthConfigMapData["config.yaml"]), &greenhouseAuthConfig); err != nil { + if err := yaml.Unmarshal([]byte(r.AuthConfigMapData[authConfigKey]), &greenhouseAuthConfig); err != nil { return fmt.Errorf("failed to parse Greenhouse AuthenticationConfiguration: %w", err) } @@ -109,7 +111,7 @@ func (r *ShootController) configureOIDCAuthentication(ctx context.Context, shoot Namespace: shoot.Namespace, }, Data: map[string]string{ - "config.yaml": "", + authConfigKey: "", }, } } @@ -181,8 +183,8 @@ func (r *ShootController) mergeAuthenticationConfigurations(gardenConfigMap *cor var gardenAuthConfig apiserverv1beta1.AuthenticationConfiguration // Parse existing Garden configuration if present - if gardenConfigMap.Data != nil && gardenConfigMap.Data["config.yaml"] != "" { - existingConfigYAML := gardenConfigMap.Data["config.yaml"] + if gardenConfigMap.Data != nil && gardenConfigMap.Data[authConfigKey] != "" { + existingConfigYAML := gardenConfigMap.Data[authConfigKey] if err := yaml.Unmarshal([]byte(existingConfigYAML), &gardenAuthConfig); err != nil { return fmt.Errorf("failed to parse existing Garden AuthenticationConfiguration: %w", err) } @@ -231,7 +233,7 @@ func (r *ShootController) mergeAuthenticationConfigurations(gardenConfigMap *cor if gardenConfigMap.Data == nil { gardenConfigMap.Data = make(map[string]string) } - gardenConfigMap.Data["config.yaml"] = string(mergedConfigYAML) + gardenConfigMap.Data[authConfigKey] = string(mergedConfigYAML) return nil } diff --git a/controller/shoot/auth_test.go b/controller/shoot/auth_test.go index 9f9f8ce8..10607e1b 100644 --- a/controller/shoot/auth_test.go +++ b/controller/shoot/auth_test.go @@ -41,23 +41,23 @@ var _ = Describe("Auth", func() { // Verify ConfigMap data was updated Expect(initialGardenConfigMap.Data).NotTo(BeNil()) - Expect(initialGardenConfigMap.Data).To(HaveKey("config.yaml")) + Expect(initialGardenConfigMap.Data).To(HaveKey(authConfigKey)) - // Verify other data keys are preserved (keys that are not "config.yaml") + // Verify other data keys are preserved (keys that are not authConfigKey) for key, value := range expectedConfigMap.Data { - if key != "config.yaml" { + if key != authConfigKey { Expect(initialGardenConfigMap.Data).To(HaveKeyWithValue(key, value)) } } // Unmarshal the actual result var actualConfig apiserverv1beta1.AuthenticationConfiguration - err = yaml.Unmarshal([]byte(initialGardenConfigMap.Data["config.yaml"]), &actualConfig) + err = yaml.Unmarshal([]byte(initialGardenConfigMap.Data[authConfigKey]), &actualConfig) Expect(err).NotTo(HaveOccurred()) // Unmarshal the expected configuration var expectedConfig apiserverv1beta1.AuthenticationConfiguration - err = yaml.Unmarshal([]byte(expectedConfigMap.Data["config.yaml"]), &expectedConfig) + err = yaml.Unmarshal([]byte(expectedConfigMap.Data[authConfigKey]), &expectedConfig) Expect(err).NotTo(HaveOccurred()) // Compare configurations @@ -101,7 +101,7 @@ var _ = Describe("Auth", func() { }, &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -142,7 +142,7 @@ jwt: Data: map[string]string{ "other-key": "other-value", "another-key": "another-value", - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -159,7 +159,7 @@ jwt: Entry("with Garden ConfigMap having one different issuer (should add Greenhouse issuer)", &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -190,7 +190,7 @@ jwt: }, &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -214,7 +214,7 @@ jwt: Entry("with Garden ConfigMap having the same issuer (Greenhouse should update it)", &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -245,7 +245,7 @@ jwt: }, &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -262,7 +262,7 @@ jwt: Entry("with multiple Greenhouse issuers and multiple Garden issuers", &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -319,7 +319,7 @@ jwt: }, &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1 + authConfigKey: `apiVersion: apiserver.config.k8s.io/v1beta1 kind: AuthenticationConfiguration jwt: - issuer: @@ -360,7 +360,7 @@ jwt: It("should return error for invalid YAML in Garden ConfigMap", func() { configMap := &corev1.ConfigMap{ Data: map[string]string{ - "config.yaml": "invalid: yaml: content: [", + authConfigKey: "invalid: yaml: content: [", }, } From a18aa613a10d64b3ea850e250e791a366b36c08a Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 12 May 2026 13:22:10 +0200 Subject: [PATCH 18/20] fix more goconst issues On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- controller/careinstruction/metrics.go | 33 +++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/controller/careinstruction/metrics.go b/controller/careinstruction/metrics.go index 9079353f..c43e742a 100644 --- a/controller/careinstruction/metrics.go +++ b/controller/careinstruction/metrics.go @@ -10,7 +10,12 @@ import ( "shoot-grafter/api/v1alpha1" ) -const labelCareInstruction = "care_instruction" +const ( + labelCareInstruction = "care_instruction" + labelNamespace = "namespace" + labelGardenNamespace = "garden_namespace" + labelShootName = "shoot_name" +) var ( TotalTargetShootsGauge = prometheus.NewGaugeVec( @@ -18,28 +23,28 @@ var ( Name: "shoot_grafter_total_target_shoots", Help: "Total number of shoots matching the CareInstruction label selector", }, - []string{labelCareInstruction, "namespace", "garden_namespace"}, + []string{labelCareInstruction, labelNamespace, labelGardenNamespace}, ) CreatedClustersGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_created_clusters", Help: "Number of clusters created by the CareInstruction", }, - []string{labelCareInstruction, "namespace", "garden_namespace"}, + []string{labelCareInstruction, labelNamespace, labelGardenNamespace}, ) FailedClustersGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_failed_clusters", Help: "Number of clusters failed to be created by the CareInstruction", }, - []string{labelCareInstruction, "namespace", "garden_namespace"}, + []string{labelCareInstruction, labelNamespace, labelGardenNamespace}, ) ShootOnboardedGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "shoot_grafter_shoot_onboarded", Help: "Is shoot onboarded by the CareInstruction", }, - []string{labelCareInstruction, "namespace", "garden_namespace", "shoot_name"}, + []string{labelCareInstruction, labelNamespace, labelGardenNamespace, labelShootName}, ) ) @@ -62,8 +67,8 @@ func UpdateCareInstructionMetrics(careInstruction *v1alpha1.CareInstruction) { func updateTotalTargetShootsMetric(careInstruction *v1alpha1.CareInstruction) { metricLabels := prometheus.Labels{ labelCareInstruction: careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, + labelNamespace: careInstruction.Namespace, + labelGardenNamespace: careInstruction.Spec.GardenNamespace, } totalTargetShoots := careInstruction.Status.TotalTargetShoots TotalTargetShootsGauge.With(metricLabels).Set(float64(totalTargetShoots)) @@ -72,8 +77,8 @@ func updateTotalTargetShootsMetric(careInstruction *v1alpha1.CareInstruction) { func updateCreatedClustersMetric(careInstruction *v1alpha1.CareInstruction) { metricLabels := prometheus.Labels{ labelCareInstruction: careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, + labelNamespace: careInstruction.Namespace, + labelGardenNamespace: careInstruction.Spec.GardenNamespace, } createdCount := careInstruction.Status.CreatedClusters CreatedClustersGauge.With(metricLabels).Set(float64(createdCount)) @@ -82,8 +87,8 @@ func updateCreatedClustersMetric(careInstruction *v1alpha1.CareInstruction) { func updateFailedClustersMetric(careInstruction *v1alpha1.CareInstruction) { metricLabels := prometheus.Labels{ labelCareInstruction: careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, + labelNamespace: careInstruction.Namespace, + labelGardenNamespace: careInstruction.Spec.GardenNamespace, } failedCount := careInstruction.Status.FailedClusters FailedClustersGauge.With(metricLabels).Set(float64(failedCount)) @@ -93,9 +98,9 @@ func updateOnboardedShootsMetrics(careInstruction *v1alpha1.CareInstruction) { for _, ss := range careInstruction.Status.Shoots { metricLabels := prometheus.Labels{ labelCareInstruction: careInstruction.Name, - "namespace": careInstruction.Namespace, - "garden_namespace": careInstruction.Spec.GardenNamespace, - "shoot_name": ss.Name, + labelNamespace: careInstruction.Namespace, + labelGardenNamespace: careInstruction.Spec.GardenNamespace, + labelShootName: ss.Name, } if ss.Status == v1alpha1.ShootStatusOnboarded { ShootOnboardedGauge.With(metricLabels).Set(float64(1)) From a15cbb9963d50c6d4376256cb26c5bc646b94fd7 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 12 May 2026 14:12:20 +0200 Subject: [PATCH 19/20] update go version to 1.26.3 and run go-makefile-maker On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .github/workflows/checks.yaml | 4 +--- .github/workflows/ci.yaml | 4 ++-- .github/workflows/codeql.yaml | 2 +- Makefile | 8 ++++---- Makefile.maker.yaml | 1 - 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index ef8d9c3d..15ce654f 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -29,7 +29,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.26.2 + go-version: 1.26.3 - name: Run golangci-lint uses: golangci/golangci-lint-action@v9 with: @@ -44,8 +44,6 @@ jobs: uses: crate-ci/typos@v1 env: CLICOLOR: "1" - - name: Delete typos binary - run: rm -f typos - name: Check if source code files have license header run: make check-addlicense - name: REUSE Compliance Check diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2f027c66..60596660 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.26.2 + go-version: 1.26.3 - name: Build all binaries run: make build-all code_coverage: @@ -65,7 +65,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.26.2 + go-version: 1.26.3 - name: Run tests and generate coverage report run: make test-with-envtest - name: Archive code coverage results diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml index b0bb5d38..0ea19110 100644 --- a/.github/workflows/codeql.yaml +++ b/.github/workflows/codeql.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.26.2 + go-version: 1.26.3 - name: Initialize CodeQL uses: github/codeql-action/init@v4 with: diff --git a/Makefile b/Makefile index 66945885..55383b7a 100644 --- a/Makefile +++ b/Makefile @@ -61,10 +61,10 @@ prepare-static-check: FORCE install-goimports install-golangci-lint install-shel # To add additional flags or values (before the default ones), specify the variable in the environment, e.g. `GO_BUILDFLAGS='-tags experimental' make`. # To override the default flags or values, specify the variable on the command line, e.g. `make GO_BUILDFLAGS='-tags experimental'`. GO_BUILDFLAGS += -GO_LDFLAGS += -GO_TESTFLAGS += -GO_TESTENV += -GO_BUILDENV += +GO_LDFLAGS += +GO_TESTFLAGS += +GO_TESTENV += +GO_BUILDENV += build-all: build/shoot-grafter diff --git a/Makefile.maker.yaml b/Makefile.maker.yaml index 2d904add..0009e4d2 100644 --- a/Makefile.maker.yaml +++ b/Makefile.maker.yaml @@ -6,7 +6,6 @@ # NOTE: After running go-makefile-maker, manually apply these changes: # 1. Add 'branches: [main]' to container-registry-ghcr.yaml for main branch builds # 2. Change 'make build/cover.out' to 'make test-with-envtest' in ci.yaml for envtest support -# 3. Change 'rm typos' to 'rm -f typos' in checks.yaml metadata: url: https://github.com/cloudoperators/shoot-grafter From 09d36cc782b5042d85b7ae2be88dcbbb0ac52ebc Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 12 May 2026 14:52:04 +0200 Subject: [PATCH 20/20] update golang dependencies On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- go.mod | 14 +++++++------- go.sum | 32 ++++++++++++++++---------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index e8d83f14..d9d89b5f 100644 --- a/go.mod +++ b/go.mod @@ -87,15 +87,15 @@ require ( go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 // indirect - golang.org/x/mod v0.31.0 // indirect - golang.org/x/net v0.48.0 // indirect + golang.org/x/mod v0.34.0 // indirect + golang.org/x/net v0.53.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect - golang.org/x/sync v0.19.0 // indirect - golang.org/x/sys v0.39.0 // indirect - golang.org/x/term v0.38.0 // indirect - golang.org/x/text v0.32.0 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/sys v0.43.0 // indirect + golang.org/x/term v0.42.0 // indirect + golang.org/x/text v0.36.0 // indirect golang.org/x/time v0.14.0 // indirect - golang.org/x/tools v0.40.0 // indirect + golang.org/x/tools v0.43.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8 // indirect diff --git a/go.sum b/go.sum index cac3d2c4..b9de7aed 100644 --- a/go.sum +++ b/go.sum @@ -367,46 +367,46 @@ go.yaml.in/yaml/v4 v4.0.0-rc.2/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfP golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= -golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0= +golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= +golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 h1:fQsdNF2N+/YewlRZiricy4P1iimyPKZ/xwniHj8Q2a0= golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93/go.mod h1:EPRbTFwzwjXj9NpYyyrvenVh9Y+GFeEvMNh7Xuz7xgU= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.31.0 h1:HaW9xtz0+kOcWKwli0ZXy79Ix+UW/vOfmWI5QVd2tgI= -golang.org/x/mod v0.31.0/go.mod h1:43JraMp9cGx1Rx3AqioxrbrhNsLl2l/iNAvuBkrezpg= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.48.0 h1:zyQRTTrjc33Lhh0fBgT/H3oZq9WuvRR5gPC70xpDiQU= -golang.org/x/net v0.48.0/go.mod h1:+ndRgGjkh8FGtu1w1FGbEC31if4VrNVMuKTgcAAnQRY= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= -golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/term v0.38.0 h1:PQ5pkm/rLO6HnxFR7N2lJHOZX6Kez5Y1gDSJla6jo7Q= -golang.org/x/term v0.38.0/go.mod h1:bSEAKrOT1W+VSu9TSCMtoGEOUcKxOKgl3LE5QEF/xVg= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.42.0 h1:UiKe+zDFmJobeJ5ggPwOshJIVt6/Ft0rcfrXZDLWAWY= +golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= -golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= +golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.40.0 h1:yLkxfA+Qnul4cs9QA3KnlFu0lVmd8JJfoq+E41uSutA= -golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc= +golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=