From b0bdaeea735d79b2528c4f2375c320833839f47d Mon Sep 17 00:00:00 2001
From: Adnan Abdulhussein <aadnan@vmware.com>
Date: Mon, 24 Jun 2019 14:50:25 -0700
Subject: [PATCH] ensure correct backup item actions run with namespace
 selector (#1601)

* ensure correct backup item actions run with namespace selector

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>

* changelog

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>

* don't run backup item actions for namespace resources

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>

* simplify cluster-scope resources checks

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>
---
 changelogs/unreleased/1601-prydonius |  1 +
 pkg/backup/backup_new_test.go        |  9 ++++++---
 pkg/backup/item_backupper.go         |  5 +++++
 pkg/test/resources.go                | 21 +++++++++++++++++++++
 4 files changed, 33 insertions(+), 3 deletions(-)
 create mode 100644 changelogs/unreleased/1601-prydonius

diff --git a/changelogs/unreleased/1601-prydonius b/changelogs/unreleased/1601-prydonius
new file mode 100644
index 000000000..7fc955ddc
--- /dev/null
+++ b/changelogs/unreleased/1601-prydonius
@@ -0,0 +1 @@
+ensure correct backup item actions run with namespace selector
diff --git a/pkg/backup/backup_new_test.go b/pkg/backup/backup_new_test.go
index 6dcf30f06..e7220fcc6 100644
--- a/pkg/backup/backup_new_test.go
+++ b/pkg/backup/backup_new_test.go
@@ -806,8 +806,7 @@ func TestBackupActionsRunForCorrectItems(t *testing.T) {
 			},
 		},
 		{
-			// TODO this seems like a bug
-			name: "single action with a namespace selector runs for resources in that namespace plus cluster-scoped resources",
+			name: "single action with a namespace selector runs only for resources in that namespace",
 			backup: defaultBackup().
 				Backup(),
 			apiResources: []*test.APIResource{
@@ -823,9 +822,13 @@ func TestBackupActionsRunForCorrectItems(t *testing.T) {
 					test.NewPV("pv-1"),
 					test.NewPV("pv-2"),
 				),
+				test.Namespaces(
+					test.NewNamespace("ns-1"),
+					test.NewNamespace("ns-2"),
+				),
 			},
 			actions: map[*recordResourcesAction][]string{
-				new(recordResourcesAction).ForNamespace("ns-1"): {"ns-1/pod-1", "ns-1/pvc-1", "pv-1", "pv-2"},
+				new(recordResourcesAction).ForNamespace("ns-1"): {"ns-1/pod-1", "ns-1/pvc-1"},
 			},
 		},
 		{
diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go
index 189f562c0..b581841cc 100644
--- a/pkg/backup/item_backupper.go
+++ b/pkg/backup/item_backupper.go
@@ -302,6 +302,11 @@ func (ib *defaultItemBackupper) executeActions(
 			continue
 		}
 
+		if namespace == "" && !action.namespaceIncludesExcludes.IncludeEverything() {
+			log.Debug("Skipping action because resource is cluster-scoped and action only applies to specific namespaces")
+			continue
+		}
+
 		if !action.selector.Matches(labels.Set(metadata.GetLabels())) {
 			log.Debug("Skipping action because label selector does not match")
 			continue
diff --git a/pkg/test/resources.go b/pkg/test/resources.go
index f40fc2c5c..95f6de044 100644
--- a/pkg/test/resources.go
+++ b/pkg/test/resources.go
@@ -110,6 +110,17 @@ func ExtensionsDeployments(items ...metav1.Object) *APIResource {
 	}
 }
 
+func Namespaces(items ...metav1.Object) *APIResource {
+	return &APIResource{
+		Group:      "",
+		Version:    "v1",
+		Name:       "namespaces",
+		ShortName:  "ns",
+		Namespaced: false,
+		Items:      items,
+	}
+}
+
 func NewPod(ns, name string) *corev1.Pod {
 	return &corev1.Pod{
 		TypeMeta: metav1.TypeMeta{
@@ -160,6 +171,16 @@ func NewDeployment(ns, name string) *appsv1.Deployment {
 	}
 }
 
+func NewNamespace(name string) *corev1.Namespace {
+	return &corev1.Namespace{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Namespace",
+			APIVersion: "v1",
+		},
+		ObjectMeta: objectMeta("", name),
+	}
+}
+
 func objectMeta(ns, name string) metav1.ObjectMeta {
 	return metav1.ObjectMeta{
 		Namespace: ns,