diff --git a/pkg/liboras/registry.go b/pkg/liboras/registry.go index 317619700..de28cd40d 100644 --- a/pkg/liboras/registry.go +++ b/pkg/liboras/registry.go @@ -5,11 +5,15 @@ import ( portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/pkg/registryhttp" + "github.com/rs/zerolog/log" "oras.land/oras-go/v2/registry/remote" "oras.land/oras-go/v2/registry/remote/auth" ) +// CreateClient creates a new ORAS registry client based on the provided portainer.Registry. +// It configures the client for authentication if the registry requires it. +// Furthermore, the client is configured to use the default retry client. Its policy is found in retry.DefaultPolicy func CreateClient(registry portainer.Registry) (*remote.Registry, error) { registryClient, err := remote.NewRegistry(registry.URL) if err != nil { @@ -32,36 +36,39 @@ func CreateClient(registry portainer.Registry) (*remote.Registry, error) { registryClient.TagListPageSize = 1000 registryClient.ReferrerListPageSize = 1000 - // Only apply authentication if explicitly enabled AND credentials are provided - if registry.Authentication && + authClient := &auth.Client{ + Client: httpClient, + } + + configureCredentials := registry.Authentication && strings.TrimSpace(registry.Username) != "" && - strings.TrimSpace(registry.Password) != "" { - - registryClient.Client = &auth.Client{ - Client: httpClient, - Cache: auth.NewCache(), - Credential: auth.StaticCredential(registry.URL, auth.Credential{ - Username: registry.Username, - Password: registry.Password, - }), - } - - log.Debug(). - Str("registryURL", registry.URL). - Str("registryType", getRegistryTypeName(registry.Type)). - Bool("authentication", true). - Msg("Created ORAS registry client with authentication") - } else { - // Use the configured HTTP client for anonymous access - registryClient.Client = httpClient - + strings.TrimSpace(registry.Password) != "" + if !configureCredentials { + // The authClient is still needed to handle anonymous access and token refresh. For instance to send requests to + // DockerHub which requires a token even for anonymous access. + registryClient.Client = authClient log.Debug(). Str("registryURL", registry.URL). Str("registryType", getRegistryTypeName(registry.Type)). Bool("authentication", false). Msg("Created ORAS registry client for anonymous access") + + return registryClient, nil } + authClient.Cache = auth.NewCache() + authClient.Credential = auth.StaticCredential(registry.URL, auth.Credential{ + Username: registry.Username, + Password: registry.Password, + }) + registryClient.Client = authClient + + log.Debug(). + Str("registryURL", registry.URL). + Str("registryType", getRegistryTypeName(registry.Type)). + Bool("authentication", true). + Msg("Created ORAS registry client with authentication") + return registryClient, nil } diff --git a/pkg/liboras/registry_test.go b/pkg/liboras/registry_test.go index 8fe502dec..064217749 100644 --- a/pkg/liboras/registry_test.go +++ b/pkg/liboras/registry_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "oras.land/oras-go/v2/registry/remote/auth" - "oras.land/oras-go/v2/registry/remote/retry" ) func TestCreateClient_AuthenticationScenarios(t *testing.T) { @@ -16,6 +15,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { name string registry portainer.Registry expectAuthenticated bool + expectTLS bool description string }{ { @@ -27,6 +27,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: "testpass", }, expectAuthenticated: false, + expectTLS: false, description: "Even with credentials present, authentication=false should result in anonymous access", }, { @@ -38,6 +39,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: "testpass", }, expectAuthenticated: true, + expectTLS: false, description: "Valid credentials with authentication=true should result in authenticated access", }, { @@ -49,6 +51,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: "testpass", }, expectAuthenticated: false, + expectTLS: false, description: "Empty username should fallback to anonymous access", }, { @@ -60,6 +63,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: "testpass", }, expectAuthenticated: false, + expectTLS: false, description: "Whitespace-only username should fallback to anonymous access", }, { @@ -71,6 +75,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: "", }, expectAuthenticated: false, + expectTLS: false, description: "Empty password should fallback to anonymous access", }, { @@ -82,6 +87,7 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: " ", }, expectAuthenticated: false, + expectTLS: false, description: "Whitespace-only password should fallback to anonymous access", }, { @@ -93,19 +99,9 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { Password: "", }, expectAuthenticated: false, + expectTLS: false, description: "Both credentials empty should fallback to anonymous access", }, - { - name: "public registry with no authentication should create anonymous client", - registry: portainer.Registry{ - URL: "docker.io", - Authentication: false, - Username: "", - Password: "", - }, - expectAuthenticated: false, - description: "Public registries without authentication should use anonymous access", - }, { name: "GitLab registry with valid credentials should create authenticated client", registry: portainer.Registry{ @@ -121,8 +117,20 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { }, }, expectAuthenticated: true, + expectTLS: true, description: "GitLab registry with valid credentials should result in authenticated access", }, + { + name: "DockerHub registry without credentials should create anonymous client with TLS", + registry: portainer.Registry{ + Type: portainer.DockerHubRegistry, + URL: "registry-1.docker.io", + Authentication: false, + }, + expectAuthenticated: false, + expectTLS: true, + description: "DockerHub registry with without credentials should result in authenticated anonymous access", + }, } for _, tt := range tests { @@ -132,25 +140,19 @@ func TestCreateClient_AuthenticationScenarios(t *testing.T) { require.NoError(t, err, "CreateClient should not return an error") assert.NotNil(t, client, "Client should not be nil") + // Should have auth.Client with credentials regardless of authentication setting + // If authentication is enabled, it should have the credentials set. + // If authentication is disabled, it should still be an auth.Client but with retry.DefaultClient + // (which handles anonymous access by requesting an anonymous token if needed). + authClient, ok := client.Client.(*auth.Client) + assert.True(t, ok, "Expected auth.Client for authenticated access") + assert.NotNil(t, authClient, "Auth client should not be nil") // Check if the client has authentication configured if tt.expectAuthenticated { - // Should have auth.Client with credentials - authClient, ok := client.Client.(*auth.Client) - assert.True(t, ok, "Expected auth.Client for authenticated access") - assert.NotNil(t, authClient, "Auth client should not be nil") assert.NotNil(t, authClient.Credential, "Credential function should be set") - } else { - // For anonymous access without custom TLS, all registries should use retry.DefaultClient - // (Only registries with custom TLS configuration use a different retry client) - if tt.registry.ManagementConfiguration == nil || !tt.registry.ManagementConfiguration.TLSConfig.TLS { - assert.Equal(t, retry.DefaultClient, client.Client, - "Expected retry.DefaultClient for anonymous access without custom TLS") - } else { - // Custom TLS configuration means a custom retry client - assert.NotEqual(t, retry.DefaultClient, client.Client, - "Expected custom retry client for registry with custom TLS") - } } + + assert.Equal(t, tt.expectTLS, !client.PlainHTTP, "Expected TLS setting to match registry configuration") }) } } diff --git a/pkg/registryhttp/client.go b/pkg/registryhttp/client.go index ab2b0f6f1..fb95e6654 100644 --- a/pkg/registryhttp/client.go +++ b/pkg/registryhttp/client.go @@ -5,6 +5,7 @@ import ( portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/crypto" + "github.com/rs/zerolog/log" "oras.land/oras-go/v2/registry/remote/retry" ) @@ -14,7 +15,7 @@ import ( // Returns the HTTP client, whether to use plainHTTP, and any error. func CreateClient(registry *portainer.Registry) (*http.Client, bool, error) { switch registry.Type { - case portainer.AzureRegistry, portainer.EcrRegistry, portainer.GithubRegistry, portainer.GitlabRegistry: + case portainer.AzureRegistry, portainer.EcrRegistry, portainer.GithubRegistry, portainer.GitlabRegistry, portainer.DockerHubRegistry: // Cloud registries use the default retry client with built-in TLS return retry.DefaultClient, false, nil default: