Retain relevant kubernetes.io labels when syncing namespaces (#5156)

* Retain relevant kubernetes.io labels when syncing namespaces

addLabelsFromOptions previously replaced all namespace labels with the
options-provided set. Labels managed by the cluster are now preserved
during the sync.

* Fix namespace label equality check before update

Compare the computed desired state against the current namespace
labels and annotations instead of comparing raw options directly.
The previous reflect.DeepEqual check could never return true because
ns.Labels always includes kubernetes.io/metadata.name (and now also
preserved pod-security labels), causing an unnecessary updateNamespace
call on every reconcile even when no label changes were needed.

* Fix nil map panic and update addLabelsFromOptions doc comment

addLabelsFromOptions panics when ns.Labels is nil (maps.Clone returns
nil) and NamespaceLabels options are non-nil. Add the same nil guard
already present for annotations.

Update the function doc comment to reflect the actual label retention
rules: options labels, kubernetes.io/metadata.name, and existing
pod-security.kubernetes.io/* labels are preserved; pod-security labels
from options are ignored.

Author: thardeck

internal/cmd/agent/deployer/deployer.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"maps"
-	"reflect"
 	"regexp"
 	"slices"
 	"strings"
@@ -215,25 +214,28 @@ func (d *Deployer) setNamespaceLabelsAndAnnotations(ctx context.Context, bd *fle
 		return err
 	}
 
-	if reflect.DeepEqual(bd.Spec.Options.NamespaceLabels, ns.Labels) && reflect.DeepEqual(bd.Spec.Options.NamespaceAnnotations, ns.Annotations) {
-		return nil
-	}
-
+	desiredLabels := maps.Clone(ns.Labels)
 	if bd.Spec.Options.NamespaceLabels != nil {
-		addLabelsFromOptions(ns.Labels, bd.Spec.Options.NamespaceLabels)
+		if desiredLabels == nil {
+			desiredLabels = make(map[string]string)
+		}
+		addLabelsFromOptions(log.FromContext(ctx), desiredLabels, bd.Spec.Options.NamespaceLabels)
 	}
+	desiredAnnotations := maps.Clone(ns.Annotations)
 	if bd.Spec.Options.NamespaceAnnotations != nil {
-		if ns.Annotations == nil {
-			ns.Annotations = map[string]string{}
+		if desiredAnnotations == nil {
+			desiredAnnotations = make(map[string]string)
 		}
-		addAnnotationsFromOptions(ns.Annotations, bd.Spec.Options.NamespaceAnnotations)
+		addAnnotationsFromOptions(desiredAnnotations, bd.Spec.Options.NamespaceAnnotations)
 	}
-	err = d.updateNamespace(ctx, ns)
-	if err != nil {
-		return err
+
+	if maps.Equal(desiredLabels, ns.Labels) && maps.Equal(desiredAnnotations, ns.Annotations) {
+		return nil
 	}
 
-	return nil
+	ns.Labels = desiredLabels
+	ns.Annotations = desiredAnnotations
+	return d.updateNamespace(ctx, ns)
 }
 
 // updateNamespace updates a namespace resource in the cluster.
@@ -258,13 +260,28 @@ func (d *Deployer) fetchNamespace(ctx context.Context, releaseID string) (*corev
 	return ns, nil
 }
 
-// addLabelsFromOptions updates nsLabels so that it only contains all labels specified in optLabels, plus the `kubernetes.io/metadata.name` labels added by kubernetes when creating the namespace.
-func addLabelsFromOptions(nsLabels map[string]string, optLabels map[string]string) {
-	maps.Copy(nsLabels, optLabels)
+const podSecurityLabelPrefix = "pod-security.kubernetes.io/"
+
+// addLabelsFromOptions updates nsLabels to contain labels from optLabels, while preserving
+// the `kubernetes.io/metadata.name` label added by Kubernetes when creating the namespace
+// and any existing `pod-security.kubernetes.io/*` labels. Labels with the
+// `pod-security.kubernetes.io/` prefix in optLabels are ignored.
+func addLabelsFromOptions(logger logr.Logger, nsLabels map[string]string, optLabels map[string]string) {
+	for k, v := range optLabels {
+		if strings.HasPrefix(k, podSecurityLabelPrefix) {
+			logger.V(1).Info("Ignoring label from options", "label", k)
+			continue
+		}
+		nsLabels[k] = v
+	}
 
 	// Delete labels not defined in the options.
 	// Keep the `kubernetes.io/metadata.name` label as it is added by kubernetes when creating the namespace.
+	// Keep pod-security.kubernetes.io/ labels as they are managed by cluster administrators.
 	for k := range nsLabels {
+		if strings.HasPrefix(k, podSecurityLabelPrefix) {
+			continue
+		}
 		if _, ok := optLabels[k]; k != corev1.LabelMetadataName && !ok {
 			delete(nsLabels, k)
 		}

internal/cmd/agent/deployer/deployer_test.go

@@ -14,7 +14,9 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 )
 
 func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
@@ -174,6 +176,51 @@ func TestSetNamespaceLabelsAndAnnotationsError(t *testing.T) {
 	}
 }
 
+// TestSetNamespaceLabelsAndAnnotations_NoUpdateWhenAlreadyCorrect verifies that
+// updateNamespace is not called when the namespace already reflects the desired state.
+// This guards against the broken reflect.DeepEqual check that compared raw option
+// labels to ns.Labels; ns.Labels always includes kubernetes.io/metadata.name and
+// may include preserved pod-security labels, so a direct equality check never holds.
+func TestSetNamespaceLabelsAndAnnotations_NoUpdateWhenAlreadyCorrect(t *testing.T) {
+	bd := &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
+		Options: fleet.BundleDeploymentOptions{
+			NamespaceLabels:      map[string]string{"optLabel": "optValue"},
+			NamespaceAnnotations: map[string]string{"optAnn": "optValue"},
+		},
+	}}
+	ns := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "namespace",
+			Labels:      map[string]string{"kubernetes.io/metadata.name": "namespace", "optLabel": "optValue"},
+			Annotations: map[string]string{"optAnn": "optValue"},
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+
+	updateCalled := false
+	fakeClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(&ns).
+		WithInterceptorFuncs(interceptor.Funcs{
+			Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
+				updateCalled = true
+				return c.Update(ctx, obj, opts...)
+			},
+		}).
+		Build()
+
+	h := Deployer{client: fakeClient}
+	err := h.setNamespaceLabelsAndAnnotations(context.Background(), bd, "namespace/foo/bar")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if updateCalled {
+		t.Error("updateNamespace was called when namespace was already in the desired state")
+	}
+}
+
 func TestIsStateAccepted(t *testing.T) {
 	tests := []struct {
 		name     string