feat: move auth ConfigMap watch from ShootController to CareInstruction controller, adapt tests

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>

Author: Zaggy21

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
+}

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)
 	}
 

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 {