diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 64d5c78ff..401ade43b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -9,7 +9,7 @@ package v1alpha1 import ( "github.com/d2iq-labs/capi-runtime-extensions/api/external/sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files.go b/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files.go index 496488817..5a839352b 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files.go @@ -6,7 +6,6 @@ package credentials import ( "bytes" _ "embed" - "errors" "fmt" "net/url" "path" @@ -47,8 +46,6 @@ var ( ) ) -var ErrCredentialsNotFound = errors.New("registry credentials not found") - type providerConfig struct { URL string Username string @@ -61,6 +58,43 @@ func (c providerConfig) isCredentialsEmpty() bool { c.Password == "" } +func (c providerConfig) requiresStaticCredentials() (bool, error) { + registryHostWithPath, err := c.registryHostWithPath() + if err != nil { + return false, fmt.Errorf( + "failed to get registry host with path: %w", + err, + ) + } + + knownRegistryProvider, err := credentialprovider.URLMatchesKnownRegistryProvider( + registryHostWithPath, + ) + if err != nil { + return false, fmt.Errorf( + "failed to check if registry matches a known registry provider: %w", + err, + ) + } + + // require static credentials if the registry provider is not known + return !knownRegistryProvider, nil +} + +func (c providerConfig) registryHostWithPath() (string, error) { + registryURL, err := url.ParseRequestURI(c.URL) + if err != nil { + return "", fmt.Errorf("failed parsing registry URL: %w", err) + } + + registryHostWithPath := registryURL.Host + if registryURL.Path != "" { + registryHostWithPath = path.Join(registryURL.Host, registryURL.Path) + } + + return registryHostWithPath, nil +} + func templateFilesForImageCredentialProviderConfigs( configs []providerConfig, ) ([]cabpkv1.File, error) { @@ -121,27 +155,9 @@ func templateDynamicCredentialProviderConfig( inputs := make([]templateInput, 0, len(configs)) for _, config := range configs { - registryURL, err := url.ParseRequestURI(config.URL) - if err != nil { - return nil, fmt.Errorf("failed parsing registry URL: %w", err) - } - - registryHostWithPath := registryURL.Host - if registryURL.Path != "" { - registryHostWithPath = path.Join(registryURL.Host, registryURL.Path) - } - - supportedProvider, err := credentialprovider.URLMatchesSupportedProvider( - registryHostWithPath, - ) + registryHostWithPath, err := config.registryHostWithPath() if err != nil { - return nil, fmt.Errorf( - "failed to check if registry matches a supported provider: %w", - err, - ) - } - if config.isCredentialsEmpty() && !supportedProvider { - return nil, ErrCredentialsNotFound + return nil, err } providerBinary, providerArgs, providerAPIVersion, err := dynamicCredentialProvider( diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files_test.go b/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files_test.go index abaaebc92..cf95732f1 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files_test.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files_test.go @@ -184,13 +184,6 @@ credentialProviders: `, }, }, - { - name: "error for a registry with no credentials", - credentials: []providerConfig{{ - URL: "https://registry.example.com", - }}, - wantErr: ErrCredentialsNotFound, - }, { name: "multiple registries", credentials: []providerConfig{{ diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/credentialprovider/matcher.go b/pkg/handlers/generic/mutation/imageregistries/credentials/credentialprovider/matcher.go index 667382dc3..ff92f4a2c 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/credentialprovider/matcher.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/credentialprovider/matcher.go @@ -33,7 +33,7 @@ var ( } ) -func URLMatchesSupportedProvider(target string) (bool, error) { +func URLMatchesKnownRegistryProvider(target string) (bool, error) { urlGlobs := make([]string, 0, len(ecrURLGlobs)+len(gcrURLGlobs)+len(acrURLGlobs)) urlGlobs = append(urlGlobs, ecrURLGlobs...) urlGlobs = append(urlGlobs, gcrURLGlobs...) diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go b/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go index a5f456c2b..bb4bcdfac 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go @@ -5,6 +5,7 @@ package credentials import ( "context" + "errors" "fmt" corev1 "k8s.io/api/core/v1" @@ -34,6 +35,8 @@ type imageRegistriesPatchHandler struct { variableFieldPath []string } +var ErrCredentialsNotFound = errors.New("registry credentials not found") + func NewPatch( cl ctrlclient.Client, ) *imageRegistriesPatchHandler { @@ -70,7 +73,7 @@ func (h *imageRegistriesPatchHandler) Mutate( "holderRef", holderRef, ) - imageRegistries, found, err := variables.Get[v1alpha1.ImageRegistries]( + imageRegistries, imageRegistriesFound, err := variables.Get[v1alpha1.ImageRegistries]( vars, h.variableName, h.variableFieldPath..., @@ -78,7 +81,18 @@ func (h *imageRegistriesPatchHandler) Mutate( if err != nil { return err } - if !found { + + // add credentials for global image registry mirror + globalMirror, globalMirrorFound, err := variables.Get[v1alpha1.GlobalImageRegistryMirror]( + vars, + h.variableName, + mirrors.GlobalMirrorVariableName, + ) + if err != nil { + return err + } + + if !imageRegistriesFound && !globalMirrorFound { log.V(5).Info("Image Registry Credentials variable not defined") return nil } @@ -100,16 +114,8 @@ func (h *imageRegistriesPatchHandler) Mutate( registryWithOptionalCredentials, ) } - // add credentials for global image registry mirror - globalMirror, found, err := variables.Get[v1alpha1.GlobalImageRegistryMirror]( - vars, - h.variableName, - mirrors.GlobalMirrorVariableName, - ) - if err != nil { - return err - } - if found { + + if globalMirrorFound { mirrorCredentials, generateErr := mirrorConfigFromGlobalImageRegistryMirror( ctx, h.client, @@ -125,6 +131,15 @@ func (h *imageRegistriesPatchHandler) Mutate( ) } + needCredentials, err := needImageRegistryCredentialsConfiguration(registriesWithOptionalCredentials) + if err != nil { + return err + } + if !needCredentials { + log.V(5).Info("Only Global Registry Mirror is defined but credentials are not needed") + return nil + } + files, commands, generateErr := generateFilesAndCommands( registriesWithOptionalCredentials, clusterKey.Name, @@ -359,3 +374,35 @@ func secretForImageRegistryCredentials( err := c.Get(ctx, key, secret) return secret, err } + +// This handler reads input from two user provided variables: globalImageRegistryMirror and imageRegistries. +// We expect if imageRegistries is set it will either have static credentials +// or be for a registry where the credential plugin returns the credentials, ie ECR, GCR, ACR, etc, +// and if that is not the case we assume the users missed setting static credentials and return an error. +// However, in addition to passing credentials with the globalImageRegistryMirror variable, +// it can also be used to only set Containerd mirror configuration, +// in that case it valid for static credentials to not be set and will return false, no error +// and this handler will skip generating any credential plugin related configuration. +func needImageRegistryCredentialsConfiguration(configs []providerConfig) (bool, error) { + for _, config := range configs { + requiresStaticCredentials, err := config.requiresStaticCredentials() + if err != nil { + return false, + fmt.Errorf("error determining if Image Registry is a supported provider: %w", err) + } + // verify the credentials are actually set if the plugin requires static credentials + if config.isCredentialsEmpty() && requiresStaticCredentials { + // not setting credentials for a mirror is valid + // but if it's the only configuration then return false here and exit the handler early + if config.Mirror { + if len(configs) == 1 { + return false, nil + } + } else { + return false, fmt.Errorf("invalid image registry: %s: %w", config.URL, ErrCredentialsNotFound) + } + } + } + + return true, nil +} diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go b/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go new file mode 100644 index 000000000..e51af2b70 --- /dev/null +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go @@ -0,0 +1,102 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package credentials + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_needImageRegistryCredentialsConfiguration(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + configs []providerConfig + need bool + wantErr error + }{ + { + name: "ECR credentials", + configs: []providerConfig{ + {URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com"}, + }, + need: true, + }, + { + name: "registry with static credentials", + configs: []providerConfig{{ + URL: "https://myregistry.com", + Username: "myuser", + Password: "mypassword", + }}, + need: true, + }, + { + name: "ECR mirror", + configs: []providerConfig{ + { + URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com", + Mirror: true, + }, + }, + need: true, + }, + { + name: "mirror with static credentials", + configs: []providerConfig{{ + URL: "https://myregistry.com", + Username: "myuser", + Password: "mypassword", + Mirror: true, + }}, + need: true, + }, + { + name: "mirror with no credentials", + configs: []providerConfig{{ + URL: "https://myregistry.com", + Mirror: true, + }}, + need: false, + }, + { + name: "a registry with static credentials and a mirror with no credentials", + configs: []providerConfig{ + { + URL: "https://myregistry.com", + Username: "myuser", + Password: "mypassword", + Mirror: true, + }, + { + URL: "https://myregistry.com", + Mirror: true, + }, + }, + need: true, + }, + { + name: "registry with missing credentials", + configs: []providerConfig{{ + URL: "https://myregistry.com", + }}, + need: false, + wantErr: ErrCredentialsNotFound, + }, + } + + for idx := range testCases { + tt := testCases[idx] + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + need, err := needImageRegistryCredentialsConfiguration(tt.configs) + assert.ErrorIs(t, err, tt.wantErr) + assert.Equal(t, tt.need, need) + }) + } +}