From 7c60829f38f66d811ce0242f91d485d1356c1152 Mon Sep 17 00:00:00 2001
From: Steve Kriss <krisss@vmware.com>
Date: Thu, 12 Dec 2019 13:45:41 -0700
Subject: [PATCH] bug fix: don't restore cluster-scoped resources by default
 when restoring specific namespaces (#2118)

* add failing test case for bug

Signed-off-by: Steve Kriss <krisss@vmware.com>
---
 changelogs/unreleased/2118-skriss |  1 +
 pkg/restore/restore.go            |  5 ++++
 pkg/restore/restore_test.go       | 40 +++++++++++++++++++++++++++----
 3 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/2118-skriss

diff --git a/changelogs/unreleased/2118-skriss b/changelogs/unreleased/2118-skriss
new file mode 100644
index 000000000..34e8dc08f
--- /dev/null
+++ b/changelogs/unreleased/2118-skriss
@@ -0,0 +1 @@
+bug fix: don't restore cluster-scoped resources when restoring specific namespaces and IncludeClusterResources is nil
diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go
index c4529332a..fa6a8d40d 100644
--- a/pkg/restore/restore.go
+++ b/pkg/restore/restore.go
@@ -663,6 +663,11 @@ func (ctx *context) restoreResource(resource, targetNamespace, originalNamespace
 		return warnings, errs
 	}
 
+	if targetNamespace == "" && !boolptr.IsSetToTrue(ctx.restore.Spec.IncludeClusterResources) && !ctx.namespaceIncludesExcludes.IncludeEverything() {
+		ctx.log.Infof("Skipping resource %s because it's cluster-scoped and only specific namespaces are included in the restore", resource)
+		return warnings, errs
+	}
+
 	if targetNamespace != "" {
 		ctx.log.Infof("Restoring resource '%s' into namespace '%s'", resource, targetNamespace)
 	} else {
diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
index 581147629..29c668214 100644
--- a/pkg/restore/restore_test.go
+++ b/pkg/restore/restore_test.go
@@ -311,6 +311,36 @@ func TestRestoreResourceFiltering(t *testing.T) {
 			want: map[*test.APIResource][]string{
 				test.Pods():        {"ns-1/pod-1"},
 				test.Deployments(): {"ns-1/deploy-1"},
+				test.PVs():         {},
+			},
+		},
+		{
+			name:    "should not include cluster-scoped resources if restoring subset of namespaces and IncludeClusterResources=nil",
+			restore: defaultRestore().IncludedNamespaces("ns-1").Result(),
+			backup:  defaultBackup().Result(),
+			tarball: newTarWriter(t).
+				addItems("pods",
+					builder.ForPod("ns-1", "pod-1").Result(),
+					builder.ForPod("ns-2", "pod-2").Result(),
+				).
+				addItems("deployments.apps",
+					builder.ForDeployment("ns-1", "deploy-1").Result(),
+					builder.ForDeployment("ns-2", "deploy-2").Result(),
+				).
+				addItems("persistentvolumes",
+					builder.ForPersistentVolume("pv-1").Result(),
+					builder.ForPersistentVolume("pv-2").Result(),
+				).
+				done(),
+			apiResources: []*test.APIResource{
+				test.Pods(),
+				test.Deployments(),
+				test.PVs(),
+			},
+			want: map[*test.APIResource][]string{
+				test.Pods():        {"ns-1/pod-1"},
+				test.Deployments(): {"ns-1/deploy-1"},
+				test.PVs():         {},
 			},
 		},
 		{
@@ -1378,7 +1408,7 @@ func TestRestoreActionAdditionalItems(t *testing.T) {
 			},
 		},
 		{
-			name:    "when using a restore namespace filter, additional items that are cluster-scoped are restored",
+			name:    "when using a restore namespace filter, additional items that are cluster-scoped are restored when IncludeClusterResources=nil",
 			restore: defaultRestore().IncludedNamespaces("ns-1").Result(),
 			backup:  defaultBackup().Result(),
 			tarball: newTarWriter(t).
@@ -1404,8 +1434,8 @@ func TestRestoreActionAdditionalItems(t *testing.T) {
 			},
 		},
 		{
-			name:    "when using a restore resource filter, additional items that are non-included resources are not restored",
-			restore: defaultRestore().IncludedResources("pods").Result(),
+			name:    "additional items that are cluster-scoped are not restored when IncludeClusterResources=false",
+			restore: defaultRestore().IncludeClusterResources(false).Result(),
 			backup:  defaultBackup().Result(),
 			tarball: newTarWriter(t).
 				addItems("pods", builder.ForPod("ns-1", "pod-1").Result()).
@@ -1430,8 +1460,8 @@ func TestRestoreActionAdditionalItems(t *testing.T) {
 			},
 		},
 		{
-			name:    "when IncludeClusterResources=false, additional items that are cluster-scoped are not restored",
-			restore: defaultRestore().IncludeClusterResources(false).Result(),
+			name:    "when using a restore resource filter, additional items that are non-included resources are not restored",
+			restore: defaultRestore().IncludedResources("pods").Result(),
 			backup:  defaultBackup().Result(),
 			tarball: newTarWriter(t).
 				addItems("pods", builder.ForPod("ns-1", "pod-1").Result()).