From ce25d1329306ac9cf41a1af2c78538af44245418 Mon Sep 17 00:00:00 2001
From: Ciara Stacke <18287516+ciarams87@users.noreply.github.com>
Date: Tue, 19 Aug 2025 15:05:44 +0100
Subject: [PATCH] fix: remove patch label/ annotation cross contamination
 (#3754)

Problem: Patching the Service object labels and annotations results in the Deployment also getting these labels and annotations, and vice versa, which is unexpected behavior.

Solution: Create separate copies of the base label and annotation maps for each object. Since maps are passed by reference in Go, modifying the labels for one object was affecting the other by updating the shared reference.
---
 internal/controller/provisioner/objects.go    | 19 +++++-
 .../controller/provisioner/objects_test.go    | 58 +++++++++++++++++++
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/internal/controller/provisioner/objects.go b/internal/controller/provisioner/objects.go
index 6abe6f230c..14b3cbcc72 100644
--- a/internal/controller/provisioner/objects.go
+++ b/internal/controller/provisioner/objects.go
@@ -152,13 +152,28 @@ func (p *NginxProvisioner) buildNginxResourceObjects(
 		ports[int32(listener.Port)] = struct{}{}
 	}
 
-	service, err := buildNginxService(objectMeta, nProxyCfg, ports, selectorLabels)
+	// Create separate copies of objectMeta for service and deployment to avoid shared map references
+	serviceObjectMeta := metav1.ObjectMeta{
+		Name:        objectMeta.Name,
+		Namespace:   objectMeta.Namespace,
+		Labels:      maps.Clone(objectMeta.Labels),
+		Annotations: maps.Clone(objectMeta.Annotations),
+	}
+
+	deploymentObjectMeta := metav1.ObjectMeta{
+		Name:        objectMeta.Name,
+		Namespace:   objectMeta.Namespace,
+		Labels:      maps.Clone(objectMeta.Labels),
+		Annotations: maps.Clone(objectMeta.Annotations),
+	}
+
+	service, err := buildNginxService(serviceObjectMeta, nProxyCfg, ports, selectorLabels)
 	if err != nil {
 		errs = append(errs, err)
 	}
 
 	deployment, err := p.buildNginxDeployment(
-		objectMeta,
+		deploymentObjectMeta,
 		nProxyCfg,
 		ngxIncludesConfigMapName,
 		ngxAgentConfigMapName,
diff --git a/internal/controller/provisioner/objects_test.go b/internal/controller/provisioner/objects_test.go
index 600ff5d87f..4af6f2deca 100644
--- a/internal/controller/provisioner/objects_test.go
+++ b/internal/controller/provisioner/objects_test.go
@@ -1696,4 +1696,62 @@ func TestBuildNginxResourceObjects_Patches(t *testing.T) {
 	}
 	g.Expect(svc).ToNot(BeNil())
 	g.Expect(svc.Labels).ToNot(HaveKey("patched")) // Should not have patch-related labels
+
+	// Test that Service patches don't affect Deployment labels and vice versa (cross-contamination)
+	nProxyCfg = &graph.EffectiveNginxProxy{
+		Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{
+			Service: &ngfAPIv1alpha2.ServiceSpec{
+				Patches: []ngfAPIv1alpha2.Patch{
+					{
+						Type: helpers.GetPointer(ngfAPIv1alpha2.PatchTypeStrategicMerge),
+						Value: &apiextv1.JSON{
+							Raw: []byte(`{"metadata":{"labels":{"service-only":"true"}}}`),
+						},
+					},
+				},
+			},
+			Deployment: &ngfAPIv1alpha2.DeploymentSpec{
+				Patches: []ngfAPIv1alpha2.Patch{
+					{
+						Type: helpers.GetPointer(ngfAPIv1alpha2.PatchTypeStrategicMerge),
+						Value: &apiextv1.JSON{
+							Raw: []byte(`{"metadata":{"labels":{"deployment-only":"true"}}}`),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	objects, err = provisioner.buildNginxResourceObjects("gw-nginx", gateway, nProxyCfg)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(objects).To(HaveLen(6))
+
+	// Find and validate service - should only have service-specific labels
+	svc = nil
+	for _, obj := range objects {
+		if s, ok := obj.(*corev1.Service); ok {
+			svc = s
+			break
+		}
+	}
+	g.Expect(svc).ToNot(BeNil())
+	g.Expect(svc.Labels).To(HaveKeyWithValue("service-only", "true"))
+	g.Expect(svc.Labels).ToNot(HaveKey("deployment-only"))
+
+	// Find and validate deployment - should only have deployment-specific labels
+	dep = nil
+	for _, obj := range objects {
+		if d, ok := obj.(*appsv1.Deployment); ok {
+			dep = d
+			break
+		}
+	}
+	g.Expect(dep).ToNot(BeNil())
+	g.Expect(dep.Labels).To(HaveKeyWithValue("deployment-only", "true"))
+	g.Expect(dep.Labels).ToNot(HaveKey("service-only"))
+
+	// Both should still have the common base labels
+	g.Expect(svc.Labels).To(HaveKeyWithValue("app", "nginx"))
+	g.Expect(dep.Labels).To(HaveKeyWithValue("app", "nginx"))
 }