diff --git a/changelogs/unreleased/7757-kaovilai b/changelogs/unreleased/7757-kaovilai new file mode 100644 index 000000000..92317ed6b --- /dev/null +++ b/changelogs/unreleased/7757-kaovilai @@ -0,0 +1 @@ +Add existingResourcePolicy restore CR validation to controller diff --git a/hack/update-1fmt.sh b/hack/update-1fmt.sh index 204801bdf..a23261cb2 100755 --- a/hack/update-1fmt.sh +++ b/hack/update-1fmt.sh @@ -33,7 +33,7 @@ if ! command -v goimports > /dev/null; then exit 1 fi -files="$(find . -type f -name '*.go' -not -path './.go/*' -not -path './vendor/*' -not -path './site/*' -not -path '*/generated/*' -not -name 'zz_generated*' -not -path '*/mocks/*')" +files="$(find . -type f -name '*.go' -not -path './.go/*' -not -path './vendor/*' -not -path './site/*' -not -path './.git/*' -not -path '*/generated/*' -not -name 'zz_generated*' -not -path '*/mocks/*')" echo "${ACTION} gofmt" output=$(gofmt "${MODE}" -s ${files}) if [[ -n "${output}" ]]; then diff --git a/pkg/cmd/cli/restore/create.go b/pkg/cmd/cli/restore/create.go index e855ed99c..6d4b25f0a 100644 --- a/pkg/cmd/cli/restore/create.go +++ b/pkg/cmd/cli/restore/create.go @@ -39,6 +39,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd/util/output" "github.com/vmware-tanzu/velero/pkg/util/boolptr" "github.com/vmware-tanzu/velero/pkg/util/kube" + "github.com/vmware-tanzu/velero/pkg/util/velero/restore" ) func NewCreateCommand(f client.Factory, use string) *cobra.Command { @@ -199,7 +200,7 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return errors.New("either a 'selector' or an 'or-selector' can be specified, but not both") } - if len(o.ExistingResourcePolicy) > 0 && !isResourcePolicyValid(o.ExistingResourcePolicy) { + if len(o.ExistingResourcePolicy) > 0 && !restore.IsResourcePolicyValid(o.ExistingResourcePolicy) { return errors.New("existing-resource-policy has invalid value, it accepts only none, update as value") } @@ -428,10 +429,3 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { return nil } - -func isResourcePolicyValid(resourcePolicy string) bool { - if resourcePolicy == string(api.PolicyTypeNone) || resourcePolicy == string(api.PolicyTypeUpdate) { - return true - } - return false -} diff --git a/pkg/cmd/cli/restore/create_test.go b/pkg/cmd/cli/restore/create_test.go index 4fbe7f372..bf771bfe3 100644 --- a/pkg/cmd/cli/restore/create_test.go +++ b/pkg/cmd/cli/restore/create_test.go @@ -34,12 +34,6 @@ import ( velerotest "github.com/vmware-tanzu/velero/pkg/test" ) -func TestIsResourcePolicyValid(t *testing.T) { - require.True(t, isResourcePolicyValid(string(velerov1api.PolicyTypeNone))) - require.True(t, isResourcePolicyValid(string(velerov1api.PolicyTypeUpdate))) - require.False(t, isResourcePolicyValid("")) -} - func TestMostRecentBackup(t *testing.T) { backups := []velerov1api.Backup{ *builder.ForBackup(cmdtest.VeleroNameSpace, "backup0").StartTimestamp(time.Now().Add(3 * time.Second)).Phase(velerov1api.BackupPhaseDeleting).Result(), diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 86372b453..31b060e12 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -57,6 +57,7 @@ import ( kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube" "github.com/vmware-tanzu/velero/pkg/util/logging" "github.com/vmware-tanzu/velero/pkg/util/results" + pkgrestoreUtil "github.com/vmware-tanzu/velero/pkg/util/velero/restore" ) // nonRestorableResources is an exclusion list for the restoration process. Any resources @@ -346,6 +347,11 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf } } + // validate ExistingResourcePolicy + if restore.Spec.ExistingResourcePolicy != "" && !pkgrestoreUtil.IsResourcePolicyValid(string(restore.Spec.ExistingResourcePolicy)) { + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("Invalid ExistingResourcePolicy: %s", restore.Spec.ExistingResourcePolicy)) + } + // if ScheduleName is specified, fill in BackupName with the most recent successful backup from // the schedule if restore.Spec.ScheduleName != "" { diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 1b60f9532..e124a98e9 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -312,6 +312,39 @@ func TestRestoreReconcile(t *testing.T) { expectedRestoreErrors: 1, expectedRestorerCall: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseInProgress).Result(), }, + { + name: "valid restore with none existingresourcepolicy gets executed", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("none").Result(), + backup: defaultBackup().StorageLocation("default").Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseInProgress), + expectedStartTime: ×tamp, + expectedCompletedTime: ×tamp, + expectedRestorerCall: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseInProgress).ExistingResourcePolicy("none").Result(), + }, + { + name: "valid restore with update existingresourcepolicy gets executed", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("update").Result(), + backup: defaultBackup().StorageLocation("default").Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseInProgress), + expectedStartTime: ×tamp, + expectedCompletedTime: ×tamp, + expectedRestorerCall: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseInProgress).ExistingResourcePolicy("update").Result(), + }, + { + name: "invalid restore with invalid existingresourcepolicy errors", + location: defaultStorageLocation, + restore: NewRestore("foo", "invalidexistingresourcepolicy", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("invalid").Result(), + backup: defaultBackup().StorageLocation("default").Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedStartTime: ×tamp, + expectedCompletedTime: ×tamp, + expectedRestorerCall: nil, // this restore should fail validation and not be passed to the restorer + }, { name: "valid restore gets executed", location: defaultStorageLocation, diff --git a/pkg/util/velero/restore/util.go b/pkg/util/velero/restore/util.go new file mode 100644 index 000000000..e0812884b --- /dev/null +++ b/pkg/util/velero/restore/util.go @@ -0,0 +1,12 @@ +package restore + +import ( + api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func IsResourcePolicyValid(resourcePolicy string) bool { + if resourcePolicy == string(api.PolicyTypeNone) || resourcePolicy == string(api.PolicyTypeUpdate) { + return true + } + return false +} diff --git a/pkg/util/velero/restore/util_test.go b/pkg/util/velero/restore/util_test.go new file mode 100644 index 000000000..be72ff8ba --- /dev/null +++ b/pkg/util/velero/restore/util_test.go @@ -0,0 +1,15 @@ +package restore + +import ( + "testing" + + "github.com/stretchr/testify/require" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func TestIsResourcePolicyValid(t *testing.T) { + require.True(t, IsResourcePolicyValid(string(velerov1api.PolicyTypeNone))) + require.True(t, IsResourcePolicyValid(string(velerov1api.PolicyTypeUpdate))) + require.False(t, IsResourcePolicyValid("")) +}