Improve test coverage

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

internal/resourcemodifiers/resource_modifiers_test.go

@@ -116,13 +116,21 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "invalid payload",
+			name: "invalid payload version1",
 			args: args{
 				cm: cm3,
 			},
 			want:    nil,
 			wantErr: true,
 		},
+		{
+			name: "nil configmap",
+			args: args{
+				cm: nil,
+			},
+			want:    nil,
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/resourcemodifiers/resource_modifiers_validator.go

@@ -4,17 +4,15 @@ import (
 	"strings"
 
 	"fmt"
-
-	"github.com/pkg/errors"
 )
 
 func (r *ResourceModifierRule) Validate() error {
 	if err := r.Conditions.Validate(); err != nil {
-		return errors.WithStack(err)
+		return err
 	}
 	for _, patch := range r.Patches {
 		if err := patch.Validate(); err != nil {
-			return errors.WithStack(err)
+			return err
 		}
 	}
 	return nil
@@ -29,7 +27,7 @@ func (p *ResourceModifiers) Validate() error {
 	}
 	for _, rule := range p.ResourceModifierRules {
 		if err := rule.Validate(); err != nil {
-			return errors.WithStack(err)
+			return err
 		}
 	}
 

internal/resourcemodifiers/resource_modifiers_validator_test.go

@@ -68,6 +68,52 @@ func TestResourceModifiers_Validate(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "patch has invalid operation",
+			fields: fields{
+				Version: "v1",
+				ResourceModifierRules: []ResourceModifierRule{
+					{
+						Conditions: Conditions{
+							GroupKind:         "persistentvolumeclaims",
+							ResourceNameRegex: ".*",
+							Namespaces:        []string{"bar", "foo"},
+						},
+						Patches: []JSONPatch{
+							{
+								Operation: "invalid",
+								Path:      "/spec/storageClassName",
+								Value:     "premium",
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Condition has empty GroupKind",
+			fields: fields{
+				Version: "v1",
+				ResourceModifierRules: []ResourceModifierRule{
+					{
+						Conditions: Conditions{
+							GroupKind:         "",
+							ResourceNameRegex: ".*",
+							Namespaces:        []string{"bar", "foo"},
+						},
+						Patches: []JSONPatch{
+							{
+								Operation: "invalid",
+								Path:      "/spec/storageClassName",
+								Value:     "premium",
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -144,47 +190,3 @@ func TestJsonPatch_Validate(t *testing.T) {
 		})
 	}
 }
-
-func TestConditions_Validate(t *testing.T) {
-	type fields struct {
-		Namespaces        []string
-		GroupKind         string
-		ResourceNameRegex string
-	}
-	tests := []struct {
-		name    string
-		fields  fields
-		wantErr bool
-	}{
-		{
-			name: "non 0 length namespaces, non empty group kind, non empty resource name regex",
-			fields: fields{
-				Namespaces:        []string{"bar", "foo"},
-				GroupKind:         "persistentvolumeclaims",
-				ResourceNameRegex: ".*",
-			},
-			wantErr: false,
-		},
-		{
-			name: "empty group kind throws error",
-			fields: fields{
-				Namespaces:        []string{"bar", "foo"},
-				GroupKind:         "",
-				ResourceNameRegex: ".*",
-			},
-			wantErr: true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			c := &Conditions{
-				Namespaces:        tt.fields.Namespaces,
-				GroupKind:         tt.fields.GroupKind,
-				ResourceNameRegex: tt.fields.ResourceNameRegex,
-			}
-			if err := c.Validate(); (err != nil) != tt.wantErr {
-				t.Errorf("Conditions.Validate() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}

pkg/controller/restore_controller_test.go

@@ -34,6 +34,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/builder"
 	"github.com/vmware-tanzu/velero/pkg/metrics"
@@ -47,6 +48,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/util/logging"
 	"github.com/vmware-tanzu/velero/pkg/util/results"
 	"github.com/vmware-tanzu/velero/pkg/volume"
+	corev1 "k8s.io/api/core/v1"
 )
 
 func TestFetchBackupInfo(t *testing.T) {
@@ -659,6 +661,145 @@ func TestValidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
 	assert.Equal(t, "foo", restore.Spec.BackupName)
 }
 
+func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
+	formatFlag := logging.FormatText
+
+	var (
+		logger        = velerotest.NewLogger()
+		pluginManager = &pluginmocks.Manager{}
+		fakeClient    = velerotest.NewFakeControllerRuntimeClient(t)
+		backupStore   = &persistencemocks.BackupStore{}
+	)
+
+	r := NewRestoreReconciler(
+		context.Background(),
+		velerov1api.DefaultNamespace,
+		nil,
+		fakeClient,
+		logger,
+		logrus.DebugLevel,
+		func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+		NewFakeSingleObjectBackupStoreGetter(backupStore),
+		metrics.NewServerMetrics(),
+		formatFlag,
+		60*time.Minute,
+	)
+
+	restore := &velerov1api.Restore{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "restore-1",
+		},
+		Spec: velerov1api.RestoreSpec{
+			BackupName: "backup-1",
+			ResourceModifier: &corev1.TypedLocalObjectReference{
+				Kind: resourcemodifiers.ConfigmapRefType,
+				Name: "test-configmap",
+			},
+		},
+	}
+
+	location := builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Result()
+	require.NoError(t, r.kbClient.Create(context.Background(), location))
+
+	require.NoError(t, r.kbClient.Create(
+		context.Background(),
+		defaultBackup().
+			ObjectMeta(
+				builder.WithName("backup-1"),
+			).StorageLocation("default").
+			Phase(velerov1api.BackupPhaseCompleted).
+			Result(),
+	))
+
+	r.validateAndComplete(restore)
+	assert.Contains(t, restore.Status.ValidationErrors[0], "failed to get resource modifiers configmap")
+
+	restore1 := &velerov1api.Restore{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "restore-1",
+		},
+		Spec: velerov1api.RestoreSpec{
+			BackupName: "backup-1",
+			ResourceModifier: &corev1.TypedLocalObjectReference{
+				Kind: resourcemodifiers.ConfigmapRefType,
+				Name: "test-configmap",
+			},
+		},
+	}
+
+	cm1 := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-configmap",
+			Namespace: velerov1api.DefaultNamespace,
+		},
+		Data: map[string]string{
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+		},
+	}
+	require.NoError(t, r.kbClient.Create(context.Background(), cm1))
+
+	r.validateAndComplete(restore1)
+	assert.Nil(t, restore1.Status.ValidationErrors)
+
+	restore2 := &velerov1api.Restore{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "restore-1",
+		},
+		Spec: velerov1api.RestoreSpec{
+			BackupName: "backup-1",
+			ResourceModifier: &corev1.TypedLocalObjectReference{
+				Kind: resourcemodifiers.ConfigmapRefType,
+				Name: "test-configmap-invalid",
+			},
+		},
+	}
+
+	invalidVersionCm := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-configmap-invalid",
+			Namespace: velerov1api.DefaultNamespace,
+		},
+		Data: map[string]string{
+			"sub.yml": "version1: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+		},
+	}
+	require.NoError(t, r.kbClient.Create(context.Background(), invalidVersionCm))
+
+	r.validateAndComplete(restore2)
+	assert.Contains(t, restore2.Status.ValidationErrors[0], "Error in parsing resource modifiers provided in configmap")
+
+	restore3 := &velerov1api.Restore{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "restore-1",
+		},
+		Spec: velerov1api.RestoreSpec{
+			BackupName: "backup-1",
+			ResourceModifier: &corev1.TypedLocalObjectReference{
+				Kind: resourcemodifiers.ConfigmapRefType,
+				Name: "test-configmap-invalid-operator",
+			},
+		},
+	}
+
+	invalidOperatorCm := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-configmap-invalid-operator",
+			Namespace: velerov1api.DefaultNamespace,
+		},
+		Data: map[string]string{
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: invalid\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+		},
+	}
+	require.NoError(t, r.kbClient.Create(context.Background(), invalidOperatorCm))
+
+	r.validateAndComplete(restore3)
+	assert.Contains(t, restore3.Status.ValidationErrors[0], "Validation error in resource modifiers provided in configmap")
+}
+
 func TestBackupXorScheduleProvided(t *testing.T) {
 	r := &velerov1api.Restore{}
 	assert.False(t, backupXorScheduleProvided(r))