Repository: clusternet/clusternet
Pull request #416 (2022-07-12 11:43:10)

ignore missing key error during json patch
Signed-off-by: Di Xu <stephenhsu90@gmail.com>

<!--  Thanks for sending a pull request!

-->

#### What type of PR is this?
kind/bug

#### What this PR does / why we need it:
introduced by #385

During json patch, any missing key errors should be ignored. **Also unchanged original value should be returned.**

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #

#### Special notes for your reviewer:
This bug does not affect any previous releases.

dixudx: cc @DanielXLee 

Patch:
diff --git a/pkg/hub/localizer/localizer.go b/pkg/hub/localizer/localizer.go
index 8e388c4..38e973b 100644
--- a/pkg/hub/localizer/localizer.go
+++ b/pkg/hub/localizer/localizer.go
@@ -274,13 +274,12 @@ func (l *Localizer) handleGlobalization(glob *appsapi.Globalization) error {
 
 func (l *Localizer) ApplyOverridesToDescription(desc *appsapi.Description) error {
 	var allErrs []error
-	descCopy := desc.DeepCopy()
-	switch descCopy.Spec.Deployer {
+	switch desc.Spec.Deployer {
 	case appsapi.DescriptionHelmDeployer:
-		desc.Spec.Raw = make([][]byte, len(descCopy.Spec.Charts))
+		desc.Spec.Raw = make([][]byte, len(desc.Spec.Charts))
 
-		for idx, chartRef := range descCopy.Spec.Charts {
-			overrides, err := l.getOverrides(descCopy.Namespace, appsapi.Feed{
+		for idx, chartRef := range desc.Spec.Charts {
+			overrides, err := l.getOverrides(desc.Namespace, appsapi.Feed{
 				Kind:       chartKind.Kind,
 				APIVersion: chartKind.Version,
 				Namespace:  chartRef.Namespace,
@@ -321,14 +320,14 @@ func (l *Localizer) ApplyOverridesToDescription(desc *appsapi.Description) error
 		}
 		return utilerrors.NewAggregate(allErrs)
 	case appsapi.DescriptionGenericDeployer:
-		for idx, rawObject := range descCopy.Spec.Raw {
+		for idx, rawObject := range desc.Spec.Raw {
 			obj := &unstructured.Unstructured{}
 			if err := json.Unmarshal(rawObject, obj); err != nil {
 				allErrs = append(allErrs, err)
 				continue
 			}
 
-			overrides, err := l.getOverrides(descCopy.Namespace, appsapi.Feed{
+			overrides, err := l.getOverrides(desc.Namespace, appsapi.Feed{
 				Kind:       obj.GetKind(),
 				APIVersion: obj.GetAPIVersion(),
 				Namespace:  obj.GetNamespace(),
@@ -348,7 +347,7 @@ func (l *Localizer) ApplyOverridesToDescription(desc *appsapi.Description) error
 		}
 		return utilerrors.NewAggregate(allErrs)
 	default:
-		return fmt.Errorf("unsupported deployer %s", descCopy.Spec.Deployer)
+		return fmt.Errorf("unsupported deployer %s", desc.Spec.Deployer)
 	}
 }
 
diff --git a/pkg/hub/localizer/patch.go b/pkg/hub/localizer/patch.go
index 8f326f5..2a40d3a 100644
--- a/pkg/hub/localizer/patch.go
+++ b/pkg/hub/localizer/patch.go
@@ -103,10 +103,13 @@ func applyJSONPatch(cur, overrideBytes []byte) ([]byte, error) {
 			maxJSONPatchOperations, len(patchObj))
 	}
 	patchedJS, err := patchObj.Apply(cur)
-	if err != nil && !errors.Is(err, jsonpatch.ErrMissing) {
-		return nil, err
+	if err == nil {
+		return patchedJS, nil
+	}
+	if errors.Is(err, jsonpatch.ErrMissing) {
+		return cur, nil
 	}
-	return patchedJS, nil
+	return nil, err
 }
 
 func applyHelmValuesOverride(currentByte, overrideByte []byte) ([]byte, error) {


Test patch:
diff --git a/pkg/hub/localizer/patch_test.go b/pkg/hub/localizer/patch_test.go
index 53ff111..baaf9b6 100644
--- a/pkg/hub/localizer/patch_test.go
+++ b/pkg/hub/localizer/patch_test.go
@@ -418,3 +418,47 @@ hole: black
 		})
 	}
 }
+
+func TestApplyJSONPatch(t *testing.T) {
+	tests := []struct {
+		name          string
+		cur           []byte
+		overrideBytes []byte
+		want          []byte
+		wantErr       bool
+	}{
+		{
+			name:          "remove nonexistent key (/spec/chart)",
+			cur:           []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
+			overrideBytes: []byte(defaultChartOverrideConfig[0].Value),
+			want:          []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
+			wantErr:       false,
+		},
+		{
+			name:          "remove nonexistent key (/spec/targetNamespace)",
+			cur:           []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
+			overrideBytes: []byte(defaultChartOverrideConfig[1].Value),
+			want:          []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
+			wantErr:       false,
+		},
+		{
+			name:          "remove key (/spec/version)",
+			cur:           []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
+			overrideBytes: []byte(`[{"path":"/spec/version","op":"remove"}]`),
+			want:          []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{}}`),
+			wantErr:       false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := applyJSONPatch(tt.cur, tt.overrideBytes)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("applyJSONPatch() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("applyJSONPatch() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}