From 7e85aafc17dcf5aeb117c6aae696e332d377632c Mon Sep 17 00:00:00 2001
From: Tiger Kaovilai <tkaovila@redhat.com>
Date: Wed, 23 Oct 2024 16:11:51 -0500
Subject: [PATCH] fix itemcollector

---
 pkg/backup/backup.go         |  1 -
 pkg/backup/backup_test.go    |  6 ++++--
 pkg/backup/item_collector.go | 35 ++++++++++++++++++++++++++++++-----
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go
index 0304e71ced2..8e6e3c5a427 100644
--- a/pkg/backup/backup.go
+++ b/pkg/backup/backup.go
@@ -239,7 +239,6 @@ func (kb *kubernetesBackupper) BackupWithResolvers(
 	if err := kb.writeBackupVersion(tw); err != nil {
 		return errors.WithStack(err)
 	}
-
 	backupRequest.NamespaceIncludesExcludes = getNamespaceIncludesExcludes(backupRequest.Backup)
 	log.Infof("Including namespaces: %s", backupRequest.NamespaceIncludesExcludes.IncludesString())
 	log.Infof("Excluding namespaces: %s", backupRequest.NamespaceIncludesExcludes.ExcludesString())
diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go
index 77969cfe966..74745c0492e 100644
--- a/pkg/backup/backup_test.go
+++ b/pkg/backup/backup_test.go
@@ -286,10 +286,12 @@ func TestBackupOldResourceFiltering(t *testing.T) {
 				),
 			},
 			want: []string{
-				"resources/pods/namespaces/foo/bar.json",
 				"resources/deployments.apps/namespaces/foo/bar.json",
-				"resources/pods/v1-preferredversion/namespaces/foo/bar.json",
 				"resources/deployments.apps/v1-preferredversion/namespaces/foo/bar.json",
+				"resources/pods/v1-preferredversion/namespaces/foo/bar.json",
+				"resources/pods/namespaces/foo/bar.json",
+				"resources/namespaces/cluster/foo.json",
+				"resources/namespaces/v1-preferredversion/cluster/foo.json",
 			},
 		},
 		// {
diff --git a/pkg/backup/item_collector.go b/pkg/backup/item_collector.go
index a1fce5ee745..b4c464f6a54 100644
--- a/pkg/backup/item_collector.go
+++ b/pkg/backup/item_collector.go
@@ -46,6 +46,8 @@ import (
 type itemCollector struct {
 	log                   logrus.FieldLogger
 	backupRequest         *Request
+	// Namespaces that are included by the backup's labelSelector will backup all resources in that namespace even if resources are not labeled.
+	namespacesIncludedByLabelSelector map[string]struct{}
 	discoveryHelper       discovery.Helper
 	dynamicFactory        client.DynamicFactory
 	cohabitatingResources map[string]*cohabitatingResource
@@ -436,13 +438,29 @@ func (r *itemCollector) getResourceItems(
 	// Namespace are filtered by namespace include/exclude filters,
 	// backup LabelSelectors and OrLabelSelectors are checked too.
 	if gr == kuberesource.Namespaces {
-		return r.collectNamespaces(
+		namespaces, err := r.collectNamespaces(
 			resource,
 			gv,
 			gr,
 			preferredGVR,
 			log,
 		)
+		// The namespaces collected contains namespaces selected by label selector.
+		// Per https://github.com/vmware-tanzu/velero/issues/7492#issuecomment-1986146411 we want these to act as IncludedNamespaces so add to stringset.
+		for i := range namespaces {
+			if namespaces[i] != nil {
+				if r.backupRequest.NamespaceIncludesExcludes.ShouldInclude(namespaces[i].name) {
+					// this namespace is included by the backup's label selector, not the namespace include/exclude filter
+					// this is a special case where we want to include this namespace in the backup and all resources in it regardless of the resource's label selector
+					if r.namespacesIncludedByLabelSelector == nil {
+						r.namespacesIncludedByLabelSelector = make(map[string]struct{})
+					}
+					r.namespacesIncludedByLabelSelector[namespaces[i].name] = struct{}{}
+				}
+				r.backupRequest.NamespaceIncludesExcludes.Includes(namespaces[i].name)
+			}
+		}
+		return namespaces, err
 	}
 
 	clusterScoped := !resource.Namespaced
@@ -509,9 +527,13 @@ func (r *itemCollector) listResourceByLabelsPerNamespace(
 		logger.WithError(err).Error("Error getting dynamic client")
 		return nil, err
 	}
-
+	ignoreItemLabels := false
+	if _, namespaceSelectedByLabelSelector := r.namespacesIncludedByLabelSelector[namespace]; namespaceSelectedByLabelSelector {
+		logger.Infof("Listing all items in namespace %s because ns was selected by the backup's label selector", namespace)
+		ignoreItemLabels = true
+	}
 	var orLabelSelectors []string
-	if r.backupRequest.Spec.OrLabelSelectors != nil {
+	if r.backupRequest.Spec.OrLabelSelectors != nil && !ignoreItemLabels {
 		for _, s := range r.backupRequest.Spec.OrLabelSelectors {
 			orLabelSelectors = append(orLabelSelectors, metav1.FormatLabelSelector(s))
 		}
@@ -537,7 +559,7 @@ func (r *itemCollector) listResourceByLabelsPerNamespace(
 	}
 
 	var labelSelector string
-	if selector := r.backupRequest.Spec.LabelSelector; selector != nil {
+	if selector := r.backupRequest.Spec.LabelSelector; selector != nil && !ignoreItemLabels {
 		labelSelector = metav1.FormatLabelSelector(selector)
 	}
 
@@ -594,7 +616,8 @@ func sortCoreGroup(group *metav1.APIResourceList) {
 // pod is backed up, we can perform a pre hook, then process pvcs and pvs (including taking a
 // snapshot), then perform a post hook on the pod.
 const (
-	pod = iota
+	namespaces = iota
+	pod
 	pvc
 	pv
 	other
@@ -604,6 +627,8 @@ const (
 // pods, pvcs, pvs, everything else.
 func coreGroupResourcePriority(resource string) int {
 	switch strings.ToLower(resource) {
+	case "namespaces":
+		return namespaces
 	case "pods":
 		return pod
 	case "persistentvolumeclaims":