Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit b504a0e

Browse files
sjbermanciarams87
andauthored
fix: remove patch label/ annotation cross contamination (#3754) (#3766)
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. Co-authored-by: Ciara Stacke <18287516+ciarams87@users.noreply.github.com>
1 parent 9b7c9a8 commit b504a0e

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)

AltStyle によって変換されたページ (->オリジナル) /