E2E test for Resource Modifier & Bug fixes (#6483)

* Add resource modifier e2e test and bug fixes

Signed-off-by: Anshul Ahuja <[email protected]>

internal/resourcemodifiers/resource_modifiers.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"regexp"
+	"strconv"
 	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
@@ -117,10 +118,10 @@ func (r *ResourceModifierRule) PatchArrayToByteArray() ([]byte, error) {
 }
 
 func (p *JSONPatch) ToString() string {
-	if strings.Contains(p.Value, "\"") {
-		return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value)
+	if addQuotes(p.Value) {
+		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)
+	return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value)
 }
 
 func ApplyPatch(patch []byte, obj *unstructured.Unstructured, log logrus.FieldLogger) error {
@@ -162,3 +163,26 @@ func decodeStruct(r io.Reader, s interface{}) error {
 	dec.KnownFields(true)
 	return dec.Decode(s)
 }
+
+func addQuotes(value string) bool {
+	if value == "" {
+		return true
+	}
+	// if value is null, then don't add quotes
+	if value == "null" {
+		return false
+	}
+	// if value is a boolean, then don't add quotes
+	if _, err := strconv.ParseBool(value); err == nil {
+		return false
+	}
+	// if value is a json object or array, then don't add quotes.
+	if strings.HasPrefix(value, "{") || strings.HasPrefix(value, "[") {
+		return false
+	}
+	// if value is a number, then don't add quotes
+	if _, err := strconv.ParseFloat(value, 64); err == nil {
+		return false
+	}
+	return true
+}

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,19 +614,37 @@ 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",
+			name: "replace integer",
 			fields: fields{
 				Operation: "replace",
 				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",
+			name: "replace array",
+			fields: fields{
+				Operation: "replace",
+				Path:      "/spec/template/spec/containers/0/ports",
+				Value:     `[{"containerPort": 80}]`,
+			},
+			want: `{"op": "replace", "from": "", "path": "/spec/template/spec/containers/0/ports", "value": [{"containerPort": 80}]}`,
+		},
+		{
+			name: "replace with null",
+			fields: fields{
+				Operation: "replace",
+				Path:      "/spec/template/spec/containers/0/ports",
+				Value:     `null`,
+			},
+			want: `{"op": "replace", "from": "", "path": "/spec/template/spec/containers/0/ports", "value": null}`,
+		},
+		{
+			name: "add json object",
 			fields: fields{
 				Operation: "add",
 				Path:      "/spec/template/spec/containers/0",

pkg/cmd/cli/restore/create.go

@@ -281,8 +281,10 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
 
 	if o.ResourceModifierConfigMap != "" {
 		resModifiers = &corev1.TypedLocalObjectReference{
-			Kind: resourcemodifiers.ConfigmapRefType,
-			Name: o.ResourceModifierConfigMap,
+			// Group for core API is ""
+			APIGroup: &corev1.SchemeGroupVersion.Group,
+			Kind:     resourcemodifiers.ConfigmapRefType,
+			Name:     o.ResourceModifierConfigMap,
 		}
 	}
 

test/e2e/README.md

@@ -126,7 +126,7 @@ For programming conventions, Take ResourcePoliciesCase case for example:
 ##### CreateResources
 - It's better to set a global timeout in CreateResources function which is the real beginning of one e2e test
 ##### Destroy
-- It's only clean up resources in currently test namespaces, if wantting to clean up all resources including resources created which is not in currently test namespaces, it's better to override base Destroy function
+- It only cleans up resources in currently test namespaces, if you wish to clean up all resources including resources created which are not in currently test namespaces, it's better to override base Destroy function
 ##### Clean
 - Clean function only clean resources in namespaces which has the prefix CaseBaseName. So the the names of test namespaces should start with prefix of CaseBaseName.
 - If wantting to clean up all resources including resources created which is not in currently test namespaces, it's better to override base Clean function

test/e2e/e2e_suite_test.go

@@ -39,6 +39,7 @@ import (
 	. "github.com/vmware-tanzu/velero/test/e2e/privilegesmgmt"
 	. "github.com/vmware-tanzu/velero/test/e2e/pv-backup"
 	. "github.com/vmware-tanzu/velero/test/e2e/resource-filtering"
+	. "github.com/vmware-tanzu/velero/test/e2e/resourcemodifiers"
 	. "github.com/vmware-tanzu/velero/test/e2e/resourcepolicies"
 	. "github.com/vmware-tanzu/velero/test/e2e/scale"
 	. "github.com/vmware-tanzu/velero/test/e2e/schedule"
@@ -119,6 +120,8 @@ var _ = Describe("[ResourceFiltering][IncludeResources][Restore] Velero test on
 var _ = Describe("[ResourceFiltering][LabelSelector] Velero test on backup include resources matching the label selector", BackupWithLabelSelector)
 var _ = Describe("[ResourceFiltering][ResourcePolicies] Velero test on skip backup of volume by resource policies", ResourcePoliciesTest)
 
+var _ = Describe("[ResourceModifier][Restore] Velero test on resource modifiers from the cluster restore", ResourceModifiersTest)
+
 var _ = Describe("[Backups][Deletion][Restic] Velero tests of Restic backup deletion", BackupDeletionWithRestic)
 var _ = Describe("[Backups][Deletion][Snapshot] Velero tests of snapshot backup deletion", BackupDeletionWithSnapshots)
 var _ = Describe("[Backups][TTL][LongTime] Local backups and restic repos will be deleted once the corresponding backup storage location is deleted", TTLTest)