Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check the existence of the namespaces provided in the "--include-namespaces" option #7569

Merged
merged 1 commit into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/7569-ywk253100
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check the existence of the namespaces provided in the "--include-namespaces" option
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
Loading