refactor: update namespace sync configuration to use label selector

- Changed `targetNamespaceLabelKey` to `targetNamespaceLabelSelector` in `values.yaml` for clarity.
- Updated deployment template to reflect the new label selector usage.
- Modified main application logic to validate and parse the label selector.
- Removed deprecated `IsTargetNamespace` function in favor of a more flexible label selector approach.
- Adjusted related tests and documentation accordingly.

Author: jimmidyson

charts/cluster-api-runtime-extensions-nutanix/README.md

@@ -109,7 +109,7 @@ A Helm chart for cluster-api-runtime-extensions-nutanix
 | imagePullSecrets | list | `[]` | Optional secrets used for pulling the container image |
 | namespaceSync.enabled | bool | `true` |  |
 | namespaceSync.sourceNamespace | string | `""` |  |
-| namespaceSync.targetNamespaceLabelKey | string | `"caren.nutanix.com/namespace-sync"` |  |
+| namespaceSync.targetNamespaceLabelSelector | string | `"caren.nutanix.com/namespace-sync"` |  |
 | nodeSelector | object | `{}` |  |
 | priorityClassName | string | `"system-cluster-critical"` | Priority class to be used for the pod. |
 | resources.limits.cpu | string | `"100m"` |  |

charts/cluster-api-runtime-extensions-nutanix/templates/deployment.yaml

@@ -32,7 +32,7 @@ spec:
         - --defaults-namespace=$(POD_NAMESPACE)
         - --namespacesync-enabled={{ .Values.namespaceSync.enabled }}
         - --namespacesync-source-namespace={{ default .Release.Namespace .Values.namespaceSync.sourceNamespace }}
-        - --namespacesync-target-namespace-label-key={{ .Values.namespaceSync.targetNamespaceLabelKey }}
+        - --namespacesync-target-namespace-label-selector={{ .Values.namespaceSync.targetNamespaceLabelSelector }}
         - --enforce-clusterautoscaler-limits-enabled={{ .Values.enforceClusterAutoscalerLimits.enabled }}
         - --failure-domain-rollout-enabled={{ .Values.failureDomainRollout.enabled }}
         - --failure-domain-rollout-concurrency={{ .Values.failureDomainRollout.concurrency }}

charts/cluster-api-runtime-extensions-nutanix/values.schema.json

@@ -683,7 +683,7 @@
                 "sourceNamespace": {
                     "type": "string"
                 },
-                "targetNamespaceLabelKey": {
+                "targetNamespaceLabelSelector": {
                     "type": "string"
                 }
             }

charts/cluster-api-runtime-extensions-nutanix/values.yaml

@@ -143,7 +143,7 @@ deployDefaultClusterClasses: true
 # target namespace, i.e., every namespace that has a label with a matching key.
 namespaceSync:
   enabled: true
-  targetNamespaceLabelKey: caren.nutanix.com/namespace-sync
+  targetNamespaceLabelSelector: caren.nutanix.com/namespace-sync
   # By default, sourceNamespace is the helm release namespace.
   sourceNamespace: ""
 

cmd/main.go

@@ -9,6 +9,7 @@ import (
 	"os"
 
 	"github.com/spf13/pflag"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -191,10 +192,32 @@ func main() {
 
 	if namespacesyncOptions.Enabled {
 		if namespacesyncOptions.SourceNamespace == "" ||
-			namespacesyncOptions.TargetNamespaceLabelKey == "" {
+			namespacesyncOptions.TargetNamespaceLabelSelector == "" {
 			setupLog.Error(
 				nil,
-				"Namespace Sync is enabled, but source namespace and/or target namespace label key are not configured.",
+				"Namespace Sync is enabled, but source namespace and/or target namespace label selector are not configured.",
+			)
+			os.Exit(1)
+		}
+
+		targetSelector, err := metav1.ParseToLabelSelector(namespacesyncOptions.TargetNamespaceLabelSelector)
+		if err != nil {
+			setupLog.Error(
+				err,
+				"unable to parse target namespace label selector",
+				"selector",
+				namespacesyncOptions.TargetNamespaceLabelSelector,
+			)
+			os.Exit(1)
+		}
+
+		targetLabelSelector, err := metav1.LabelSelectorAsSelector(targetSelector)
+		if err != nil {
+			setupLog.Error(
+				err,
+				"unable to convert label selector",
+				"selector",
+				namespacesyncOptions.TargetNamespaceLabelSelector,
 			)
 			os.Exit(1)
 		}
@@ -215,7 +238,7 @@ func main() {
 			Client:                      mgr.GetClient(),
 			UnstructuredCachingClient:   unstructuredCachingClient,
 			SourceClusterClassNamespace: namespacesyncOptions.SourceNamespace,
-			IsTargetNamespace:           namespacesync.NamespaceHasLabelKey(namespacesyncOptions.TargetNamespaceLabelKey),
+			TargetNamespaceSelector:     targetLabelSelector,
 		}).SetupWithManager(
 			mgr,
 			&controller.Options{MaxConcurrentReconciles: namespacesyncOptions.Concurrency},

pkg/controllers/namespacesync/controller.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -28,16 +29,17 @@ type Reconciler struct {
 	// SourceClusterClassNamespace is the namespace from which ClusterClasses are copied.
 	SourceClusterClassNamespace string
 
-	// IsTargetNamespace determines whether ClusterClasses should be copied to a given namespace.
-	IsTargetNamespace func(ns *corev1.Namespace) bool
+	// TargetNamespaceSelector is a label selector to determine which namespaces should receive
+	// copies of ClusterClasses and Templates from the source namespace.
+	TargetNamespaceSelector labels.Selector
 }
 
 func (r *Reconciler) SetupWithManager(
 	mgr ctrl.Manager,
 	options *controller.Options,
 ) error {
-	if r.IsTargetNamespace == nil {
-		return fmt.Errorf("define IsTargetNamespace function to use controller")
+	if r.TargetNamespaceSelector == nil {
+		return fmt.Errorf("TargetNamespaceSelector must be defined to use controller")
 	}
 
 	err := ctrl.NewControllerManagedBy(mgr).
@@ -51,7 +53,7 @@ func (r *Reconciler) SetupWithManager(
 						if !ok {
 							return false
 						}
-						return r.IsTargetNamespace(ns)
+						return r.TargetNamespaceSelector.Matches(labels.Set(ns.GetLabels()))
 					},
 					UpdateFunc: func(e event.UpdateEvent) bool {
 						// Called when an object is already in the cache, and it is either updated,
@@ -66,7 +68,9 @@ func (r *Reconciler) SetupWithManager(
 						}
 						// Only reconcile the namespace if the answer to the question "Is this a
 						// target namespace?" changed from no to yes.
-						return !r.IsTargetNamespace(nsOld) && r.IsTargetNamespace(nsNew)
+						matchesOld := r.TargetNamespaceSelector.Matches(labels.Set(nsOld.GetLabels()))
+						matchesNew := r.TargetNamespaceSelector.Matches(labels.Set(nsNew.GetLabels()))
+						return !matchesOld && matchesNew
 					},
 					DeleteFunc: func(e event.DeleteEvent) bool {
 						// Ignore deletes.
@@ -93,22 +97,23 @@ func (r *Reconciler) SetupWithManager(
 
 func (r *Reconciler) clusterClassToNamespaces(ctx context.Context, o client.Object) []ctrl.Request {
 	namespaceList := &corev1.NamespaceList{}
-	err := r.Client.List(ctx, namespaceList)
+	err := r.Client.List(ctx, namespaceList, &client.ListOptions{
+		LabelSelector: r.TargetNamespaceSelector,
+	})
 	if err != nil {
 		// TODO Log the error, and record an Event.
 		return nil
 	}
 
-	rs := []ctrl.Request{}
+	// Pre-allocate slice with exact capacity since we're using label selector
+	rs := make([]ctrl.Request, 0, len(namespaceList.Items))
 	for i := range namespaceList.Items {
 		ns := &namespaceList.Items[i]
-		if r.IsTargetNamespace(ns) {
-			rs = append(rs,
-				ctrl.Request{
-					NamespacedName: client.ObjectKeyFromObject(ns),
-				},
-			)
-		}
+		rs = append(rs,
+			ctrl.Request{
+				NamespacedName: client.ObjectKeyFromObject(ns),
+			},
+		)
 	}
 	return rs
 }
@@ -130,22 +135,29 @@ func (r *Reconciler) Reconcile(
 
 	// TODO Consider running in parallel.
 	for i := range sccs {
-		scc := &sccs[i]
-		err := copyClusterClassAndTemplates(
-			ctx,
-			r.Client,
-			r.UnstructuredCachingClient,
-			scc,
-			namespace,
-		)
-		if err != nil {
-			// TODO Record an Event.
-			return ctrl.Result{}, fmt.Errorf(
-				"failed to copy source ClusterClass %s or its referenced Templates to namespace %s: %w",
-				client.ObjectKeyFromObject(scc),
+		// Check for context cancellation to prevent goroutine leaks
+		select {
+		case <-ctx.Done():
+			return ctrl.Result{}, ctx.Err()
+		default:
+			scc := &sccs[i]
+
+			err := copyClusterClassAndTemplates(
+				ctx,
+				r.Client,
+				r.UnstructuredCachingClient,
+				scc,
 				namespace,
-				err,
 			)
+			if err != nil {
+				// TODO Record an Event.
+				return ctrl.Result{}, fmt.Errorf(
+					"failed to copy source ClusterClass %s or its referenced Templates to namespace %s: %w",
+					client.ObjectKeyFromObject(scc),
+					namespace,
+					err,
+				)
+			}
 		}
 	}
 

pkg/controllers/namespacesync/copy.go

@@ -71,12 +71,6 @@ func copyObjectForCreate[T client.Object](src T, name, namespace string) T {
 	dst.SetName(name)
 	dst.SetNamespace(namespace)
 
-	// Zero out ManagedFields (clients will set them)
-	dst.SetManagedFields(nil)
-	// Zero out OwnerReferences (object is garbage-collected if
-	// owners are not in the target namespace)
-	dst.SetOwnerReferences(nil)
-
 	// Zero out fields that are ignored by the API server on create
 	dst.SetCreationTimestamp(metav1.Time{})
 	dst.SetDeletionGracePeriodSeconds(nil)

pkg/controllers/namespacesync/flags.go

@@ -8,38 +8,48 @@ import (
 )
 
 type Options struct {
-	Enabled                 bool
-	Concurrency             int
-	SourceNamespace         string
-	TargetNamespaceLabelKey string
+	Enabled                      bool
+	Concurrency                  int
+	SourceNamespace              string
+	TargetNamespaceLabelSelector string
 }
 
 func (o *Options) AddFlags(flags *pflag.FlagSet) {
-	pflag.CommandLine.BoolVar(
+	flags.BoolVar(
 		&o.Enabled,
 		"namespacesync-enabled",
 		false,
 		"Enable copying of ClusterClasses and Templates from a source namespace to one or more target namespaces.",
 	)
 
-	pflag.CommandLine.IntVar(
+	flags.IntVar(
 		&o.Concurrency,
 		"namespacesync-concurrency",
 		10,
 		"Number of target namespaces to sync concurrently.",
 	)
 
-	pflag.CommandLine.StringVar(
+	flags.StringVar(
 		&o.SourceNamespace,
 		"namespacesync-source-namespace",
 		"",
 		"Namespace from which ClusterClasses and Templates are copied.",
 	)
 
-	pflag.CommandLine.StringVar(
-		&o.TargetNamespaceLabelKey,
+	flags.StringVar(
+		&o.TargetNamespaceLabelSelector,
 		"namespacesync-target-namespace-label-key",
 		"",
 		"Label key to determine if a namespace is a target. If a namespace has a label with this key, copy ClusterClasses and Templates to it from the source namespace.", //nolint:lll // Output will be wrapped.
 	)
+	_ = flags.MarkDeprecated(
+		"namespacesync-target-namespace-label-key",
+		"use namespacesync-target-namespace-label-selector instead",
+	)
+	flags.StringVar(
+		&o.TargetNamespaceLabelSelector,
+		"namespacesync-target-namespace-label-selector",
+		"",
+		"Label selector to determine target namespaces. Namespaces matching this selector will receive copies of ClusterClasses and Templates from the source namespace. Example: 'environment=production' or 'team in (platform,infrastructure)'.", //nolint:lll // Output will be wrapped.
+	)
 }