Merge pull request #7757 from kaovilai/addExistingResourcePolicyResto…

…reCRValidation

Add existingResourcePolicy restore CR validation to controller.

changelogs/unreleased/7757-kaovilai

@@ -0,0 +1 @@
+Add existingResourcePolicy restore CR validation to controller

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

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
-}

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(),

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 != "" {

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:     &timestamp,
+			expectedCompletedTime: &timestamp,
+			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:     &timestamp,
+			expectedCompletedTime: &timestamp,
+			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:     &timestamp,
+			expectedCompletedTime: &timestamp,
+			expectedRestorerCall:  nil, // this restore should fail validation and not be passed to the restorer
+		},
 		{
 			name:                  "valid restore gets executed",
 			location:              defaultStorageLocation,

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
+}

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(""))
+}