Ignore malformed secrets when creating keychain

This changes the behavior of the NewFromPullSecrets function to ignore
secrets that contain malformed data. This allows processing of the other
secrets which may be sufficient for authentication.

Resolves #1833

Signed-off-by: Luiz Carvalho <[email protected]>

pkg/authn/kubernetes/keychain.go

@@ -17,7 +17,7 @@ package kubernetes
 import (
 	"context"
 	"encoding/json"
-	"fmt"
+	"errors"
 	"net"
 	"net/url"
 	"path/filepath"
@@ -132,7 +132,6 @@ func New(ctx context.Context, client kubernetes.Interface, opt Options) (authn.K
 			}
 		}
 	}
-
 	return NewFromPullSecrets(ctx, pullSecrets)
 }
 
@@ -165,17 +164,22 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key
 	}
 
 	var cfg dockerConfigJSON
+	// hasValid tracks if at least one of the given secrets provided is valid and contains at least
+	// one valid auth.
+	var hasValid bool
 
 	// From: https://github.com/kubernetes/kubernetes/blob/0dcafb1f37ee522be3c045753623138e5b907001/pkg/credentialprovider/keyring.go
 	for _, secret := range secrets {
 		if b, exists := secret.Data[corev1.DockerConfigJsonKey]; secret.Type == corev1.SecretTypeDockerConfigJson && exists && len(b) > 0 {
 			if err := json.Unmarshal(b, &cfg); err != nil {
-				return nil, err
+				logs.Warn.Printf("cannot decode secret %s/%s; ignoring", secret.Namespace, secret.Name)
+				continue
 			}
 		}
 		if b, exists := secret.Data[corev1.DockerConfigKey]; secret.Type == corev1.SecretTypeDockercfg && exists && len(b) > 0 {
 			if err := json.Unmarshal(b, &cfg.Auths); err != nil {
-				return nil, err
+				logs.Warn.Printf("cannot decode secret %s/%s; ignoring", secret.Namespace, secret.Name)
+				continue
 			}
 		}
 
@@ -186,9 +190,11 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key
 			}
 			parsed, err := url.Parse(value)
 			if err != nil {
-				return nil, fmt.Errorf("Entry %q in dockercfg invalid (%w)", value, err)
+				logs.Warn.Printf("entry %q in dockercfg secret %s/%s invalid (%s); ignoring", secret.Namespace, secret.Name, value, err)
+				continue
 			}
 
+			hasValid = true
 			// The docker client allows exact matches:
 			//    foo.bar.com/namespace
 			// Or hostname matches:
@@ -218,6 +224,10 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key
 		// when matching for creds
 		sort.Sort(sort.Reverse(sort.StringSlice(keyring.index)))
 	}
+	if !hasValid && len(secrets) > 0 {
+		return nil, errors.New("could not find a valid secret")
+	}
+
 	return keyring, nil
 }
 

pkg/authn/kubernetes/keychain_test.go

@@ -132,6 +132,49 @@ func TestServiceAccountNotFound(t *testing.T) {
 	testResolve(t, kc, registry(t, "fake.registry.io"), authn.Anonymous)
 }
 
+func TestMalformedSecretsIgnored(t *testing.T) {
+	secrets := []corev1.Secret{
+		// Malformed .dockerconfigjson is ignored
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bad-dockerconfigjson",
+				Namespace: "ns",
+			},
+			Type: corev1.SecretTypeDockerConfigJson,
+			Data: map[string][]byte{
+				corev1.DockerConfigJsonKey: []byte("bad-base64"),
+			},
+		},
+		// Malformed .dockercfg is ignored
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bad-dockercfg",
+				Namespace: "ns",
+			},
+			Type: corev1.SecretTypeDockercfg,
+			Data: map[string][]byte{
+				corev1.DockerConfigKey: []byte("bad-base64"),
+			},
+		},
+		// Malformed registry URL is ignored
+		*dockerCfgSecretType.Create(t, "ns", "secret", `\tfake.registry.io`, authn.AuthConfig{
+			Username: "ignored", Password: "ignored",
+		}),
+		// Correct secret is used
+		*dockerCfgSecretType.Create(t, "ns", "secret", "fake.registry.io", authn.AuthConfig{
+			Username: "user", Password: "pass",
+		}),
+	}
+
+	kc, err := NewFromPullSecrets(context.Background(), secrets)
+	if err != nil {
+		t.Fatalf("NewFromPullSecrets() = %v", err)
+	}
+
+	expectedAuth := &authn.Basic{Username: "user", Password: "pass"}
+	testResolve(t, kc, registry(t, "fake.registry.io"), expectedAuth)
+}
+
 func TestImagePullSecretAttachedServiceAccount(t *testing.T) {
 	username, password := "foo", "bar"
 	client := fakeclient.NewSimpleClientset(&corev1.ServiceAccount{
@@ -413,6 +456,31 @@ func TestAuthWithGlobs(t *testing.T) {
 	testResolve(t, kc, repo(t, "blah.registry.io/repo"), authn.FromConfig(auth))
 }
 
+func TestNoValidAuthsFound(t *testing.T) {
+	secrets := []corev1.Secret{
+		// Malformed .dockerconfigjson is ignored
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bad-dockerconfigjson",
+				Namespace: "ns",
+			},
+			Type: corev1.SecretTypeDockerConfigJson,
+			Data: map[string][]byte{
+				corev1.DockerConfigJsonKey: []byte("bad-base64"),
+			},
+		},
+	}
+
+	kc, err := NewFromPullSecrets(context.Background(), secrets)
+	if kc != nil {
+		t.Fatal("NewFromPullSecrets() = expected nil keychain")
+	}
+	expectedErr := "could not find a valid secret"
+	if err == nil || fmt.Sprintf("%s", err) != expectedErr {
+		t.Fatalf("NewFromPullSecrets() = want error %q, got %q", expectedErr, err)
+	}
+}
+
 func testResolve(t *testing.T, kc authn.Keychain, target authn.Resource, expectedAuth authn.Authenticator) {
 	t.Helper()