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 <lucarval@redhat.com>
google · Nov 1, 2023 · 6701ddb · 6701ddb
1 parent dbcd01c
commit 6701ddb
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 4 deletions.
diff --git a/pkg/authn/kubernetes/keychain.go b/pkg/authn/kubernetes/keychain.go
@@ -17,7 +17,6 @@ package kubernetes
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net"
 	"net/url"
 	"path/filepath"
@@ -170,12 +169,14 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key
 	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,7 +187,8 @@ 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
 			}
 
 			// The docker client allows exact matches:

diff --git a/pkg/authn/kubernetes/keychain_test.go b/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{