Correctly scope imagePullSecrets by their namespace

PR fluxcd#2520 broke the scoping of imagePullSecrets when
allowing to get workloads from all namespaces at once.

The problem is that a secret cache (`seenCreds`) was indexed
by the secret name (making it namespace specific).

Processing all the namespaces at once without adjusting the
scoping opened the door to clashes between secrets from
different namespaces which are called the same.

pkg/cluster/kubernetes/images.go

@@ -21,7 +21,7 @@ func mergeCredentials(log func(...interface{}) error,
 	client ExtendedClient,
 	namespace string, podTemplate apiv1.PodTemplateSpec,
 	imageCreds registry.ImageCreds,
-	seenCreds map[string]registry.Credentials) {
+	imagePullSecretCache map[string]registry.Credentials) {
 	var images []image.Name
 	for _, container := range podTemplate.Spec.InitContainers {
 		r, err := image.ParseRef(container.Image)
@@ -68,15 +68,16 @@ func mergeCredentials(log func(...interface{}) error,
 	}
 
 	for _, name := range imagePullSecrets {
-		if seen, ok := seenCreds[name]; ok {
+		namespacedSecretName := fmt.Sprintf("%s/%s", namespace, name)
+		if seen, ok := imagePullSecretCache[namespacedSecretName]; ok {
 			creds.Merge(seen)
 			continue
 		}
 
 		secret, err := client.CoreV1().Secrets(namespace).Get(name, meta_v1.GetOptions{})
 		if err != nil {
 			log("err", errors.Wrapf(err, "getting secret %q from namespace %q", name, namespace))
-			seenCreds[name] = registry.NoCredentials()
+			imagePullSecretCache[namespacedSecretName] = registry.NoCredentials()
 			continue
 		}
 
@@ -91,24 +92,24 @@ func mergeCredentials(log func(...interface{}) error,
 			decoded, ok = secret.Data[apiv1.DockerConfigJsonKey]
 		default:
 			log("skip", "unknown type", "secret", namespace+"/"+secret.Name, "type", secret.Type)
-			seenCreds[name] = registry.NoCredentials()
+			imagePullSecretCache[namespacedSecretName] = registry.NoCredentials()
 			continue
 		}
 
 		if !ok {
 			log("err", errors.Wrapf(err, "retrieving pod secret %q", secret.Name))
-			seenCreds[name] = registry.NoCredentials()
+			imagePullSecretCache[namespacedSecretName] = registry.NoCredentials()
 			continue
 		}
 
 		// Parse secret
 		crd, err := registry.ParseCredentials(fmt.Sprintf("%s:secret/%s", namespace, name), decoded)
 		if err != nil {
 			log("err", err.Error())
-			seenCreds[name] = registry.NoCredentials()
+			imagePullSecretCache[namespacedSecretName] = registry.NoCredentials()
 			continue
 		}
-		seenCreds[name] = crd
+		imagePullSecretCache[namespacedSecretName] = crd
 
 		// Merge into the credentials for this PodSpec
 		creds.Merge(crd)
@@ -132,7 +133,7 @@ func (c *Cluster) ImagesToFetch() registry.ImageCreds {
 	}
 
 	for _, ns := range namespaces {
-		seenCreds := make(map[string]registry.Credentials)
+		imagePullSecretCache := make(map[string]registry.Credentials) // indexed by the namespace/name of pullImageSecrets
 		for kind, resourceKind := range resourceKinds {
 			workloads, err := resourceKind.getWorkloads(ctx, c, ns)
 			if err != nil {
@@ -146,7 +147,7 @@ func (c *Cluster) ImagesToFetch() registry.ImageCreds {
 			imageCreds := make(registry.ImageCreds)
 			for _, workload := range workloads {
 				logger := log.With(c.logger, "resource", resource.MakeID(workload.GetNamespace(), kind, workload.GetName()))
-				mergeCredentials(logger.Log, c.includeImage, c.client, workload.GetNamespace(), workload.podTemplate, imageCreds, seenCreds)
+				mergeCredentials(logger.Log, c.includeImage, c.client, workload.GetNamespace(), workload.podTemplate, imageCreds, imagePullSecretCache)
 			}
 
 			// Merge creds

pkg/cluster/kubernetes/images_test.go

@@ -2,6 +2,7 @@ package kubernetes
 
 import (
 	"encoding/base64"
+	"fmt"
 	"testing"
 
 	"github.com/ryanuber/go-glob"
@@ -13,7 +14,8 @@ import (
 	"github.com/fluxcd/flux/pkg/registry"
 )
 
-func noopLog(...interface{}) error {
+func noopLog(e ...interface{}) error {
+	fmt.Println(e...)
 	return nil
 }
 
@@ -78,6 +80,44 @@ func TestMergeCredentials(t *testing.T) {
 	assert.ElementsMatch(t, []string{"docker.io", "quay.io"}, hosts)
 }
 
+func TestMergeCredentials_SameSecretSameNameDifferentNamespace(t *testing.T) {
+	ns1, ns2, secretName := "foo-ns", "bar-ns", "pull-secretname"
+	saName := "service-account"
+	ref, _ := image.ParseRef("foo/bar:tag")
+	spec := apiv1.PodTemplateSpec{
+		Spec: apiv1.PodSpec{
+			ServiceAccountName: saName,
+			ImagePullSecrets: []apiv1.LocalObjectReference{
+				{Name: secretName},
+			},
+			Containers: []apiv1.Container{
+				{Name: "container1", Image: ref.String()},
+			},
+		},
+	}
+
+	clientset := fake.NewSimpleClientset(
+		makeServiceAccount(ns1, saName, []string{secretName}),
+		makeServiceAccount(ns2, saName, []string{secretName}),
+		makeImagePullSecret(ns1, secretName, "docker.io"),
+		makeImagePullSecret(ns2, secretName, "quay.io"))
+	client := ExtendedClient{coreClient: clientset}
+
+	creds := registry.ImageCreds{}
+
+	pullImageSecretCache := make(map[string]registry.Credentials)
+	mergeCredentials(noopLog, func(imageName string) bool { return true },
+		client, ns1, spec, creds, pullImageSecretCache)
+	mergeCredentials(noopLog, func(imageName string) bool { return true },
+		client, ns2, spec, creds, pullImageSecretCache)
+	// check that we accumulated some credentials
+	assert.Contains(t, creds, ref.Name)
+	c := creds[ref.Name]
+	hosts := c.Hosts()
+	// Make sure we get the host from the second secret
+	assert.ElementsMatch(t, []string{"quay.io"}, hosts)
+}
+
 func TestMergeCredentials_ImageExclusion(t *testing.T) {
 	creds := registry.ImageCreds{}
 	gcrImage, _ := image.ParseRef("gcr.io/foo/bar:tag")