Skip to content

Commit e09b173

Browse files
authored
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.
1 parent 235a4b2 commit e09b173

File tree

2 files changed

+75
-2
lines changed

2 files changed

+75
-2
lines changed

internal/controller/provisioner/objects.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,28 @@ func (p *NginxProvisioner) buildNginxResourceObjects(
152152
ports[int32(listener.Port)] = struct{}{}
153153
}
154154

155-
service, err := buildNginxService(objectMeta, nProxyCfg, ports, selectorLabels)
155+
// Create separate copies of objectMeta for service and deployment to avoid shared map references
156+
serviceObjectMeta := metav1.ObjectMeta{
157+
Name: objectMeta.Name,
158+
Namespace: objectMeta.Namespace,
159+
Labels: maps.Clone(objectMeta.Labels),
160+
Annotations: maps.Clone(objectMeta.Annotations),
161+
}
162+
163+
deploymentObjectMeta := metav1.ObjectMeta{
164+
Name: objectMeta.Name,
165+
Namespace: objectMeta.Namespace,
166+
Labels: maps.Clone(objectMeta.Labels),
167+
Annotations: maps.Clone(objectMeta.Annotations),
168+
}
169+
170+
service, err := buildNginxService(serviceObjectMeta, nProxyCfg, ports, selectorLabels)
156171
if err != nil {
157172
errs = append(errs, err)
158173
}
159174

160175
deployment, err := p.buildNginxDeployment(
161-
objectMeta,
176+
deploymentObjectMeta,
162177
nProxyCfg,
163178
ngxIncludesConfigMapName,
164179
ngxAgentConfigMapName,

internal/controller/provisioner/objects_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,4 +1696,62 @@ func TestBuildNginxResourceObjects_Patches(t *testing.T) {
16961696
}
16971697
g.Expect(svc).ToNot(BeNil())
16981698
g.Expect(svc.Labels).ToNot(HaveKey("patched")) // Should not have patch-related labels
1699+
1700+
// Test that Service patches don't affect Deployment labels and vice versa (cross-contamination)
1701+
nProxyCfg = &graph.EffectiveNginxProxy{
1702+
Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{
1703+
Service: &ngfAPIv1alpha2.ServiceSpec{
1704+
Patches: []ngfAPIv1alpha2.Patch{
1705+
{
1706+
Type: helpers.GetPointer(ngfAPIv1alpha2.PatchTypeStrategicMerge),
1707+
Value: &apiextv1.JSON{
1708+
Raw: []byte(`{"metadata":{"labels":{"service-only":"true"}}}`),
1709+
},
1710+
},
1711+
},
1712+
},
1713+
Deployment: &ngfAPIv1alpha2.DeploymentSpec{
1714+
Patches: []ngfAPIv1alpha2.Patch{
1715+
{
1716+
Type: helpers.GetPointer(ngfAPIv1alpha2.PatchTypeStrategicMerge),
1717+
Value: &apiextv1.JSON{
1718+
Raw: []byte(`{"metadata":{"labels":{"deployment-only":"true"}}}`),
1719+
},
1720+
},
1721+
},
1722+
},
1723+
},
1724+
}
1725+
1726+
objects, err = provisioner.buildNginxResourceObjects("gw-nginx", gateway, nProxyCfg)
1727+
g.Expect(err).ToNot(HaveOccurred())
1728+
g.Expect(objects).To(HaveLen(6))
1729+
1730+
// Find and validate service - should only have service-specific labels
1731+
svc = nil
1732+
for _, obj := range objects {
1733+
if s, ok := obj.(*corev1.Service); ok {
1734+
svc = s
1735+
break
1736+
}
1737+
}
1738+
g.Expect(svc).ToNot(BeNil())
1739+
g.Expect(svc.Labels).To(HaveKeyWithValue("service-only", "true"))
1740+
g.Expect(svc.Labels).ToNot(HaveKey("deployment-only"))
1741+
1742+
// Find and validate deployment - should only have deployment-specific labels
1743+
dep = nil
1744+
for _, obj := range objects {
1745+
if d, ok := obj.(*appsv1.Deployment); ok {
1746+
dep = d
1747+
break
1748+
}
1749+
}
1750+
g.Expect(dep).ToNot(BeNil())
1751+
g.Expect(dep.Labels).To(HaveKeyWithValue("deployment-only", "true"))
1752+
g.Expect(dep.Labels).ToNot(HaveKey("service-only"))
1753+
1754+
// Both should still have the common base labels
1755+
g.Expect(svc.Labels).To(HaveKeyWithValue("app", "nginx"))
1756+
g.Expect(dep.Labels).To(HaveKeyWithValue("app", "nginx"))
16991757
}

0 commit comments

Comments
 (0)