Repository: clusternet/clusternet
Pull request #103 (2021-09-04 08:57:15)

validate override value first before applying
<!--  Thanks for sending a pull request!

-->

#### What type of PR is this?
kind/enhancement

#### What this PR does / why we need it:

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
supercedes #101 

#### Special notes for your reviewer:



Patch:
diff --git a/pkg/hub/localizer/patch.go b/pkg/hub/localizer/patch.go
index 3b976c2..09b2df8 100644
--- a/pkg/hub/localizer/patch.go
+++ b/pkg/hub/localizer/patch.go
@@ -36,21 +36,20 @@ const (
 func applyOverrides(original []byte, overrides []appsapi.OverrideConfig) ([]byte, error) {
 	result := original
 	for _, overrideConfig := range overrides {
+		// validates override value first
+		if len(strings.TrimSpace(overrideConfig.Value)) == 0 {
+			continue
+		}
 		overrideBytes, err := yaml.YAMLToJSON([]byte(overrideConfig.Value))
 		if err != nil {
 			return nil, fmt.Errorf("failed to convert patch %s to JSON: %v", overrideConfig.Value, err)
 		}
 
-		// validation before apply override
 		if len(strings.TrimSpace(string(result))) == 0 {
 			result = overrideBytes
 			continue
 		}
 
-		if len(strings.TrimSpace(string(overrideBytes))) == 0 {
-			continue
-		}
-
 		switch overrideConfig.Type {
 		case appsapi.HelmType:
 			result, err = applyHelmOverride(result, overrideBytes)


Test patch:
diff --git a/pkg/hub/localizer/patch_test.go b/pkg/hub/localizer/patch_test.go
index 65943ad..3119200 100644
--- a/pkg/hub/localizer/patch_test.go
+++ b/pkg/hub/localizer/patch_test.go
@@ -76,6 +76,11 @@ hole: black
 					Type:  appsapi.HelmType,
 					Value: `{"address":{"country":"US","state":"MA"},"boat":"fighter"}`,
 				},
+				{
+					Name:  "empty override with whitespaces",
+					Type:  appsapi.HelmType,
+					Value: `   `,
+				},
 				{
 					Name:  "add/update value - yaml format",
 					Type:  appsapi.HelmType,
@@ -100,7 +105,54 @@ hole: black
 				}`),
 		},
 		{
-			name: "JSONPatch/MergePatch/StrategicMergePatch",
+			name:     "Helm with Empty Original",
+			original: []byte(``),
+			overrides: []appsapi.OverrideConfig{
+				{
+					Name:  "empty override",
+					Type:  appsapi.HelmType,
+					Value: `  `,
+				},
+				{
+					Name:  "initial override",
+					Type:  appsapi.HelmType,
+					Value: `{"kind":"Guess","address":{"city":"Nantucket","street":"123 Spouter Inn Ct."},"boat":"pequod","details":{"friends":["Tashtego"]},"name":"Ishmael"}`,
+				},
+				{
+					Name:  "add/update value - json format",
+					Type:  appsapi.HelmType,
+					Value: `{"address":{"country":"US","state":"MA"},"boat":"fighter"}`,
+				},
+				{
+					Name:  "empty override with whitespaces",
+					Type:  appsapi.HelmType,
+					Value: `   `,
+				},
+				{
+					Name:  "add/update value - yaml format",
+					Type:  appsapi.HelmType,
+					Value: helmDataYaml,
+				},
+			},
+			want: []byte(`{
+				"kind": "Guess",
+				"address": {
+					"city": "Nantucket",
+					"country": "US",
+					"planet": "Earth",
+					"state": "MA",
+					"street": "234 Spouter Inn Ct."
+                },
+				"boat": "fighter",
+				"details": {
+					"friends": ["Tashtego"]
+				},
+				"hole": "black",
+				"name": "Ishmael"
+				}`),
+		},
+		{
+			name: "JSONPatch and MergePatch",
 			original: []byte(`{
 				"apiVersion": "v1",
 				"kind": "Pod",
@@ -116,11 +168,21 @@ hole: black
 				}
 			}`),
 			overrides: []appsapi.OverrideConfig{
+				{
+					Name:  "empty override with whitespaces",
+					Type:  appsapi.MergePatchType,
+					Value: `   `,
+				},
 				{
 					Name:  "add namespace - json format",
 					Type:  appsapi.MergePatchType,
 					Value: `{"metadata":{"namespace":"test"}}`,
 				},
+				{
+					Name:  "empty override with whitespaces",
+					Type:  appsapi.JSONPatchType,
+					Value: `   `,
+				},
 				{
 					Name:  "add namespace - yaml format",
 					Type:  appsapi.MergePatchType,