Validate ACR token

pkg/cluster/acrtoken.go

@@ -176,3 +176,12 @@ func retryOperation(retryable func() error) error {
 		return kerrors.IsBadRequest(err) || kerrors.IsInternalError(err) || kerrors.IsServerTimeout(err) || kerrors.IsConflict(err)
 	}, retryable)
 }
+
+func (m *manager) validateACRToken(ctx context.Context) error {
+	pullSecret, err := m.kubernetescli.CoreV1().Secrets(operator.Namespace).Get(ctx, operator.SecretName, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+	rc := pullsecret.NewRegistryClient()
+	return rc.ValidatePullSecret(ctx, pullSecret)
+}

pkg/cluster/adminupdate_test.go

@@ -130,7 +130,9 @@ func TestAdminUpdateSteps(t *testing.T) {
 				"[Action fixMCSCert-fm]",
 				"[Action fixMCSUserData-fm]",
 				"[Action ensureGatewayUpgrade-fm]",
+				"[Condition aroDeploymentReady-fm, timeout 5m0s]",
 				"[Action rotateACRTokenPassword-fm]",
+				"[Condition ensureValidACRToken-fm, timeout 5m0s]",
 				"[Action configureAPIServerCertificate-fm]",
 				"[Action configureIngressCertificate-fm]",
 				"[Action populateRegistryStorageAccountName-fm]",
@@ -218,7 +220,9 @@ func TestAdminUpdateSteps(t *testing.T) {
 				"[Action fixMCSCert-fm]",
 				"[Action fixMCSUserData-fm]",
 				"[Action ensureGatewayUpgrade-fm]",
+				"[Condition aroDeploymentReady-fm, timeout 5m0s]",
 				"[Action rotateACRTokenPassword-fm]",
+				"[Condition ensureValidACRToken-fm, timeout 5m0s]",
 				"[Action configureAPIServerCertificate-fm]",
 				"[Action configureIngressCertificate-fm]",
 				"[Action populateRegistryStorageAccountName-fm]",
@@ -259,7 +263,9 @@ func TestAdminUpdateSteps(t *testing.T) {
 				"[Action fixMCSCert-fm]",
 				"[Action fixMCSUserData-fm]",
 				"[Action ensureGatewayUpgrade-fm]",
+				"[Condition aroDeploymentReady-fm, timeout 5m0s]",
 				"[Action rotateACRTokenPassword-fm]",
+				"[Condition ensureValidACRToken-fm, timeout 5m0s]",
 				"[Action configureAPIServerCertificate-fm]",
 				"[Action configureIngressCertificate-fm]",
 				"[Action populateRegistryStorageAccountName-fm]",
@@ -331,7 +337,9 @@ func TestAdminUpdateSteps(t *testing.T) {
 				"[Action fixMCSCert-fm]",
 				"[Action fixMCSUserData-fm]",
 				"[Action ensureGatewayUpgrade-fm]",
+				"[Condition aroDeploymentReady-fm, timeout 5m0s]",
 				"[Action rotateACRTokenPassword-fm]",
+				"[Condition ensureValidACRToken-fm, timeout 5m0s]",
 				"[Action configureAPIServerCertificate-fm]",
 				"[Action configureIngressCertificate-fm]",
 				"[Action populateRegistryStorageAccountName-fm]",

pkg/cluster/install.go

@@ -102,7 +102,9 @@ func (m *manager) adminUpdate() []steps.Step {
 	if isEverything {
 		toRun = append(toRun,
 			steps.Action(m.ensureGatewayUpgrade),
+			steps.Condition(m.aroDeploymentReady, 5*time.Minute, true),
 			steps.Action(m.rotateACRTokenPassword),
+			steps.Action(m.validateACRToken),
 		)
 	}
 
@@ -195,7 +197,9 @@ func (m *manager) Update(ctx context.Context) error {
 		steps.Action(m.createOrUpdateDenyAssignment),
 		steps.Action(m.startVMs),
 		steps.Condition(m.apiServersReady, 30*time.Minute, true),
+		steps.Condition(m.aroDeploymentReady, 5*time.Minute, true),
 		steps.Action(m.rotateACRTokenPassword),
+		steps.Action(m.validateACRToken),
 		steps.Action(m.configureAPIServerCertificate),
 		steps.Action(m.configureIngressCertificate),
 		steps.Action(m.renewMDSDCertificate),

pkg/operator/controllers/pullsecret/pullsecret_controller.go

@@ -192,10 +192,22 @@ func (r *Reconciler) ensureGlobalPullSecret(ctx context.Context, operatorSecret,
 		}
 
 		err = r.client.Create(ctx, secret)
+		if err != nil {
+			return secret, err
+		}
+
+		rc := pullsecret.NewRegistryClient()
+		err = rc.ValidatePullSecret(ctx, secret)
 		return secret, err
 	}
 
 	err = r.client.Update(ctx, secret)
+	if err != nil {
+		return secret, err
+	}
+
+	rc := pullsecret.NewRegistryClient()
+	err = rc.ValidatePullSecret(ctx, secret)
 	return secret, err
 }
 

pkg/util/pullsecret/pullsecret.go

@@ -4,11 +4,16 @@ package pullsecret
 // Licensed under the Apache License 2.0.
 
 import (
+	"context"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"os"
 	"reflect"
+	"strings"
 
+	dockerregistry "github.com/containers/image/v5/docker"
+	"github.com/containers/image/v5/types"
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/Azure/ARO-RP/pkg/api"
@@ -164,3 +169,41 @@ func Build(oc *api.OpenShiftCluster, ps string) (string, error) {
 
 	return pullSecret, nil
 }
+
+type registryClient struct {
+	checkAuth func(context.Context, *types.SystemContext, string, string, string) error
+}
+
+func NewRegistryClient() registryClient {
+	return registryClient{
+		checkAuth: dockerregistry.CheckAuth,
+	}
+}
+
+// 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 {
+	dockerConfig, err := UnmarshalSecretData(secret)
+	if err != nil {
+		return err
+	}
+	for registry, authBase64 := range dockerConfig {
+		authDecoded, err := base64.StdEncoding.DecodeString(authBase64)
+		if err != nil {
+			return err
+		}
+
+		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)
+		}
+	}
+	return nil
+}

pkg/util/pullsecret/pullsecret_test.go

@@ -4,10 +4,13 @@ package pullsecret
 // Licensed under the Apache License 2.0.
 
 import (
+	"context"
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"testing"
 
+	"github.com/containers/image/v5/types"
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/Azure/ARO-RP/pkg/api"
@@ -373,3 +376,79 @@ 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."
+	erroringRegistry := registryClient{
+		checkAuth: func(ctx context.Context, sc *types.SystemContext, s1, s2, s3 string) error {
+			return fmt.Errorf(azurecrError)
+		},
+	}
+	succeedingRegistry := registryClient{
+		checkAuth: func(ctx context.Context, sc *types.SystemContext, s1, s2, s3 string) error {
+			return nil
+		},
+	}
+	test := []struct {
+		name     string
+		ps       *corev1.Secret
+		wantAuth map[string]string
+		wantErr  string
+		client   registryClient
+	}{
+		{
+			name: "ok secret",
+			ps: &corev1.Secret{
+				Data: map[string][]byte{
+					corev1.DockerConfigJsonKey: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}, "registry.redhat.io":{"auth":"ZnJlZDplbnRlcg=="}}}`),
+				},
+			},
+			wantAuth: map[string]string{
+				"arosvc.azurecr.io":  "ZnJlZDplbnRlcg==",
+				"registry.redhat.io": "ZnJlZDplbnRlcg==",
+			},
+			client: succeedingRegistry,
+		},
+		{
+			name: "authentication failure",
+			ps: &corev1.Secret{
+				Data: map[string][]byte{
+					corev1.DockerConfigJsonKey: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}, "registry.redhat.io":{"auth":"ZnJlZDplbnRlcg=="}}}`),
+				},
+			},
+			wantErr: fmt.Sprintf("failed to authenticate to registry arosvc.azurecr.io: %s", azurecrError),
+			client:  erroringRegistry,
+		},
+		{
+			name: "broken secret, bad bas64",
+			ps: &corev1.Secret{
+				Data: map[string][]byte{
+					corev1.DockerConfigJsonKey: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":"INVALID"}}}`),
+				},
+			},
+			wantErr: "illegal base64 data at input byte 4",
+			client:  erroringRegistry,
+		},
+		{
+			name: "broken secret, missing colon",
+			ps: &corev1.Secret{
+				Data: map[string][]byte{
+					corev1.DockerConfigJsonKey: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZGVudGVy"}}}`),
+				},
+			},
+			wantErr: "credentials format error: arosvc.azurecr.io",
+			client:  erroringRegistry,
+		},
+	}
+
+	for _, tt := range test {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.client.ValidatePullSecret(context.TODO(), tt.ps)
+			if err != nil {
+				if err.Error() != tt.wantErr {
+					t.Fatal(err.Error())
+				}
+			}
+		})
+	}
+}