Skip to content

Commit 1175fc1

Browse files
committed
use JSON Patch to fully replace labels/annotations on co-managed resources
Signed-off-by: chiragkyal <ckyal@redhat.com>
1 parent 14110ae commit 1175fc1

4 files changed

Lines changed: 626 additions & 13 deletions

File tree

pkg/controller/external_secrets/configmap_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ func TestEnsureTrustedCABundleConfigMap(t *testing.T) {
104104
if cm.Labels[trustedCABundleInjectLabel] != "true" {
105105
t.Errorf("expected inject label in patch target, got %q", cm.Labels[trustedCABundleInjectLabel])
106106
}
107-
if patch.Type() != types.MergePatchType {
108-
t.Errorf("expected MergePatch, got %s", patch.Type())
107+
if patch.Type() != types.JSONPatchType {
108+
t.Errorf("expected JSONPatch, got %s", patch.Type())
109109
}
110110
return nil
111111
})
@@ -140,8 +140,8 @@ func TestEnsureTrustedCABundleConfigMap(t *testing.T) {
140140
if cm.Labels[trustedCABundleInjectLabel] != "true" {
141141
t.Errorf("expected inject label in patch target, got %q", cm.Labels[trustedCABundleInjectLabel])
142142
}
143-
if patch.Type() != types.MergePatchType {
144-
t.Errorf("expected MergePatch, got %s", patch.Type())
143+
if patch.Type() != types.JSONPatchType {
144+
t.Errorf("expected JSONPatch, got %s", patch.Type())
145145
}
146146
return nil
147147
})

pkg/controller/external_secrets/utils.go

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -250,19 +250,42 @@ func (r *Reconciler) createWithFallback(desired client.Object, resourceMetadata
250250
return nil
251251
}
252252

253-
// patchResourceMetadata sends a MergePatch that sets only labels and annotations on the
254-
// object, leaving all other fields untouched. It also removes obsolete annotations before patching.
253+
// jsonPatchOp is a single JSON Patch operation.
254+
type jsonPatchOp struct {
255+
Op string `json:"op"`
256+
Path string `json:"path"`
257+
Value interface{} `json:"value"`
258+
}
259+
260+
// patchResourceMetadata fully replaces the labels and annotations on an object using a
261+
// JSON Patch, leaving all other fields untouched. This is safe for co-managed
262+
// resources where this operator owns all metadata but data fields are owned by an external
263+
// controller (e.g. CNO-managed ConfigMap data, cert-controller-managed TLS Secret data).
264+
//
265+
// JSON Patch "add" on an existing path replaces the entire value, so the resulting
266+
// labels/annotations on the server exactly match desired.
267+
//
268+
// RemoveObsoleteAnnotations is called defensively to strip any deleted annotation keys
269+
// from desired before building the patch, regardless of whether the caller already did so.
255270
func (r *Reconciler) patchResourceMetadata(desired client.Object, resourceMetadata common.ResourceMetadata) error {
256271
common.RemoveObsoleteAnnotations(desired, resourceMetadata)
257-
patch := map[string]interface{}{
258-
"metadata": map[string]interface{}{
259-
"labels": desired.GetLabels(),
260-
"annotations": desired.GetAnnotations(),
261-
},
272+
273+
annotations := desired.GetAnnotations()
274+
if annotations == nil {
275+
annotations = map[string]string{}
276+
}
277+
labels := desired.GetLabels()
278+
if labels == nil {
279+
labels = map[string]string{}
280+
}
281+
282+
ops := []jsonPatchOp{
283+
{Op: "add", Path: "/metadata/labels", Value: labels},
284+
{Op: "add", Path: "/metadata/annotations", Value: annotations},
262285
}
263-
patchBytes, err := json.Marshal(patch)
286+
patchBytes, err := json.Marshal(ops)
264287
if err != nil {
265288
return fmt.Errorf("failed to marshal metadata patch: %w", err)
266289
}
267-
return r.CtrlClient.Patch(r.ctx, desired, client.RawPatch(types.MergePatchType, patchBytes))
290+
return r.CtrlClient.Patch(r.ctx, desired, client.RawPatch(types.JSONPatchType, patchBytes))
268291
}

0 commit comments

Comments
 (0)