Skip to content

Commit

Permalink
fix: handle globalImageRegistryMirror with no credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
dkoshkin committed Feb 15, 2024
1 parent 3e43d0d commit abb1e9c
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package credentials
import (
"bytes"
_ "embed"
"errors"
"fmt"
"net/url"
"path"
Expand Down Expand Up @@ -47,8 +46,6 @@ var (
)
)

var ErrCredentialsNotFound = errors.New("registry credentials not found")

type providerConfig struct {
URL string
Username string
Expand All @@ -61,6 +58,42 @@ func (c providerConfig) isCredentialsEmpty() bool {
c.Password == ""
}

func (c providerConfig) isSupportedProvider() (bool, error) {
registryHostWithPath, err := c.registryHostWithPath()
if err != nil {
return false, fmt.Errorf(
"failed to get registry host with path: %w",
err,
)
}

supportedProvider, err := credentialprovider.URLMatchesSupportedProvider(
registryHostWithPath,
)
if err != nil {
return false, fmt.Errorf(
"failed to check if registry matches a supported provider: %w",
err,
)
}

return supportedProvider, 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) {
Expand Down Expand Up @@ -121,27 +154,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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package credentials

import (
"context"
"errors"
"fmt"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -34,6 +35,8 @@ type imageRegistriesPatchHandler struct {
variableFieldPath []string
}

var ErrCredentialsNotFound = errors.New("registry credentials not found")

func NewPatch(
cl ctrlclient.Client,
) *imageRegistriesPatchHandler {
Expand Down Expand Up @@ -128,6 +131,15 @@ func (h *imageRegistriesPatchHandler) Mutate(
)
}

needCredentials, err := needImageRegistryCredentials(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,
Expand Down Expand Up @@ -362,3 +374,27 @@ func secretForImageRegistryCredentials(
err := c.Get(ctx, key, secret)
return secret, err
}

// needImageRegistryCredentials will return true if all the providerConfigs have valid credentials
// will return false if there is only a single providerConfig for a mirror with no credentials
// otherwise will return an error
func needImageRegistryCredentials(configs []providerConfig) (bool, error) {
for _, config := range configs {
supportedProvider, err := config.isSupportedProvider()
if err != nil {
return false,
fmt.Errorf("error determining if Image Registry is a supported provider: %w", err)
}
if config.isCredentialsEmpty() && !supportedProvider {
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
}
Original file line number Diff line number Diff line change
@@ -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_needImageRegistryCredentials(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 := needImageRegistryCredentials(tt.configs)
assert.ErrorIs(t, err, tt.wantErr)
assert.Equal(t, tt.need, need)
})
}
}

0 comments on commit abb1e9c

Please sign in to comment.