Check whether the namespaces specified in namespace filter exist.

Check whether the namespaces specified in the
backup.Spec.IncludeNamespaces exist during backup resource collcetion
If not, log error to mark the backup as PartiallyFailed.

Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>

changelogs/unreleased/7965-blackpiglet

@@ -0,0 +1 @@
+Check whether the namespaces specified in namespace filter exist.

pkg/backup/item_collector.go

@@ -741,6 +741,23 @@ func (r *itemCollector) collectNamespaces(
 		return nil, errors.WithStack(err)
 	}
 
+	for _, includedNSName := range r.backupRequest.Backup.Spec.IncludedNamespaces {
+		nsExists := false
+		// Skip checking the namespace existing when it's "*".
+		if includedNSName == "*" {
+			continue
+		}
+		for _, unstructuredNS := range unstructuredList.Items {
+			if unstructuredNS.GetName() == includedNSName {
+				nsExists = true
+			}
+		}
+
+		if !nsExists {
+			log.Errorf("fail to get the namespace %s specified in backup.Spec.IncludedNamespaces", includedNSName)
+		}
+	}
+
 	var singleSelector labels.Selector
 	var orSelectors []labels.Selector
 

pkg/backup/item_collector_test.go

@@ -224,6 +224,17 @@ func TestItemCollectorBackupNamespaces(t *testing.T) {
 			},
 			expectedTrackedNS: []string{"ns1", "ns2"},
 		},
+		{
+			name:   "ns specified by the IncludeNamespaces cannot be found",
+			backup: builder.ForBackup("velero", "backup").IncludedNamespaces("ns1", "invalid", "*").Result(),
+			ie:     collections.NewIncludesExcludes().Includes("ns1", "invalid", "*"),
+			namespaces: []*corev1.Namespace{
+				builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(),
+				builder.ForNamespace("ns2").Result(),
+				builder.ForNamespace("ns3").Result(),
+			},
+			expectedTrackedNS: []string{"ns1"},
+		},
 	}
 
 	for _, tc := range tests {

pkg/controller/backup_controller.go

@@ -464,7 +464,7 @@ func (b *backupReconciler) prepareBackupRequest(backup *velerov1api.Backup, logg
 	}
 
 	// validate the included/excluded namespaces
-	for _, err := range b.validateNamespaceIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) {
+	for _, err := range collections.ValidateNamespaceIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) {
 		request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))
 	}
 
@@ -596,24 +596,6 @@ 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.

pkg/controller/backup_controller_test.go

@@ -190,16 +190,10 @@ func TestProcessBackupValidationFailures(t *testing.T) {
 		},
 		{
 			name:           "use old filter parameters and new filter parameters together",
-			backup:         defaultBackup().IncludeClusterResources(true).IncludedNamespaceScopedResources("Deployment").IncludedNamespaces("foo").Result(),
+			backup:         defaultBackup().IncludeClusterResources(true).IncludedNamespaceScopedResources("Deployment").IncludedNamespaces("default").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 {
@@ -214,11 +208,10 @@ 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, namespace)
+				fakeClient = velerotest.NewFakeControllerRuntimeClient(t, test.backupLocation)
 			} else {
-				fakeClient = velerotest.NewFakeControllerRuntimeClient(t, namespace)
+				fakeClient = velerotest.NewFakeControllerRuntimeClient(t)
 			}
 
 			c := &backupReconciler{
@@ -1574,43 +1567,6 @@ 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.