Skip to content

Commit 2cd3485

Browse files
committed
Fix incorrect metadata patch generation
When annotations and labels were missing on the cluster object but present on the desired object, drift detection produced an "add /metadata" patch. JSON Patch interprets this as replacing the entire metadata block, dropping fields like metadata.name. This update preserves metadata.name in copyAnnotationsAndLabels and emits separate add operations for annotations and labels. A test verifies the correct behavior. Signed-off-by: Yasin Özel <yozel@nebius.com>
1 parent f92d4c6 commit 2cd3485

3 files changed

Lines changed: 59 additions & 12 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
apiVersion: v1
3+
kind: ConfigMap
4+
metadata:
5+
name: "configmap-data"
6+
annotations:
7+
prev: some-previous-value

ssa/jsondiff/unstructured.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,14 @@ func copyAnnotationsAndLabels(obj *unstructured.Unstructured) *unstructured.Unst
246246
Object: make(map[string]interface{}),
247247
}
248248

249+
// When annotations and labels were missing on the cluster object but present on the desired object, drift
250+
// detection produced an "add /metadata" patch. JSON Patch interprets this as replacing the entire metadata block,
251+
// dropping fields like metadata.name. By copying name, we preserve at least one field in metadata to avoid this.
252+
name, ok, _ := unstructured.NestedFieldCopy(obj.Object, "metadata", "name")
253+
if ok {
254+
_ = unstructured.SetNestedField(c.Object, name, "metadata", "name")
255+
}
256+
249257
annotations, ok, _ := unstructured.NestedFieldCopy(obj.Object, "metadata", "annotations")
250258
if ok {
251259
_ = unstructured.SetNestedField(c.Object, annotations, "metadata", "annotations")

ssa/jsondiff/unstructured_test.go

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,8 @@ func TestUnstructuredList(t *testing.T) {
9494
DesiredObject: desired[1],
9595
ClusterObject: cluster[1],
9696
Patch: jsondiff.Patch{
97-
{Type: jsondiff.OperationAdd, Path: "/metadata", Value: map[string]interface{}{
98-
"annotations": map[string]interface{}{
99-
"annotated": "yes",
100-
},
97+
{Type: jsondiff.OperationAdd, Path: "/metadata/annotations", Value: map[string]interface{}{
98+
"annotated": "yes",
10199
}},
102100
},
103101
},
@@ -250,10 +248,8 @@ func TestUnstructuredList(t *testing.T) {
250248
DesiredObject: desired[1],
251249
ClusterObject: cluster[1],
252250
Patch: jsondiff.Patch{
253-
{Type: jsondiff.OperationAdd, Path: "/metadata", Value: map[string]interface{}{
254-
"labels": map[string]interface{}{
255-
"labeled": "change",
256-
},
251+
{Type: jsondiff.OperationAdd, Path: "/metadata/labels", Value: map[string]interface{}{
252+
"labeled": "change",
257253
}},
258254
},
259255
},
@@ -294,10 +290,8 @@ func TestUnstructuredList(t *testing.T) {
294290
DesiredObject: desired[1],
295291
ClusterObject: cluster[1],
296292
Patch: jsondiff.Patch{
297-
{Type: jsondiff.OperationAdd, Path: "/metadata", Value: map[string]interface{}{
298-
"labels": map[string]interface{}{
299-
"labeled": "change",
300-
},
293+
{Type: jsondiff.OperationAdd, Path: "/metadata/labels", Value: map[string]interface{}{
294+
"labeled": "change",
301295
}},
302296
},
303297
},
@@ -507,6 +501,44 @@ func TestUnstructured(t *testing.T) {
507501
}
508502
},
509503
},
504+
{
505+
name: "ConfigMap with added label and annotation",
506+
path: "testdata/empty-configmap.yaml",
507+
mutateDesired: func(obj *unstructured.Unstructured) {
508+
_ = unstructured.SetNestedField(obj.Object, "yes", "metadata", "annotations", "annotated")
509+
_ = unstructured.SetNestedField(obj.Object, "yes", "metadata", "labels", "labeled")
510+
},
511+
want: func(desired, cluster client.Object) *Diff {
512+
return &Diff{
513+
Type: DiffTypeUpdate,
514+
DesiredObject: desired,
515+
ClusterObject: cluster,
516+
Patch: jsondiff.Patch{
517+
{Type: jsondiff.OperationAdd, Path: "/metadata/annotations", Value: map[string]interface{}{"annotated": "yes"}},
518+
{Type: jsondiff.OperationAdd, Path: "/metadata/labels", Value: map[string]interface{}{"labeled": "yes"}},
519+
},
520+
}
521+
},
522+
},
523+
{
524+
name: "Annotated ConfigMap with added label and annotation",
525+
path: "testdata/annotated-configmap.yaml",
526+
mutateDesired: func(obj *unstructured.Unstructured) {
527+
_ = unstructured.SetNestedField(obj.Object, "yes", "metadata", "annotations", "annotated")
528+
_ = unstructured.SetNestedField(obj.Object, "yes", "metadata", "labels", "labeled")
529+
},
530+
want: func(desired, cluster client.Object) *Diff {
531+
return &Diff{
532+
Type: DiffTypeUpdate,
533+
DesiredObject: desired,
534+
ClusterObject: cluster,
535+
Patch: jsondiff.Patch{
536+
{Type: jsondiff.OperationAdd, Path: "/metadata/annotations/annotated", Value: "yes"},
537+
{Type: jsondiff.OperationAdd, Path: "/metadata/labels", Value: map[string]interface{}{"labeled": "yes"}},
538+
},
539+
}
540+
},
541+
},
510542
{
511543
name: "Deployment with missing label and annotation",
512544
path: "testdata/deployment.yaml",

0 commit comments

Comments
 (0)