Add new resource filters can separate cluster and namespace scope res…

…ources.

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

changelogs/unreleased/5838-blackpiglet

@@ -0,0 +1 @@
+Add new resource filters can separate cluster and namespace scope resources.

config/crd/v1/bases/velero.io_backups.yaml

@@ -54,6 +54,22 @@ spec:
                   Use DefaultVolumesToFsBackup instead."
                 nullable: true
                 type: boolean
+              excludedClusterScopeResources:
+                description: ExcludedClusterScopeResources is a slice of cluster scope
+                  resource type names to exclude from the backup. If set to "*", all
+                  cluster scope resource types are excluded.
+                items:
+                  type: string
+                nullable: true
+                type: array
+              excludedNamespacedResources:
+                description: ExcludedNamespacedResources is a slice of namespace scope
+                  resource type names to exclude from the backup. If set to "*", all
+                  namespace scope resource types are excluded.
+                items:
+                  type: string
+                nullable: true
+                type: array
               excludedNamespaces:
                 description: ExcludedNamespaces contains a list of namespaces that
                   are not included in the backup.
@@ -259,6 +275,23 @@ spec:
                   resources should be included for consideration in the backup.
                 nullable: true
                 type: boolean
+              includedClusterScopeResources:
+                description: IncludedClusterScopeResources is a slice of cluster scope
+                  resource type names to include in the backup. If set to "*", all
+                  cluster scope resource types are included. The default value is
+                  empty, which means only related cluster scope resources are included.
+                items:
+                  type: string
+                nullable: true
+                type: array
+              includedNamespacedResources:
+                description: IncludedNamespacedResources is a slice of namespace scope
+                  resource type names to include in the backup. The default value
+                  is "*".
+                items:
+                  type: string
+                nullable: true
+                type: array
               includedNamespaces:
                 description: IncludedNamespaces is a slice of namespace names to include
                   objects from. If empty, all namespaces are included.

config/crd/v1/bases/velero.io_schedules.yaml

@@ -84,6 +84,22 @@ spec:
                       entirely in future. Use DefaultVolumesToFsBackup instead."
                     nullable: true
                     type: boolean
+                  excludedClusterScopeResources:
+                    description: ExcludedClusterScopeResources is a slice of cluster
+                      scope resource type names to exclude from the backup. If set
+                      to "*", all cluster scope resource types are excluded.
+                    items:
+                      type: string
+                    nullable: true
+                    type: array
+                  excludedNamespacedResources:
+                    description: ExcludedNamespacedResources is a slice of namespace
+                      scope resource type names to exclude from the backup. If set
+                      to "*", all namespace scope resource types are excluded.
+                    items:
+                      type: string
+                    nullable: true
+                    type: array
                   excludedNamespaces:
                     description: ExcludedNamespaces contains a list of namespaces
                       that are not included in the backup.
@@ -294,6 +310,24 @@ spec:
                       resources should be included for consideration in the backup.
                     nullable: true
                     type: boolean
+                  includedClusterScopeResources:
+                    description: IncludedClusterScopeResources is a slice of cluster
+                      scope resource type names to include in the backup. If set to
+                      "*", all cluster scope resource types are included. The default
+                      value is empty, which means only related cluster scope resources
+                      are included.
+                    items:
+                      type: string
+                    nullable: true
+                    type: array
+                  includedNamespacedResources:
+                    description: IncludedNamespacedResources is a slice of namespace
+                      scope resource type names to include in the backup. The default
+                      value is "*".
+                    items:
+                      type: string
+                    nullable: true
+                    type: array
                   includedNamespaces:
                     description: IncludedNamespaces is a slice of namespace names
                       to include objects from. If empty, all namespaces are included.

pkg/apis/velero/v1/backup.go

@@ -52,6 +52,36 @@ type BackupSpec struct {
 	// +nullable
 	ExcludedResources []string `json:"excludedResources,omitempty"`
 
+	// IncludedClusterScopeResources is a slice of cluster scope
+	// resource type names to include in the backup.
+	// If set to "*", all cluster scope resource types are included.
+	// The default value is empty, which means only related cluster
+	// scope resources are included.
+	// +optional
+	// +nullable
+	IncludedClusterScopeResources []string `json:"includedClusterScopeResources,omitempty"`
+
+	// ExcludedClusterScopeResources is a slice of cluster scope
+	// resource type names to exclude from the backup.
+	// If set to "*", all cluster scope resource types are excluded.
+	// +optional
+	// +nullable
+	ExcludedClusterScopeResources []string `json:"excludedClusterScopeResources,omitempty"`
+
+	// IncludedNamespacedResources is a slice of namespace scope
+	// resource type names to include in the backup.
+	// The default value is "*".
+	// +optional
+	// +nullable
+	IncludedNamespacedResources []string `json:"includedNamespacedResources,omitempty"`
+
+	// ExcludedNamespacedResources is a slice of namespace scope
+	// resource type names to exclude from the backup.
+	// If set to "*", all namespace scope resource types are excluded.
+	// +optional
+	// +nullable
+	ExcludedNamespacedResources []string `json:"excludedNamespacedResources,omitempty"`
+
 	// LabelSelector is a metav1.LabelSelector to filter with
 	// when adding individual objects to the backup. If empty
 	// or nil, all objects are included. Optional.

pkg/backup/backup.go

@@ -205,6 +205,15 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,
 	backupRequest.ResourceIncludesExcludes = collections.GetResourceIncludesExcludes(kb.discoveryHelper, backupRequest.Spec.IncludedResources, backupRequest.Spec.ExcludedResources)
 	log.Infof("Including resources: %s", backupRequest.ResourceIncludesExcludes.IncludesString())
 	log.Infof("Excluding resources: %s", backupRequest.ResourceIncludesExcludes.ExcludesString())
+
+	backupRequest.NamespaceResourceIncludesExcludes = collections.GetScopedResourceIncludesExcludes(kb.discoveryHelper, backupRequest.Spec.IncludedNamespacedResources, backupRequest.Spec.ExcludedNamespacedResources, true)
+	log.Infof("Including namespaced resources: %s", backupRequest.NamespaceResourceIncludesExcludes.IncludesString())
+	log.Infof("Excluding namespaced resources: %s", backupRequest.NamespaceResourceIncludesExcludes.ExcludesString())
+
+	backupRequest.ClusterResourceIncludesExcludes = collections.GetScopedResourceIncludesExcludes(kb.discoveryHelper, backupRequest.Spec.IncludedClusterScopeResources, backupRequest.Spec.ExcludedNamespacedResources, false)
+	log.Infof("Including cluster-scoped resources: %s", backupRequest.ClusterResourceIncludesExcludes.ClusterIncludesString())
+	log.Infof("Excluding cluster-scoped resources: %s", backupRequest.ClusterResourceIncludesExcludes.ExcludesString())
+
 	log.Infof("Backing up all volumes using pod volume backup: %t", boolptr.IsSetToTrue(backupRequest.Backup.Spec.DefaultVolumesToFsBackup))
 
 	var err error
@@ -392,9 +401,11 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,
 	quit <- struct{}{}
 
 	// back up CRD for resource if found. We should only need to do this if we've backed up at least
-	// one item for the resource and IncludeClusterResources is nil. If IncludeClusterResources is false
-	// we don't want to back it up, and if it's true it will already be included.
-	if backupRequest.Spec.IncludeClusterResources == nil {
+	// one item for the resource when IncludeClusterResources is nil, or cluster-scoped resource filters
+	// are not specified. If IncludeClusterResources is false we don't want to back it up,
+	// and if it's true it will already be included.
+	if backupRequest.Spec.IncludeClusterResources == nil ||
+		(len(backupRequest.Spec.IncludedClusterScopeResources) == 0 && len(backupRequest.Spec.ExcludedClusterScopeResources) == 0) {
 		for gr := range backedUpGroupResources {
 			kb.backupCRD(log, gr, itemBackupper)
 		}

pkg/backup/item_backupper.go

@@ -98,17 +98,38 @@ func (ib *itemBackupper) backupItem(logger logrus.FieldLogger, obj runtime.Unstr
 			log.Info("Excluding item because namespace is excluded")
 			return false, nil
 		}
-		// NOTE: we specifically allow namespaces to be backed up even if IncludeClusterResources is
-		// false.
-		if namespace == "" && groupResource != kuberesource.Namespaces && ib.backupRequest.Spec.IncludeClusterResources != nil && !*ib.backupRequest.Spec.IncludeClusterResources {
-			log.Info("Excluding item because resource is cluster-scoped and backup.spec.includeClusterResources is false")
-			return false, nil
-		}
 
-		if !ib.backupRequest.ResourceIncludesExcludes.ShouldInclude(groupResource.String()) {
-			log.Info("Excluding item because resource is excluded")
-			return false, nil
+		if useOldResourceFilters(ib.backupRequest.Spec) {
+			// NOTE: we specifically allow namespaces to be backed up even if IncludeClusterResources is false.
+			if namespace == "" && groupResource != kuberesource.Namespaces &&
+				ib.backupRequest.Spec.IncludeClusterResources != nil && !*ib.backupRequest.Spec.IncludeClusterResources {
+				log.Info("Excluding item because resource is cluster-scoped and is excluded by cluster filter.")
+				return false, nil
+			}
+
+			if !ib.backupRequest.ResourceIncludesExcludes.ShouldInclude(groupResource.String()) {
+				log.Info("Excluding item because resource is excluded")
+				return false, nil
+			}
+		} else {
+			// NOTE: we specifically allow namespaces to be backed up even if excluded by cluster-scoped resource filters.
+			if namespace == "" && groupResource != kuberesource.Namespaces &&
+				!ib.backupRequest.ClusterResourceIncludesExcludes.ClusterScopedShouldInclude(groupResource.String()) {
+				log.Info("Excluding item because resource is cluster-scoped and is excluded by cluster filter.")
+				return false, nil
+			}
+
+			if !ib.backupRequest.ClusterResourceIncludesExcludes.ClusterScopedShouldInclude(groupResource.String()) {
+				log.Info("Excluding cluster-scoped item because resource is excluded.")
+				return false, nil
+			}
+
+			if !ib.backupRequest.NamespaceResourceIncludesExcludes.NamespacedShouldInclude(groupResource.String()) {
+				log.Info("Excluding namespaced item because resource is excluded.")
+				return false, nil
+			}
 		}
+
 	}
 
 	if metadata.GetDeletionTimestamp() != nil {
@@ -590,3 +611,26 @@ func zoneFromPVNodeAffinity(res *corev1api.PersistentVolume, topologyKeys ...str
 
 	return "", ""
 }
+
+// useOldResourceFilters checks whether to use old resource filters (IncludeClusterResources,
+// IncludedResources and ExcludedResources), depending the backup's filters setting.
+func useOldResourceFilters(backupSpec velerov1api.BackupSpec) bool {
+	// If all resource filters are none, it is treated as using old parameter filters.
+	if backupSpec.IncludeClusterResources == nil &&
+		len(backupSpec.IncludedResources) == 0 &&
+		len(backupSpec.ExcludedResources) == 0 &&
+		len(backupSpec.IncludedClusterScopeResources) == 0 &&
+		len(backupSpec.ExcludedClusterScopeResources) == 0 &&
+		len(backupSpec.IncludedNamespacedResources) == 0 &&
+		len(backupSpec.ExcludedNamespacedResources) == 0 {
+		return true
+	}
+
+	if backupSpec.IncludeClusterResources != nil ||
+		len(backupSpec.IncludedResources) > 0 ||
+		len(backupSpec.ExcludedResources) > 0 {
+		return true
+	}
+
+	return false
+}

pkg/backup/item_backupper_test.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 
+	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/builder"
 )
 
@@ -170,3 +171,41 @@ func Test_zoneFromPVNodeAffinity(t *testing.T) {
 		})
 	}
 }
+
+func TestUseOldResourceFilters(t *testing.T) {
+	tests := []struct {
+		name                  string
+		backup                velerov1api.Backup
+		useOldResourceFilters bool
+	}{
+		{
+			name:                  "backup with no filters should use old filters",
+			backup:                *defaultBackup().Result(),
+			useOldResourceFilters: true,
+		},
+		{
+			name:                  "backup with only old filters should use old filters",
+			backup:                *defaultBackup().IncludeClusterResources(true).Result(),
+			useOldResourceFilters: true,
+		},
+		{
+			name:                  "backup with only new filters should use new filters",
+			backup:                *defaultBackup().IncludedClusterScopeResources("StorageClass").Result(),
+			useOldResourceFilters: false,
+		},
+		{
+			// This case should not happen in Velero workflow, because filter validation not old and new
+			// filters used together. So this is only used for UT checking, and I assume old filters
+			// have higher priority, because old parameter should be the default one.
+			name:                  "backup with both old and new filters should use old filters",
+			backup:                *defaultBackup().IncludeClusterResources(true).IncludedClusterScopeResources("StorageClass").Result(),
+			useOldResourceFilters: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			assert.Equal(t, test.useOldResourceFilters, useOldResourceFilters(test.backup.Spec))
+		})
+	}
+}

pkg/backup/item_collector.go

@@ -183,29 +183,61 @@ func (r *itemCollector) getResourceItems(log logrus.FieldLogger, gv schema.Group
 	}
 
 	// If the resource we are backing up is NOT namespaces, and it is cluster-scoped, check to see if
-	// we should include it based on the IncludeClusterResources setting.
+	// we should include it based on the IncludeClusterResources or cluster-scoped filters setting.
 	if gr != kuberesource.Namespaces && clusterScoped {
-		if r.backupRequest.Spec.IncludeClusterResources == nil {
-			if !r.backupRequest.NamespaceIncludesExcludes.IncludeEverything() {
-				// when IncludeClusterResources == nil (auto), only directly
-				// back up cluster-scoped resources if we're doing a full-cluster
-				// (all namespaces) backup. Note that in the case of a subset of
-				// namespaces being backed up, some related cluster-scoped resources
-				// may still be backed up if triggered by a custom action (e.g. PVC->PV).
-				// If we're processing namespaces themselves, we will not skip here, they may be
-				// filtered out later.
-				log.Info("Skipping resource because it's cluster-scoped and only specific namespaces are included in the backup")
+		if useOldResourceFilters(r.backupRequest.Spec) {
+			if r.backupRequest.Spec.IncludeClusterResources == nil {
+				if !r.backupRequest.NamespaceIncludesExcludes.IncludeEverything() {
+					// when IncludeClusterResources == nil (auto), only directly
+					// back up cluster-scoped resources if we're doing a full-cluster
+					// (all namespaces) backup. Note that in the case of a subset of
+					// namespaces being backed up, some related cluster-scoped resources
+					// may still be backed up if triggered by a custom action (e.g. PVC->PV).
+					// If we're processing namespaces themselves, we will not skip here, they may be
+					// filtered out later.
+					log.Info("Skipping resource because it's cluster-scoped and only specific namespaces are included in the backup")
+					return nil, nil
+				}
+			} else if !*r.backupRequest.Spec.IncludeClusterResources {
+				log.Info("Skipping resource because it's cluster-scoped")
+				return nil, nil
+			}
+		} else {
+			if len(r.backupRequest.Spec.IncludedClusterScopeResources) == 0 && len(r.backupRequest.Spec.ExcludedClusterScopeResources) == 0 {
+				if !r.backupRequest.NamespaceIncludesExcludes.IncludeEverything() {
+					// when IncludedClusterScopeResources and ExcludedClusterScopeResources
+					// are not specified, only directly back up cluster-scoped resources,
+					// if we're doing a full-cluster
+					// (all namespaces) backup. Note that in the case of a subset of
+					// namespaces being backed up, some related cluster-scoped resources
+					// may still be backed up if triggered by a custom action (e.g. PVC->PV).
+					// If we're processing namespaces themselves, we will not skip here, they may be
+					// filtered out later.
+					log.Info("Skipping resource because it's cluster-scoped and only specific namespaces are included in the backup")
+					return nil, nil
+				}
+			} else if !r.backupRequest.ClusterResourceIncludesExcludes.ClusterScopedShouldInclude(gr.String()) {
+				log.Info("Skipping resource because it's cluster-scoped")
 				return nil, nil
 			}
-		} else if !*r.backupRequest.Spec.IncludeClusterResources {
-			log.Info("Skipping resource because it's cluster-scoped")
-			return nil, nil
 		}
 	}
 
-	if !r.backupRequest.ResourceIncludesExcludes.ShouldInclude(gr.String()) {
-		log.Infof("Skipping resource because it's excluded")
-		return nil, nil
+	if useOldResourceFilters(r.backupRequest.Spec) {
+		if !r.backupRequest.ResourceIncludesExcludes.ShouldInclude(gr.String()) {
+			log.Infof("Skipping resource because it's excluded")
+			return nil, nil
+		}
+	} else {
+		if !r.backupRequest.ClusterResourceIncludesExcludes.ClusterScopedShouldInclude(gr.String()) {
+			log.Info("Skipping cluster-scoped resource, because it's excluded.")
+			return nil, nil
+		}
+
+		if !r.backupRequest.NamespaceResourceIncludesExcludes.NamespacedShouldInclude(gr.String()) {
+			log.Info("Skipping namespaced resource, because it's excluded.")
+			return nil, nil
+		}
 	}
 
 	if cohabitator, found := r.cohabitatingResources[resource.Name]; found {