From ca345ac70225f708fb2c59a38f59bc3065990558 Mon Sep 17 00:00:00 2001 From: Andrew Denton Date: Wed, 7 Feb 2024 15:39:23 -0800 Subject: [PATCH] Rebased --- pkg/cluster/acrtoken.go | 2 +- .../pullsecret/pullsecret_controller.go | 10 ++++- pkg/util/pullsecret/pullsecret.go | 43 ++++++++++++------- pkg/util/pullsecret/pullsecret_test.go | 30 +++++++++++-- 4 files changed, 63 insertions(+), 22 deletions(-) diff --git a/pkg/cluster/acrtoken.go b/pkg/cluster/acrtoken.go index 2b48e30d701..4cd06c5f3d2 100644 --- 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 index 18ca9b505df..516b5bcff6a 100644 --- 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 index 4eeee8bc2ba..0db5e10af1b 100644 --- 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 index da58a056564..6cd8a0fc60d 100644 --- 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)