From 09b881848c794db5209d4a381a706c218c811f35 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Sat, 8 Aug 2026 17:22:32 +0200 Subject: [PATCH] fix: mixed patch formats `drift-detection-config` supports both legacy bare patches and structured `libsveltosv1beta1.Patch` documents, but the two formats couldn't be mixed in one ConfigMap. `getPatchesFromConfigMap` parsed the whole ConfigMap as structured `Patch` documents. A single legacy-formatted entry caused that parse to fail for the **entire** ConfigMap. That caused **every** entry including already structured ones, as a raw legacy patch. This PR fixes that by making the legacy/structured fallback happen per ConfigMap entry instead of per ConfigMap. --- controllers/resourcesummary.go | 49 +++++++++++++--------- controllers/resourcesummary_test.go | 64 +++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/controllers/resourcesummary.go b/controllers/resourcesummary.go index 2e56fe68..064ea7c6 100644 --- a/controllers/resourcesummary.go +++ b/controllers/resourcesummary.go @@ -757,31 +757,42 @@ func getGlobalDriftDetectionManagerPatches(ctx context.Context, c client.Client, return getDriftDetectionManagerPatchesOld(ctx, c, logger) } -func getPatchesFromConfigMap(configMap *corev1.ConfigMap, logger logr.Logger, -) ([]libsveltosv1beta1.Patch, error) { - - patches := make([]libsveltosv1beta1.Patch, 0) - for k := range configMap.Data { - patch := &libsveltosv1beta1.Patch{} - err := yaml.Unmarshal([]byte(configMap.Data[k]), patch) +// getPatchFromConfigMapEntry parses a single ConfigMap entry into a Patch. +// The entry can either be a structured libsveltosv1beta1.Patch document (with a +// "patch" field and an optional "target"), or a legacy bare patch (the raw +// StrategicMerge/JSON6902 content, with no "patch" wrapper). Each entry is +// evaluated independently so a ConfigMap can mix both formats across its keys. +func getPatchFromConfigMapEntry(key, value string, logger logr.Logger) libsveltosv1beta1.Patch { + patch := &libsveltosv1beta1.Patch{} + err := yaml.Unmarshal([]byte(value), patch) + if err != nil || patch.Patch == "" { if err != nil { - logger.V(logs.LogInfo).Error(err, "failed to marshal unstructured object") - return nil, err + logger.V(logs.LogInfo).Info(fmt.Sprintf("key %s is not a structured Patch (%v), "+ + "treating it as a legacy patch", key, err)) } - - if patch.Patch == "" { - return nil, fmt.Errorf("ConfigMap %s: content of key %s is not a Patch", - configMap.Name, k) + // Not a structured Patch document (or "patch" field is missing/empty). Fall back to + // treating the whole entry as a legacy, bare patch. + patch = &libsveltosv1beta1.Patch{ + Patch: value, } + } - if patch.Target == nil { - patch.Target = &libsveltosv1beta1.PatchSelector{ - Kind: "Deployment", - Group: appsGroupName, - } + if patch.Target == nil { + patch.Target = &libsveltosv1beta1.PatchSelector{ + Kind: deploymentKind, + Group: appsGroupName, } + } - patches = append(patches, *patch) + return *patch +} + +func getPatchesFromConfigMap(configMap *corev1.ConfigMap, logger logr.Logger, +) ([]libsveltosv1beta1.Patch, error) { + + patches := make([]libsveltosv1beta1.Patch, 0) + for k := range configMap.Data { + patches = append(patches, getPatchFromConfigMapEntry(k, configMap.Data[k], logger)) } return patches, nil diff --git a/controllers/resourcesummary_test.go b/controllers/resourcesummary_test.go index c2f9017b..7879abea 100644 --- a/controllers/resourcesummary_test.go +++ b/controllers/resourcesummary_test.go @@ -19,6 +19,7 @@ package controllers_test import ( "context" "fmt" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -249,6 +250,69 @@ metadata: } Expect(found).To(BeTrue()) }) + + It("getGlobalDriftDetectionManagerPatches supports mixing legacy and structured patches in the same ConfigMap", func() { + cmYAML := fmt.Sprintf(`apiVersion: v1 +data: + legacy-patch: |- + apiVersion: apps/v1 + kind: Deployment + metadata: + name: drift-detection-manager + structured-patch: |- + patch: |- + - op: add + path: /spec/template/spec/imagePullSecrets + value: + - name: registry-pull-secret +kind: ConfigMap +metadata: + name: drift-detection-config-mixed + namespace: %s`, sveltosNamespace) + + cm, err := deployer.GetUnstructured([]byte(cmYAML), logger) + Expect(err).To(BeNil()) + + initObjects := []client.Object{} + for i := range cm { + initObjects = append(initObjects, cm[i]) + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build() + + controllers.SetDriftdetectionConfigMap("drift-detection-config-mixed") + patches, err := controllers.GetGlobalDriftDetectionManagerPatches(context.TODO(), c, logger) + Expect(err).To(BeNil()) + Expect(len(patches)).To(Equal(2)) + controllers.SetDriftdetectionConfigMap("") + + // Both entries default to targeting Deployment/apps since neither ConfigMap key + // specifies a target. + for i := range patches { + Expect(patches[i].Target).ToNot(BeNil()) + Expect(patches[i].Target.Kind).To(Equal(testKindDeployment)) + } + + // The structured entry must remain structured: its Patch field is the JSON6902 + // document as-is, not re-wrapped in another "patch:" key. + foundStructured := false + for i := range patches { + if strings.Contains(patches[i].Patch, "op: add") { + foundStructured = true + Expect(patches[i].Patch).ToNot(ContainSubstring("patch: |-")) + } + } + Expect(foundStructured).To(BeTrue()) + + // The legacy entry must still be carried verbatim as the raw patch content. + foundLegacy := false + for i := range patches { + if strings.Contains(patches[i].Patch, "kind: Deployment") { + foundLegacy = true + } + } + Expect(foundLegacy).To(BeTrue()) + }) }) func prepareCluster() *clusterv1.Cluster {