From 2b8e038bf20ac3ecb38b7a69efd4b98a14166feb Mon Sep 17 00:00:00 2001
From: Praveen Rewar <8457124+praveenrewar@users.noreply.github.com>
Date: Thu, 21 Jul 2022 10:32:01 +0530
Subject: [PATCH] Use fallbackAllowedNamespaces to watch pods

---
 .../resources/identified_resources_pods.go    | 29 ++++++++++++-------
 test/e2e/fallback_allowed_ns_test.go          |  6 +++-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/pkg/kapp/resources/identified_resources_pods.go b/pkg/kapp/resources/identified_resources_pods.go
index 38102372a..e3e8352bf 100644
--- a/pkg/kapp/resources/identified_resources_pods.go
+++ b/pkg/kapp/resources/identified_resources_pods.go
@@ -5,6 +5,7 @@ package resources
 
 import (
 	"fmt"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -35,28 +36,34 @@ func (w UniquePodWatcher) Watch(podsToWatchCh chan corev1.Pod, cancelCh chan str
 	go func() {
 		// Watch Pods in all namespaces first and fallback to the
 		// fallbackAllowedNamespaces if lack of permission
-		namespace := ""
-		index := 0
-		for {
+		namespaces := []string{""}
+		namespaces = append(namespaces, w.fallbackAllowedNamespaces...)
+		var forbiddenNamespaces []string
+
+		for _, namespace := range namespaces {
 			podWatcher := NewPodWatcher(
 				w.coreClient.CoreV1().Pods(namespace),
 				metav1.ListOptions{LabelSelector: w.labelSelector.String()},
 			)
-
 			err := podWatcher.Watch(nonUniquePodsToWatchCh, cancelCh)
-			if err != nil {
-				if !errors.IsForbidden(err) {
-					fmt.Printf("Pod watching error: %s\n", err) // TODO
+			if err == nil {
+				if namespace == "" {
 					break
 				}
+				continue
 			}
-
-			if len(w.fallbackAllowedNamespaces) > index {
-				namespace = w.fallbackAllowedNamespaces[index]
-				index++
+			if !errors.IsForbidden(err) {
+				fmt.Printf("Pod watching error: %s\n", err) // TODO
+				break
+			}
+			if namespace != "" {
+				forbiddenNamespaces = append(forbiddenNamespaces, fmt.Sprintf(`"%s"`, namespace))
 			}
 		}
 
+		if len(forbiddenNamespaces) > 0 {
+			fmt.Printf(`Pod watching error: pods is forbidden: User cannot list resource "pods" in API group "" in the namespace(s) %s`, strings.Join(forbiddenNamespaces, ", "))
+		}
 		close(nonUniquePodsToWatchCh)
 	}()
 
diff --git a/test/e2e/fallback_allowed_ns_test.go b/test/e2e/fallback_allowed_ns_test.go
index c82582ec7..504b064d0 100644
--- a/test/e2e/fallback_allowed_ns_test.go
+++ b/test/e2e/fallback_allowed_ns_test.go
@@ -158,9 +158,13 @@ data:
 `, env.Namespace, testNamespace)
 
 	logger.Section("deploy app using scoped context", func() {
-		kapp.RunWithOpts([]string{"deploy", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+		out, _ := kapp.RunWithOpts([]string{"deploy", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
 			RunOpts{StdinReader: strings.NewReader(yaml1)})
 
+		// Expect pod watching error for the fallback allowed namespaces as listing pods is not allowed.
+		require.Contains(t, out, fmt.Sprintf(`Pod watching error: pods is forbidden: User cannot list resource "pods" in API group "" in the namespace(s) "%s", "%s"`,
+			env.Namespace, testNamespace))
+
 		NewPresentClusterResource("configmap", "cm-1", env.Namespace, kubectl)
 		NewPresentClusterResource("configmap", "cm-2", testNamespace, kubectl)
 		NewPresentClusterResource("configmap", "cm-3", testNamespace, kubectl)