From b9e21473aede1a580635156d530cfe5c7aba8a44 Mon Sep 17 00:00:00 2001
From: Devan Goodwin <dgoodwin@redhat.com>
Date: Thu, 2 Nov 2017 12:36:52 -0300
Subject: [PATCH] Fix missing namespace metadata after restore.

If backing up specific namespaces with "auto" cluster resources, the
actual namespace objects themselves were not being included in the
backup. Restore would create them but any labels or metadata would be
lost.

Instead handle the special case of namespace as a cluster level resource
we may still need, even if excluding most cluster level resources.

Signed-off-by: Devan Goodwin <dgoodwin@redhat.com>
(cherry picked from commit 9471f9da3c489f28aa388f7a135bce7a99e15876)
---
 pkg/backup/backup_test.go             | 10 +++++++++-
 pkg/backup/resource_backupper.go      |  6 +++++-
 pkg/backup/resource_backupper_test.go | 24 +++++++++++++++++++++++-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go
index 06fdfbe85..0eef90cc9 100644
--- a/pkg/backup/backup_test.go
+++ b/pkg/backup/backup_test.go
@@ -230,7 +230,7 @@ func TestGetNamespaceIncludesExcludes(t *testing.T) {
 var (
 	v1Group = &metav1.APIResourceList{
 		GroupVersion: "v1",
-		APIResources: []metav1.APIResource{configMapsResource, podsResource},
+		APIResources: []metav1.APIResource{configMapsResource, podsResource, namespacesResource},
 	}
 
 	configMapsResource = metav1.APIResource{
@@ -266,6 +266,14 @@ var (
 		Verbs:        metav1.Verbs([]string{"create", "update", "get", "list", "watch", "delete"}),
 	}
 
+	namespacesResource = metav1.APIResource{
+		Name:         "namespaces",
+		SingularName: "namespace",
+		Namespaced:   false,
+		Kind:         "Namespace",
+		Verbs:        metav1.Verbs([]string{"create", "update", "get", "list", "watch", "delete"}),
+	}
+
 	certificatesGroup = &metav1.APIResourceList{
 		GroupVersion: "certificates.k8s.io/v1beta1",
 		APIResources: []metav1.APIResource{certificateSigningRequestsResource},
diff --git a/pkg/backup/resource_backupper.go b/pkg/backup/resource_backupper.go
index 28d8b293d..b3e06f641 100644
--- a/pkg/backup/resource_backupper.go
+++ b/pkg/backup/resource_backupper.go
@@ -129,7 +129,9 @@ func (rb *defaultResourceBackupper) backupResource(
 		// (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 !resource.Namespaced && !rb.namespaces.IncludeEverything() {
+		// If we're processing namespaces themselves, we will not skip here, they may be
+		// filtered out later.
+		if !resource.Namespaced && resource.Kind != "Namespace" && !rb.namespaces.IncludeEverything() {
 			log.Info("Skipping resource because it's cluster-scoped and only specific namespaces are included in the backup")
 			return nil
 		}
@@ -173,6 +175,8 @@ func (rb *defaultResourceBackupper) backupResource(
 		rb.discoveryHelper,
 	)
 
+	// TODO: when processing namespaces, and only including certain namespaces, we still list
+	// them all here. Could optimize to get specifics, but watch out for label selector.
 	var namespacesToList []string
 	if resource.Namespaced {
 		namespacesToList = getNamespacesToList(rb.namespaces)
diff --git a/pkg/backup/resource_backupper_test.go b/pkg/backup/resource_backupper_test.go
index 78b7828df..980d83a0d 100644
--- a/pkg/backup/resource_backupper_test.go
+++ b/pkg/backup/resource_backupper_test.go
@@ -195,6 +195,24 @@ func TestBackupResource(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:                     "should include specified namespaces if backing up subset of namespaces and --include-cluster-resources=nil",
+			namespaces:               collections.NewIncludesExcludes().Includes("ns-1"),
+			resources:                collections.NewIncludesExcludes(),
+			includeClusterResources:  nil,
+			expectedListedNamespaces: []string{"ns-1"},
+			apiGroup:                 v1Group,
+			apiResource:              namespacesResource,
+			groupVersion:             schema.GroupVersion{Group: "", Version: "v1"},
+			groupResource:            schema.GroupResource{Group: "", Resource: "namespaces"},
+			expectSkip:               false,
+			listResponses: [][]*unstructured.Unstructured{
+				{
+					unstructuredOrDie(`{"apiVersion":"v1","kind":"Namespace","metadata":{"name":"ns-1"}}`),
+					unstructuredOrDie(`{"apiVersion":"v1","kind":"Namespace","metadata":{"name":"ns-2"}}`),
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -275,7 +293,11 @@ func TestBackupResource(t *testing.T) {
 					client := &arktest.FakeDynamicClient{}
 					defer client.AssertExpectations(t)
 
-					dynamicFactory.On("ClientForGroupVersionResource", test.groupVersion, test.apiResource, namespace).Return(client, nil)
+					if test.groupResource.Resource == "namespaces" {
+						dynamicFactory.On("ClientForGroupVersionResource", test.groupVersion, test.apiResource, "").Return(client, nil)
+					} else {
+						dynamicFactory.On("ClientForGroupVersionResource", test.groupVersion, test.apiResource, namespace).Return(client, nil)
+					}
 
 					list := &unstructured.UnstructuredList{
 						Items: []unstructured.Unstructured{},