Rebased

Azure · Feb 7, 2024 · ca345ac · ca345ac
1 parent b9a60bd
commit ca345ac
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 22 deletions.
diff --git a/pkg/cluster/acrtoken.go b/pkg/cluster/acrtoken.go
@@ -187,5 +187,5 @@ func (m *manager) validateACRToken(ctx context.Context) error {
 		return err
 	}
 	rc := pullsecret.NewRegistryClient()
-	return rc.ValidatePullSecret(ctx, pullSecret)
+	return rc.ValidatePullSecret(ctx, pullSecret, []string{m.env.ACRDomain()})
 }
diff --git a/pkg/operator/controllers/pullsecret/pullsecret_controller.go b/pkg/operator/controllers/pullsecret/pullsecret_controller.go
@@ -171,6 +171,12 @@ func (r *Reconciler) ensureGlobalPullSecret(ctx context.Context, operatorSecret,
 		}
 	}
 
+	err = r.registryClient.ValidatePullSecret(ctx, operatorSecret, []string{})
+	if err != nil {
+		r.Log.Error(err)
+		r.SetDegraded(ctx, err)
+	}
+
 	fixedData, update, err := pullsecret.Merge(string(secret.Data[corev1.DockerConfigJsonKey]), string(operatorSecret.Data[corev1.DockerConfigJsonKey]))
 	if err != nil {
 		return nil, err
@@ -197,7 +203,7 @@ func (r *Reconciler) ensureGlobalPullSecret(ctx context.Context, operatorSecret,
 			return secret, err
 		}
 
-		err = r.registryClient.ValidatePullSecret(ctx, secret)
+		err = r.registryClient.ValidatePullSecret(ctx, secret, []string{})
 		if err != nil {
 			r.Log.Error(err)
 			r.SetDegraded(ctx, err)
@@ -210,7 +216,7 @@ func (r *Reconciler) ensureGlobalPullSecret(ctx context.Context, operatorSecret,
 		return secret, err
 	}
 
-	err = r.registryClient.ValidatePullSecret(ctx, secret)
+	err = r.registryClient.ValidatePullSecret(ctx, secret, []string{})
 	if err != nil {
 		r.Log.Error(err)
 		r.SetDegraded(ctx, err)

diff --git a/pkg/util/pullsecret/pullsecret.go b/pkg/util/pullsecret/pullsecret.go
@@ -181,29 +181,42 @@ func NewRegistryClient() RegistryClient {
 }
 
 // ValidatePullSecret validates a passed in pull secret by attempting to log in to the registry
-func (r *RegistryClient) ValidatePullSecret(ctx context.Context, secret *corev1.Secret) error {
+// Will check only pull secrets for the specified domains, an empty domains slice will check all pull secrets
+func (r *RegistryClient) ValidatePullSecret(ctx context.Context, secret *corev1.Secret, domains []string) error {
 	dockerConfig, err := UnmarshalSecretData(secret)
 	if err != nil {
 		return err
 	}
+	errs := make([]string, 0)
 	for registry, authBase64 := range dockerConfig {
-		authDecoded, err := base64.StdEncoding.DecodeString(authBase64)
-		if err != nil {
-			return err
+		checkRegistry := false
+		for _, domain := range domains {
+			if registry == domain {
+				checkRegistry = true
+			}
 		}
+		if len(domains) == 0 || checkRegistry {
+			authDecoded, err := base64.StdEncoding.DecodeString(authBase64)
+			if err != nil {
+				errs = append(errs, err.Error())
+				continue
+			}
 
-		auth := strings.SplitN(string(authDecoded), ":", 2)
-		if len(auth) != 2 {
-			err = fmt.Errorf("credentials format error: %s", registry)
-			return err
-		}
-		if err != nil {
-			return err
-		}
-		err = r.CheckAuth(ctx, nil, auth[0], auth[1], registry)
-		if err != nil {
-			return fmt.Errorf("failed to authenticate to registry %s: %w", registry, err)
+			auth := strings.SplitN(string(authDecoded), ":", 2)
+			if len(auth) != 2 {
+				errs = append(errs, fmt.Errorf("credentials format error: %s", registry).Error())
+				continue
+			}
+			err = r.CheckAuth(ctx, nil, auth[0], auth[1], registry)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("failed to authenticate to registry %s: %w", registry, err).Error())
+			}
 		}
 	}
+	// go 1.20:
+	// return errors.Join(errs)
+	if len(errs) > 0 {
+		return fmt.Errorf(strings.Join(errs, ";"))
+	}
 	return nil
 }
diff --git a/pkg/util/pullsecret/pullsecret_test.go b/pkg/util/pullsecret/pullsecret_test.go
@@ -378,17 +378,25 @@ func TestUnmarshalSecretData(t *testing.T) {
 }
 
 func TestValidatePullSecret(t *testing.T) {
-	azurecrError := "unable to retrieve auth token: invalid username/password: unauthorized: authentication required, visit https://aka.ms/acr/authorization for more information. This error has been customized to ensure it doesn't leak to Azure."
+	azurecrError := "unable to retrieve auth token: invalid username/password: unauthorized: authentication required, visit https://aka.ms/acr/authorization for more information. This error has been customized so if it leaks to Azure the test will fail."
 	erroringRegistry := RegistryClient{
-		CheckAuth: func(ctx context.Context, sc *types.SystemContext, s1, s2, s3 string) error {
+		CheckAuth: func(ctx context.Context, sc *types.SystemContext, u, p, registry string) error {
 			return fmt.Errorf(azurecrError)
 		},
 	}
 	succeedingRegistry := RegistryClient{
-		CheckAuth: func(ctx context.Context, sc *types.SystemContext, s1, s2, s3 string) error {
+		CheckAuth: func(ctx context.Context, sc *types.SystemContext, u, p, registry string) error {
 			return nil
 		},
 	}
+	onlyAroSucceedsRegistry := RegistryClient{
+		CheckAuth: func(ctx context.Context, sc *types.SystemContext, u, p, registry string) error {
+			if registry == "arosvc.azurecr.io" {
+				return nil
+			}
+			return fmt.Errorf(azurecrError)
+		},
+	}
 	test := []struct {
 		name     string
 		ps       *corev1.Secret
@@ -409,6 +417,20 @@ func TestValidatePullSecret(t *testing.T) {
 			},
 			client: succeedingRegistry,
 		},
+		{
+			name: "broken user registry",
+			ps: &corev1.Secret{
+				Data: map[string][]byte{
+					corev1.DockerConfigJsonKey: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}, "registry.redhat.io":{"auth":"ZnJlZDplbnRlcg=="}, "registry.example.com":{"auth":"ZnJlZDplbnRlcg=="}}}`),
+				},
+			},
+			wantAuth: map[string]string{
+				"arosvc.azurecr.io":    "ZnJlZDplbnRlcg==",
+				"registry.redhat.io":   "ZnJlZDplbnRlcg==",
+				"registry.example.com": "ZnJlZDplbnRlcg==",
+			},
+			client: onlyAroSucceedsRegistry,
+		},
 		{
 			name: "authentication failure",
 			ps: &corev1.Secret{
@@ -443,7 +465,7 @@ func TestValidatePullSecret(t *testing.T) {
 
 	for _, tt := range test {
 		t.Run(tt.name, func(t *testing.T) {
-			err := tt.client.ValidatePullSecret(context.TODO(), tt.ps)
+			err := tt.client.ValidatePullSecret(context.TODO(), tt.ps, []string{"arosvc.azurecr.io"})
 			if err != nil {
 				if err.Error() != tt.wantErr {
 					t.Fatalf("%v\ndoes not match:\n%s\n", err.Error(), tt.wantErr)