fixes

Signed-off-by: Anshul Ahuja <anshulahuja@microsoft.com>
vmware-tanzu · Aug 8, 2023 · 58ba830 · 58ba830
1 parent c9debd6
commit 58ba830
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 9 deletions.
diff --git a/internal/resourcemodifiers/resource_modifiers.go b/internal/resourcemodifiers/resource_modifiers.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"regexp"
+	"strconv"
 	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
@@ -117,7 +118,20 @@ func (r *ResourceModifierRule) PatchArrayToByteArray() ([]byte, error) {
 }
 
 func (p *JSONPatch) ToString() string {
-	if strings.Contains(p.Value, "\"") {
+	// if value is empty, then add quotes
+	if p.Value == "" {
+		return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": ""}`, p.Operation, p.From, p.Path)
+	}
+	// if value is a boolean, then don't add quotes
+	if _, err := strconv.ParseBool(p.Value); err == nil {
+		return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value)
+	}
+	// if value is a json object or array, then don't add quotes
+	if strings.Contains(p.Value, "\"") || strings.Contains(p.Value, "[") || strings.Contains(p.Value, "{") {
+		return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value)
+	}
+	// if value is a number, then don't add quotes
+	if _, err := strconv.ParseFloat(p.Value, 64); err == nil {
 		return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value)
 	}
 	return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": "%s"}`, p.Operation, p.From, p.Path, p.Value)

diff --git a/internal/resourcemodifiers/resource_modifiers_test.go b/internal/resourcemodifiers/resource_modifiers_test.go
@@ -185,7 +185,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				"namespace": "foo",
 			},
 			"spec": map[string]interface{}{
-				"replicas": "1",
+				"replicas": int64(1),
 				"template": map[string]interface{}{
 					"metadata": map[string]interface{}{
 						"labels": map[string]interface{}{
@@ -213,7 +213,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				"namespace": "foo",
 			},
 			"spec": map[string]interface{}{
-				"replicas": "2",
+				"replicas": int64(2),
 				"template": map[string]interface{}{
 					"metadata": map[string]interface{}{
 						"labels": map[string]interface{}{
@@ -241,7 +241,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				"namespace": "foo",
 			},
 			"spec": map[string]interface{}{
-				"replicas": "1",
+				"replicas": int64(1),
 				"template": map[string]interface{}{
 					"metadata": map[string]interface{}{
 						"labels": map[string]interface{}{
@@ -614,7 +614,7 @@ func TestJSONPatch_ToString(t *testing.T) {
 				Path:      "/spec/replicas",
 				Value:     "1",
 			},
-			want: `{"op": "test", "from": "", "path": "/spec/replicas", "value": "1"}`,
+			want: `{"op": "test", "from": "", "path": "/spec/replicas", "value": 1}`,
 		},
 		{
 			name: "replace",
@@ -623,7 +623,7 @@ func TestJSONPatch_ToString(t *testing.T) {
 				Path:      "/spec/replicas",
 				Value:     "2",
 			},
-			want: `{"op": "replace", "from": "", "path": "/spec/replicas", "value": "2"}`,
+			want: `{"op": "replace", "from": "", "path": "/spec/replicas", "value": 2}`,
 		},
 		{
 			name: "add complex interfaces",

diff --git a/test/e2e/resourcemodifiers/resource_modifiers.go b/test/e2e/resourcemodifiers/resource_modifiers.go
@@ -39,9 +39,13 @@ resourceModifierRules:
     groupKind: deployments.apps
     resourceNameRegex: "resource-modifiers-.*"
   patches:
+  - operation: add  
+  - operation: add
+    path: "/spec/template/spec/containers/1"
+	value: "{\"name\": \"nginx\", \"image\": \"nginx:1.14.2\", \"ports\": [{\"containerPort\": 80}]}"
   - operation: replace  
-    path: "/spec/template/spec/containers/0/image"
-    value: "gcr.io/velero-gcp/busybox:9.9.9"
+    path: "/spec/replicas"
+    value: "2"
 `
 
 type ResourceModifiersCase struct {
@@ -128,7 +132,8 @@ func (r *ResourceModifiersCase) Verify() error {
 			deploy, err := GetDeployment(r.Client.ClientGo, ns, r.CaseBaseName)
 			Expect(err).To(BeNil(), fmt.Sprintf("Failed to get deployment %s in namespace %s", r.CaseBaseName, ns))
 
-			Expect(deploy.Spec.Template.Spec.Containers[0].Image).To(Equal("gcr.io/velero-gcp/busybox:9.9.9"), fmt.Sprintf("Failed to verify deployment %s in namespace %s", r.CaseBaseName, ns))
+			Expect(deploy.Spec.Replicas).To(Equal(int32(2)), fmt.Sprintf("Failed to verify deployment %s's replicas in namespace %s", r.CaseBaseName, ns))
+			Expect(deploy.Spec.Template.Spec.Containers[0].Image).To(Equal("nginx:1.14.2"), fmt.Sprintf("Failed to verify deployment %s's image in namespace %s", r.CaseBaseName, ns))
 		})
 	}
 	return nil