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 5a82c06d3..ee15ca6b7 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 @@ -11,6 +11,7 @@ import ( "path" "text/template" + corev1 "k8s.io/api/core/v1" credentialproviderv1 "k8s.io/kubelet/pkg/apis/credentialprovider/v1" cabpkv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" @@ -28,6 +29,8 @@ const ( kubeletDynamicCredentialProviderConfigOnRemote = "/etc/kubernetes/dynamic-credential-provider-config.yaml" azureCloudConfigFilePath = "/etc/kubernetes/azure.json" + + secretKeyForCACert = "ca.crt" ) var ( @@ -47,10 +50,11 @@ var ( ) type providerConfig struct { - URL string - Username string - Password string - Mirror bool + URL string + Username string + Password string + HasCACert bool + Mirror bool } func (c providerConfig) isCredentialsEmpty() bool { @@ -249,3 +253,12 @@ func fileFromTemplate( Permissions: "0600", }, nil } + +func secretHasCACert(secret *corev1.Secret) bool { + if secret == nil { + return false + } + + _, ok := secret.Data[secretKeyForCACert] + return ok +} diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go b/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go index 9a6a04adc..c5daa175a 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/inject.go @@ -116,7 +116,7 @@ func (h *imageRegistriesPatchHandler) Mutate( } if globalMirrorErr == nil { - mirrorCredentials, generateErr := mirrorConfigFromGlobalImageRegistryMirror( + mirrorCredentials, generateErr := mirrorWithOptionalCredentialsFromGlobalImageRegistryMirror( ctx, h.client, globalMirror, @@ -131,19 +131,19 @@ func (h *imageRegistriesPatchHandler) Mutate( ) } - needCredentials, err := needImageRegistryCredentialsConfiguration( + registriesThatNeedConfiguration, err := providerConfigsThatNeedConfiguration( registriesWithOptionalCredentials, ) if err != nil { return err } - if !needCredentials { - log.V(5).Info("Only Global Registry Mirror is defined but credentials are not needed") + if len(registriesThatNeedConfiguration) == 0 { + log.V(5).Info("Image registry credentials are not needed") return nil } files, commands, generateErr := generateFilesAndCommands( - registriesWithOptionalCredentials, + registriesThatNeedConfiguration, clusterKey.Name, ) if generateErr != nil { @@ -185,7 +185,7 @@ func (h *imageRegistriesPatchHandler) Mutate( return err } - err = createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, cluster) + err = createSecretIfNeeded(ctx, h.client, registriesThatNeedConfiguration, cluster) if err != nil { return err } @@ -243,7 +243,7 @@ func (h *imageRegistriesPatchHandler) Mutate( return err } - err = createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, cluster) + err = createSecretIfNeeded(ctx, h.client, registriesThatNeedConfiguration, cluster) if err != nil { return err } @@ -332,12 +332,13 @@ func registryWithOptionalCredentialsFromImageRegistryCredentials( if secret != nil { registryWithOptionalCredentials.Username = string(secret.Data["username"]) registryWithOptionalCredentials.Password = string(secret.Data["password"]) + registryWithOptionalCredentials.HasCACert = secretHasCACert(secret) } return registryWithOptionalCredentials, nil } -func mirrorConfigFromGlobalImageRegistryMirror( +func mirrorWithOptionalCredentialsFromGlobalImageRegistryMirror( ctx context.Context, c ctrlclient.Client, mirror v1alpha1.GlobalImageRegistryMirror, @@ -365,6 +366,7 @@ func mirrorConfigFromGlobalImageRegistryMirror( if secret != nil { mirrorCredentials.Username = string(secret.Data["username"]) mirrorCredentials.Password = string(secret.Data["password"]) + mirrorCredentials.HasCACert = secretHasCACert(secret) } return mirrorCredentials, nil @@ -438,31 +440,35 @@ func createSecretIfNeeded( // 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, +// or have no credentials set but to contain a CA cert, // 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 +// in which case it is valid for static credentials to not be set and will be skipped, no error // and this handler will skip generating any credential plugin related configuration. -func needImageRegistryCredentialsConfiguration(configs []providerConfig) (bool, error) { +func providerConfigsThatNeedConfiguration(configs []providerConfig) ([]providerConfig, error) { + var needConfiguration []providerConfig //nolint:prealloc // We don't know the size of the slice yet. for _, config := range configs { requiresStaticCredentials, err := config.requiresStaticCredentials() if err != nil { - return false, + return nil, 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) + if config.Mirror || config.HasCACert { + // not setting credentials for a mirror is valid, but won't need any configuration + // not setting credentials for a registry with a CA cert is valid, but won't need any configuration + continue } + return nil, fmt.Errorf( + "invalid image registry: %s: %w", + config.URL, + ErrCredentialsNotFound, + ) } + needConfiguration = append(needConfiguration, config) } - return true, nil + return needConfiguration, nil } diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go b/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go index e32e8f855..dad4e71ab 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go @@ -33,21 +33,23 @@ const ( validSecretName = "myregistry-credentials" ) -func Test_needImageRegistryCredentialsConfiguration(t *testing.T) { +func Test_providerConfigsThatNeedConfiguration(t *testing.T) { t.Parallel() testCases := []struct { - name string - configs []providerConfig - need bool - wantErr error + name string + configs []providerConfig + expected []providerConfig + wantErr error }{ { - name: "ECR credentials", + name: "ECR registry with no credentials", configs: []providerConfig{ {URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com"}, }, - need: true, + expected: []providerConfig{ + {URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com"}, + }, }, { name: "registry with static credentials", @@ -56,35 +58,45 @@ func Test_needImageRegistryCredentialsConfiguration(t *testing.T) { Username: "myuser", Password: "mypassword", }}, - need: true, + expected: []providerConfig{{ + URL: "https://myregistry.com", + Username: "myuser", + Password: "mypassword", + }}, }, { - name: "ECR mirror", - configs: []providerConfig{ - { - URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com", - Mirror: true, - }, - }, - need: true, + name: "ECR mirror with no credentials", + configs: []providerConfig{{ + URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com", + Mirror: true, + }}, + expected: []providerConfig{{ + URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com", + Mirror: true, + }}, }, { name: "mirror with static credentials", configs: []providerConfig{{ - URL: "https://myregistry.com", + URL: "https://mymirror.com", + Username: "myuser", + Password: "mypassword", + Mirror: true, + }}, + expected: []providerConfig{{ + URL: "https://mymirror.com", Username: "myuser", Password: "mypassword", Mirror: true, }}, - need: true, }, { name: "mirror with no credentials", configs: []providerConfig{{ - URL: "https://myregistry.com", + URL: "https://mymirror.com", Mirror: true, }}, - need: false, + expected: nil, }, { name: "a registry with static credentials and a mirror with no credentials", @@ -93,23 +105,58 @@ func Test_needImageRegistryCredentialsConfiguration(t *testing.T) { URL: "https://myregistry.com", Username: "myuser", Password: "mypassword", - Mirror: true, + Mirror: false, }, + { + URL: "https://mymirror.com", + Mirror: true, + }, + }, + expected: []providerConfig{ + { + URL: "https://myregistry.com", + Username: "myuser", + Password: "mypassword", + Mirror: false, + }, + }, + }, + { + name: "a registry with missing credentials and a mirror with no credentials", + configs: []providerConfig{ { URL: "https://myregistry.com", + Mirror: false, + }, + { + URL: "https://mymirror.com", Mirror: true, }, }, - need: true, + wantErr: ErrCredentialsNotFound, }, { name: "registry with missing credentials", configs: []providerConfig{{ URL: "https://myregistry.com", }}, - need: false, wantErr: ErrCredentialsNotFound, }, + { + name: "registry with missing credentials but with a CA", + configs: []providerConfig{{ + URL: "https://myregistry.com", + HasCACert: true, + }}, + }, + { + name: "mirror with missing credentials but with a CA", + configs: []providerConfig{{ + URL: "https://mymirror.com", + HasCACert: true, + Mirror: true, + }}, + }, } for idx := range testCases { @@ -118,9 +165,9 @@ func Test_needImageRegistryCredentialsConfiguration(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - need, err := needImageRegistryCredentialsConfiguration(tt.configs) + expected, err := providerConfigsThatNeedConfiguration(tt.configs) require.ErrorIs(t, err, tt.wantErr) - assert.Equal(t, tt.need, need) + assert.Equal(t, tt.expected, expected) }) } }