Skip to content

Commit

Permalink
Check the existence of the namespaces provided in the "--include-name…
Browse files Browse the repository at this point in the history
…spaces" option

Check the existence of the namespaces provided in the "--include-namespaces" opt
ion and reports validation error if not found

Fixes #7431

Signed-off-by: Wenkai Yin(尹文开) <yinw@vmware.com>
  • Loading branch information
ywk253100 committed Mar 27, 2024
1 parent 2518824 commit 6b390d2
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 4 deletions.
20 changes: 19 additions & 1 deletion pkg/controller/backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ func (b *backupReconciler) prepareBackupRequest(backup *velerov1api.Backup, logg
}

// validate the included/excluded namespaces
for _, err := range collections.ValidateNamespaceIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) {
for _, err := range b.validateNamespaceIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) {
request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))
}

Expand Down Expand Up @@ -601,6 +601,24 @@ func (b *backupReconciler) validateAndGetSnapshotLocations(backup *velerov1api.B
return providerLocations, nil
}

func (b *backupReconciler) validateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces []string) []error {
var errs []error
if errs = collections.ValidateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces); len(errs) > 0 {
return errs
}

namespace := &corev1api.Namespace{}
for _, name := range collections.NewIncludesExcludes().Includes(includedNamespaces...).GetIncludes() {
if name == "" || name == "*" {
continue
}
if err := b.kbClient.Get(context.Background(), kbclient.ObjectKey{Name: name}, namespace); err != nil {
errs = append(errs, err)
}
}
return errs
}

// runBackup runs and uploads a validated backup. Any error returned from this function
// causes the backup to be Failed; if no error is returned, the backup's status's Errors
// field is checked to see if the backup was a partial failure.
Expand Down
50 changes: 47 additions & 3 deletions pkg/controller/backup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,16 @@ func TestProcessBackupValidationFailures(t *testing.T) {
},
{
name: "use old filter parameters and new filter parameters together",
backup: defaultBackup().IncludeClusterResources(true).IncludedNamespaceScopedResources("Deployment").IncludedNamespaces("default").Result(),
backup: defaultBackup().IncludeClusterResources(true).IncludedNamespaceScopedResources("Deployment").IncludedNamespaces("foo").Result(),
backupLocation: defaultBackupLocation,
expectedErrs: []string{"include-resources, exclude-resources and include-cluster-resources are old filter parameters.\ninclude-cluster-scoped-resources, exclude-cluster-scoped-resources, include-namespace-scoped-resources and exclude-namespace-scoped-resources are new filter parameters.\nThey cannot be used together"},
},
{
name: "nonexisting namespace",
backup: defaultBackup().IncludedNamespaces("non-existing").Result(),
backupLocation: defaultBackupLocation,
expectedErrs: []string{"Invalid included/excluded namespace lists: namespaces \"non-existing\" not found"},
},
}

for _, test := range tests {
Expand All @@ -203,10 +209,11 @@ func TestProcessBackupValidationFailures(t *testing.T) {
require.NoError(t, err)

var fakeClient kbclient.Client
namespace := builder.ForNamespace("foo").Result()
if test.backupLocation != nil {
fakeClient = velerotest.NewFakeControllerRuntimeClient(t, test.backupLocation)
fakeClient = velerotest.NewFakeControllerRuntimeClient(t, test.backupLocation, namespace)
} else {
fakeClient = velerotest.NewFakeControllerRuntimeClient(t)
fakeClient = velerotest.NewFakeControllerRuntimeClient(t, namespace)
}

c := &backupReconciler{
Expand Down Expand Up @@ -1563,6 +1570,43 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) {
}
}

func TestValidateNamespaceIncludesExcludes(t *testing.T) {
namespace := builder.ForNamespace("default").Result()
reconciler := &backupReconciler{
kbClient: velerotest.NewFakeControllerRuntimeClient(t, namespace),
}

// empty string as includedNamespaces
includedNamespaces := []string{""}
excludedNamespaces := []string{"test"}
errs := reconciler.validateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces)
assert.Empty(t, errs)

// "*" as includedNamespaces
includedNamespaces = []string{"*"}
excludedNamespaces = []string{"test"}
errs = reconciler.validateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces)
assert.Empty(t, errs)

// invalid namespaces
includedNamespaces = []string{"1@#"}
excludedNamespaces = []string{"2@#"}
errs = reconciler.validateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces)
assert.Len(t, errs, 2)

// not exist namespaces
includedNamespaces = []string{"non-existing-namespace"}
excludedNamespaces = []string{}
errs = reconciler.validateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces)
assert.Len(t, errs, 1)

// valid namespaces
includedNamespaces = []string{"default"}
excludedNamespaces = []string{}
errs = reconciler.validateNamespaceIncludesExcludes(includedNamespaces, excludedNamespaces)
assert.Empty(t, errs)
}

// Test_getLastSuccessBySchedule verifies that the getLastSuccessBySchedule helper function correctly returns
// the completion timestamp of the most recent completed backup for each schedule, including an entry for ad-hoc
// or non-scheduled backups.
Expand Down

0 comments on commit 6b390d2

Please sign in to comment.