feat(autopatch): implement OCI registry patch finder BE-12111 (#1044)

2025-08-27 19:04:41 +12:00 · 2025-08-27 19:04:41 +12:00 · 965ef5246b
parent 9c88057bd1
commit 965ef5246b
3 changed files with 60 additions and 50 deletions
--- 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
 }

--- 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")
 		})
 	}
 }
--- 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: