diff --git a/internal/resourcemodifiers/resource_modifiers_test.go b/internal/resourcemodifiers/resource_modifiers_test.go index fb47ab061..f795d18e5 100644 --- a/internal/resourcemodifiers/resource_modifiers_test.go +++ b/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) { diff --git a/internal/resourcemodifiers/resource_modifiers_validator.go b/internal/resourcemodifiers/resource_modifiers_validator.go index 7ef9a5f65..623989e23 100644 --- a/internal/resourcemodifiers/resource_modifiers_validator.go +++ b/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 } } diff --git a/internal/resourcemodifiers/resource_modifiers_validator_test.go b/internal/resourcemodifiers/resource_modifiers_validator_test.go index 22d5cfab6..ed640b1ff 100644 --- a/internal/resourcemodifiers/resource_modifiers_validator_test.go +++ b/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) - } - }) - } -} diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 3e8c21677..9cb07eaaf 100644 --- a/pkg/controller/restore_controller_test.go +++ b/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))