add from field and enhance coverage

Signed-off-by: Anshul Ahuja <anshulahuja@microsoft.com>

internal/resourcemodifiers/resource_modifiers.go

@@ -23,6 +23,7 @@ const (
 
 type JSONPatch struct {
 	Operation string `yaml:"operation"`
+	From      string `yaml:"from,omitempty"`
 	Path      string `yaml:"path"`
 	Value     string `yaml:"value,omitempty"`
 }
@@ -116,7 +117,10 @@ func (r *ResourceModifierRule) PatchArrayToByteArray() ([]byte, error) {
 }
 
 func (p *JSONPatch) ToString() string {
-	return fmt.Sprintf(`{"op": "%s", "path": "%s", "value": "%s"}`, p.Operation, p.Path, p.Value)
+	if strings.Contains(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 {

internal/resourcemodifiers/resource_modifiers_test.go

@@ -1,6 +1,7 @@
 package resourcemodifiers
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -11,8 +12,7 @@ import (
 )
 
 func TestGetResourceModifiersFromConfig(t *testing.T) {
-	// Create a test ConfigMap
+	cm1 := &v1.ConfigMap{
-	cm := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-configmap",
 			Namespace: "test-namespace",
@@ -22,16 +22,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 		},
 	}
 
-	// Call the function and check for errors
+	rules1 := &ResourceModifiers{
-	original, err := GetResourceModifiersFromConfig(cm)
-	assert.Nil(t, err)
-
-	// Check that the returned resourceModifiers object contains the expected data
-	assert.Equal(t, "v1", original.Version)
-	assert.Len(t, original.ResourceModifierRules, 1)
-	assert.Len(t, original.ResourceModifierRules[0].Patches, 2)
-
-	expected := &ResourceModifiers{
 		Version: "v1",
 		ResourceModifierRules: []ResourceModifierRule{
 			{
@@ -54,10 +45,97 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 			},
 		},
 	}
-	if err != nil {
+	cm2 := &v1.ConfigMap{
-		t.Fatalf("failed to build policy with error %v", err)
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-configmap",
+			Namespace: "test-namespace",
+		},
+		Data: map[string]string{
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: deployments.apps\n    resourceNameRegex: \"^test-.*$\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: add\n    path: \"/spec/template/spec/containers/0\"\n    value: \"{\\\"name\\\": \\\"nginx\\\", \\\"image\\\": \\\"nginx:1.14.2\\\", \\\"ports\\\": [{\\\"containerPort\\\": 80}]}\"\n  - operation: copy\n    from: \"/spec/template/spec/containers/0\"\n    path: \"/spec/template/spec/containers/1\"\n\n\n",
+		},
+	}
+
+	rules2 := &ResourceModifiers{
+		Version: "v1",
+		ResourceModifierRules: []ResourceModifierRule{
+			{
+				Conditions: Conditions{
+					GroupKind:         "deployments.apps",
+					ResourceNameRegex: "^test-.*$",
+					Namespaces:        []string{"bar", "foo"},
+				},
+				Patches: []JSONPatch{
+					{
+						Operation: "add",
+						Path:      "/spec/template/spec/containers/0",
+						Value:     `{"name": "nginx", "image": "nginx:1.14.2", "ports": [{"containerPort": 80}]}`,
+					},
+					{
+						Operation: "copy",
+						From:      "/spec/template/spec/containers/0",
+						Path:      "/spec/template/spec/containers/1",
+					},
+				},
+			},
+		},
+	}
+
+	cm3 := &v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-configmap",
+			Namespace: "test-namespace",
+		},
+		Data: map[string]string{
+			"sub.yml": "version1: v1\nresourceModifierRules:\n- conditions:\n    groupKind: deployments.apps\n    resourceNameRegex: \"^test-.*$\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: add\n    path: \"/spec/template/spec/containers/0\"\n    value: \"{\\\"name\\\": \\\"nginx\\\", \\\"image\\\": \\\"nginx:1.14.2\\\", \\\"ports\\\": [{\\\"containerPort\\\": 80}]}\"\n  - operation: copy\n    from: \"/spec/template/spec/containers/0\"\n    path: \"/spec/template/spec/containers/1\"\n\n\n",
+		},
+	}
+
+	type args struct {
+		cm *v1.ConfigMap
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *ResourceModifiers
+		wantErr bool
+	}{
+		{
+			name: "test 1",
+			args: args{
+				cm: cm1,
+			},
+			want:    rules1,
+			wantErr: false,
+		},
+		{
+			name: "complex payload in add and copy operator",
+			args: args{
+				cm: cm2,
+			},
+			want:    rules2,
+			wantErr: false,
+		},
+		{
+			name: "invalid payload",
+			args: args{
+				cm: cm3,
+			},
+			want:    nil,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetResourceModifiersFromConfig(tt.args.cm)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("GetResourceModifiersFromConfig() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("GetResourceModifiersFromConfig() = %v, want %v", got, tt.want)
+			}
+		})
 	}
-	assert.Equal(t, original, expected)
 }
 
 func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
@@ -99,13 +177,22 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				"namespace": "foo",
 			},
 			"spec": map[string]interface{}{
-				"containers": []interface{}{
+				"replicas": "1",
-					map[string]interface{}{
+				"template": map[string]interface{}{
-						"name":  "nginx",
+					"metadata": map[string]interface{}{
-						"image": "nginx:latest",
+						"labels": map[string]interface{}{
+							"app": "nginx",
+						},
+					},
+					"spec": map[string]interface{}{
+						"containers": []interface{}{
+							map[string]interface{}{
+								"name":  "nginx",
+								"image": "nginx:latest",
+							},
+						},
 					},
 				},
-				"replicas": "1",
 			},
 		},
 	}
@@ -118,13 +205,54 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				"namespace": "foo",
 			},
 			"spec": map[string]interface{}{
-				"containers": []interface{}{
+				"replicas": "2",
-					map[string]interface{}{
+				"template": map[string]interface{}{
-						"name":  "nginx",
+					"metadata": map[string]interface{}{
-						"image": "nginx:latest",
+						"labels": map[string]interface{}{
+							"app": "nginx",
+						},
+					},
+					"spec": map[string]interface{}{
+						"containers": []interface{}{
+							map[string]interface{}{
+								"name":  "nginx",
+								"image": "nginx:latest",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	deployNginxMysql := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "apps/v1",
+			"kind":       "Deployment",
+			"metadata": map[string]interface{}{
+				"name":      "test-deployment",
+				"namespace": "foo",
+			},
+			"spec": map[string]interface{}{
+				"replicas": "1",
+				"template": map[string]interface{}{
+					"metadata": map[string]interface{}{
+						"labels": map[string]interface{}{
+							"app": "nginx",
+						},
+					},
+					"spec": map[string]interface{}{
+						"containers": []interface{}{
+							map[string]interface{}{
+								"name":  "nginx",
+								"image": "nginx:latest",
+							},
+							map[string]interface{}{
+								"name":  "mysql",
+								"image": "mysql:latest",
+							},
+						},
 					},
 				},
-				"replicas": "2",
 			},
 		},
 	}
@@ -372,6 +500,77 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 			wantErr: false,
 			wantObj: deployNginxOneReplica.DeepCopy(),
 		},
+		{
+			name: "add container mysql to deployment",
+			fields: fields{
+				Version: "v1",
+				ResourceModifierRules: []ResourceModifierRule{
+					{
+						Conditions: Conditions{
+							GroupKind:         "deployments.apps",
+							ResourceNameRegex: "^test-.*$",
+							Namespaces:        []string{"foo"},
+						},
+						Patches: []JSONPatch{
+							{
+								Operation: "add",
+								Path:      "/spec/template/spec/containers/1",
+								Value:     `{"name": "mysql", "image": "mysql:latest"}`,
+							},
+						},
+					},
+				},
+			},
+			args: args{
+				obj:           deployNginxOneReplica,
+				groupResource: "deployments.apps",
+			},
+			wantErr: false,
+			wantObj: deployNginxMysql,
+		},
+		{
+			name: "Copy container 0 to container 1 and then modify container 1",
+			fields: fields{
+				Version: "v1",
+				ResourceModifierRules: []ResourceModifierRule{
+					{
+						Conditions: Conditions{
+							GroupKind:         "deployments.apps",
+							ResourceNameRegex: "^test-.*$",
+							Namespaces:        []string{"foo"},
+						},
+						Patches: []JSONPatch{
+							{
+								Operation: "copy",
+								From:      "/spec/template/spec/containers/0",
+								Path:      "/spec/template/spec/containers/1",
+							},
+							{
+								Operation: "test",
+								Path:      "/spec/template/spec/containers/1/image",
+								Value:     "nginx:latest",
+							},
+							{
+								Operation: "replace",
+								Path:      "/spec/template/spec/containers/1/name",
+								Value:     "mysql",
+							},
+							{
+								Operation: "replace",
+								Path:      "/spec/template/spec/containers/1/image",
+								Value:     "mysql:latest",
+							},
+						},
+					},
+				},
+			},
+			args: args{
+				obj:           deployNginxOneReplica.DeepCopy(),
+				groupResource: "deployments.apps",
+			},
+			wantErr: false,
+			wantObj: deployNginxMysql.DeepCopy(),
+		},
 	}
 
 	for _, tt := range tests {
@@ -387,3 +586,84 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 		})
 	}
 }
+
+func TestJSONPatch_ToString(t *testing.T) {
+	type fields struct {
+		Operation string
+		From      string
+		Path      string
+		Value     string
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   string
+	}{
+		{
+			name: "test",
+			fields: fields{
+				Operation: "test",
+				Path:      "/spec/replicas",
+				Value:     "1",
+			},
+			want: `{"op": "test", "from": "", "path": "/spec/replicas", "value": "1"}`,
+		},
+		{
+			name: "replace",
+			fields: fields{
+				Operation: "replace",
+				Path:      "/spec/replicas",
+				Value:     "2",
+			},
+			want: `{"op": "replace", "from": "", "path": "/spec/replicas", "value": "2"}`,
+		},
+		{
+			name: "add complex interfaces",
+			fields: fields{
+				Operation: "add",
+				Path:      "/spec/template/spec/containers/0",
+				Value:     `{"name": "nginx", "image": "nginx:1.14.2", "ports": [{"containerPort": 80}]}`,
+			},
+			want: `{"op": "add", "from": "", "path": "/spec/template/spec/containers/0", "value": {"name": "nginx", "image": "nginx:1.14.2", "ports": [{"containerPort": 80}]}}`,
+		},
+		{
+			name: "remove",
+			fields: fields{
+				Operation: "remove",
+				Path:      "/spec/template/spec/containers/0",
+			},
+			want: `{"op": "remove", "from": "", "path": "/spec/template/spec/containers/0", "value": ""}`,
+		},
+		{
+			name: "move",
+			fields: fields{
+				Operation: "move",
+				From:      "/spec/template/spec/containers/0",
+				Path:      "/spec/template/spec/containers/1",
+			},
+			want: `{"op": "move", "from": "/spec/template/spec/containers/0", "path": "/spec/template/spec/containers/1", "value": ""}`,
+		},
+		{
+			name: "copy",
+			fields: fields{
+				Operation: "copy",
+				From:      "/spec/template/spec/containers/0",
+				Path:      "/spec/template/spec/containers/1",
+			},
+			want: `{"op": "copy", "from": "/spec/template/spec/containers/0", "path": "/spec/template/spec/containers/1", "value": ""}`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &JSONPatch{
+				Operation: tt.fields.Operation,
+				From:      tt.fields.From,
+				Path:      tt.fields.Path,
+				Value:     tt.fields.Value,
+			}
+			if got := p.ToString(); got != tt.want {
+				t.Errorf("JSONPatch.ToString() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}