fix: preserve parameter remove markers in componentparameter

Author: leon-ape

controllers/parameters/componentparameter_controller_test.go

@@ -227,8 +227,9 @@ var _ = Describe("ComponentParameter Controller", func() {
 				item := parameters.GetConfigTemplateItem(&cfg.Spec, configSpecName)
 				g.Expect(item).ShouldNot(BeNil())
 				g.Expect(item.ConfigFileParams).Should(HaveKey("my.cnf"))
-				g.Expect(item.ConfigFileParams["my.cnf"].Parameters).Should(HaveKey("max_connections"))
-				g.Expect(item.ConfigFileParams["my.cnf"].Parameters["max_connections"]).Should(BeNil())
+				decoded := parameters.DecodeParameterOverlay(item.ConfigFileParams["my.cnf"].Parameters)
+				g.Expect(decoded).Should(HaveKey("max_connections"))
+				g.Expect(decoded["max_connections"]).Should(BeNil())
 			})).Should(Succeed())
 		})
 

controllers/parameters/componentparameter_controller_utils.go

@@ -98,9 +98,8 @@ func reconcileParameterValuesIntoSpec(ctx context.Context, cli client.Client, co
 	if reflect.DeepEqual(compParam.Spec, *specCopy) {
 		return false, nil
 	}
-	patch := client.MergeFrom(compParam.DeepCopy())
 	compParam.Spec = *specCopy
-	return true, cli.Patch(ctx, compParam, patch)
+	return true, cli.Update(ctx, compParam)
 }
 
 func applyParameterInputs(spec *parametersv1alpha1.ComponentParameterSpec,
@@ -304,11 +303,24 @@ func mergeItemParameters(item *parametersv1alpha1.ConfigTemplateItemDetail, upda
 		if parametersInFile.Content != nil {
 			merged.Content = parametersInFile.Content
 		}
-		if merged.Parameters == nil && len(parametersInFile.Parameters) > 0 {
-			merged.Parameters = map[string]*string{}
-		}
-		for paramKey, paramValue := range parametersInFile.Parameters {
-			merged.Parameters[paramKey] = paramValue
+		if override {
+			encodedParameters := parameters.EncodeParameterOverlay(parametersInFile.Parameters)
+			if len(encodedParameters) == 0 {
+				merged.Parameters = nil
+			} else {
+				merged.Parameters = make(map[string]*string, len(encodedParameters))
+				for paramKey, paramValue := range encodedParameters {
+					merged.Parameters[paramKey] = paramValue
+				}
+			}
+		} else {
+			encodedParameters := parameters.EncodeParameterOverlay(parametersInFile.Parameters)
+			if merged.Parameters == nil && len(encodedParameters) > 0 {
+				merged.Parameters = map[string]*string{}
+			}
+			for paramKey, paramValue := range encodedParameters {
+				merged.Parameters[paramKey] = paramValue
+			}
 		}
 		item.ConfigFileParams[key] = merged
 	}

controllers/parameters/componentparameter_controller_utils_test.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/utils/ptr"
 
 	parametersv1alpha1 "github.com/apecloud/kubeblocks/apis/parameters/v1alpha1"
+	parampkg "github.com/apecloud/kubeblocks/pkg/parameters"
 )
 
 func TestNormalizeManagedParameterInputs(t *testing.T) {
@@ -67,3 +68,43 @@ func TestNormalizeManagedParameterInputs(t *testing.T) {
 		}
 	})
 }
+
+func TestMergeItemParameters(t *testing.T) {
+	t.Run("override replaces managed parameter overlay for a file", func(t *testing.T) {
+		item := &parametersv1alpha1.ConfigTemplateItemDetail{
+			ConfigFileParams: map[string]parametersv1alpha1.ParametersInFile{
+				"my.cnf": {
+					Content: ptr.To("[mysqld]\nmax_connections=1000\n"),
+					Parameters: map[string]*string{
+						"max_connections": ptr.To("1000"),
+						"sync_binlog":     ptr.To("1"),
+					},
+				},
+			},
+		}
+		updated := map[string]parametersv1alpha1.ParametersInFile{
+			"my.cnf": {
+				Parameters: map[string]*string{
+					"max_connections": nil,
+				},
+			},
+		}
+
+		mergeItemParameters(item, updated, true)
+
+		got := item.ConfigFileParams["my.cnf"]
+		if got.Content == nil || *got.Content != "[mysqld]\nmax_connections=1000\n" {
+			t.Fatalf("expected non-managed fields to be preserved, got %#v", got.Content)
+		}
+		if len(got.Parameters) != 1 {
+			t.Fatalf("expected managed overlay to be replaced, got %#v", got.Parameters)
+		}
+		decoded := parampkg.DecodeParameterOverlay(got.Parameters)
+		if _, ok := decoded["max_connections"]; !ok {
+			t.Fatalf("expected max_connections remove marker to be kept")
+		}
+		if decoded["max_connections"] != nil {
+			t.Fatalf("expected max_connections to be overridden to nil remove marker, got %#v", decoded["max_connections"])
+		}
+	})
+}

pkg/operations/reconfigure_test.go

@@ -65,8 +65,8 @@ var _ = Describe("Reconfigure OpsRequest", func() {
 		testapps.ClearResources(&testCtx, generics.OpsRequestSignature, inNS, ml)
 		testapps.ClearResources(&testCtx, generics.ConfigMapSignature, inNS, ml)
 		testapps.ClearResources(&testCtx, generics.ParametersDefinitionSignature, ml)
-		testapps.ClearResources(&testCtx, generics.InstanceSetSignature, inNS, ml)
-		testapps.ClearResources(&testCtx, generics.ComponentParameterSignature, inNS)
+		testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.InstanceSetSignature, true, inNS, ml)
+		testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ComponentParameterSignature, true, inNS)
 	}
 
 	BeforeEach(cleanEnv)

pkg/parameters/patch_merger.go

@@ -42,7 +42,7 @@ func DoMerge(baseData map[string]string,
 			updatedFiles[key] = *params.Content
 		}
 		if len(params.Parameters) > 0 {
-			upParams, _ := core.FromStringMap(params.Parameters, builder.BuildValueTransformer(key))
+			upParams, _ := core.FromStringMap(DecodeParameterOverlay(params.Parameters), builder.BuildValueTransformer(key))
 			updatedParams = append(updatedParams, core.ParamPairs{
 				Key:           key,
 				UpdatedParams: upParams,

pkg/parameters/remove_marker.go

@@ -0,0 +1,54 @@
+/*
+Copyright (C) 2022-2025 ApeCloud Co., Ltd
+
+This file is part of KubeBlocks project
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package parameters
+
+import "k8s.io/utils/ptr"
+
+const parameterRemoveMarker = "__kb_internal_remove_marker__"
+
+func EncodeParameterOverlay(parameters map[string]*string) map[string]*string {
+	if len(parameters) == 0 {
+		return nil
+	}
+	encoded := make(map[string]*string, len(parameters))
+	for key, value := range parameters {
+		if value == nil {
+			encoded[key] = ptr.To(parameterRemoveMarker)
+			continue
+		}
+		encoded[key] = value
+	}
+	return encoded
+}
+
+func DecodeParameterOverlay(parameters map[string]*string) map[string]*string {
+	if len(parameters) == 0 {
+		return nil
+	}
+	decoded := make(map[string]*string, len(parameters))
+	for key, value := range parameters {
+		if value != nil && *value == parameterRemoveMarker {
+			decoded[key] = nil
+			continue
+		}
+		decoded[key] = value
+	}
+	return decoded
+}