Merge pull request #2728 from 2opremio/scope-imagepullsecrets-correctly

Correctly scope imagePullSecrets by their namespace

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")