From cbae55b3eb45fd9a0d1e6f6cff45d0464342bcd5 Mon Sep 17 00:00:00 2001 From: Rafael Matias Date: Fri, 4 Apr 2025 21:25:30 +0200 Subject: [PATCH] fix: docker auth wasn't being used --- .../engine_functions/create_engine.go | 1 + .../docker_config_storage_creator.go | 7 +- .../docker/docker_manager/docker_auth.go | 23 +++++- .../docker/docker_manager/docker_auth_test.go | 75 +++++++++++++++---- .../docker/docker_manager/docker_manager.go | 1 + 5 files changed, 87 insertions(+), 20 deletions(-) diff --git a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/create_engine.go b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/create_engine.go index 8a38a59954..600f9a2332 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/create_engine.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/create_engine.go @@ -269,6 +269,7 @@ func CreateEngine( if err = dockerManager.CreateVolume(ctx, dockerConfigStorageVolNameStr, dockerConfigStorageVolLabelStrs); err != nil { return nil, stacktrace.Propagate(err, "An error occurred creating Docker config storage volume.") } + logrus.Tracef("Creating Docker config storage") err = docker_config_storage_creator.CreateDockerConfigStorage(ctx, targetNetworkId, dockerConfigStorageVolNameStr, consts.DockerConfigStorageDirPath, dockerManager) if err != nil { return nil, stacktrace.Propagate(err, "An error occurred creating Docker config storage.") diff --git a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/docker_config_storage_creator/docker_config_storage_creator.go b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/docker_config_storage_creator/docker_config_storage_creator.go index ffc53d15de..90e970f7ad 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/docker_config_storage_creator/docker_config_storage_creator.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/engine_functions/docker_config_storage_creator/docker_config_storage_creator.go @@ -111,14 +111,19 @@ func storeConfigInVolume( } // Add the auths for each registry + logrus.Tracef("Getting auth from docker config for registries: %v", registries) for _, registry := range registries { + logrus.Tracef("Getting auth from docker config for registry: %s", registry) creds, err := docker_manager.GetAuthFromDockerConfig(registry) if err != nil { logrus.Warnf("An error occurred getting auth for registry '%v' from Docker config: %v", registry, err) } // creds can be nil if the registry doesn't have auth - if err != nil && creds != nil { + if err == nil && creds != nil { cfg.Auths[registry] = *creds + logrus.Tracef("Found auth config for docker registry: %s", registry) + } else { + logrus.Tracef("No auth config found for docker registry: %s", registry) } } diff --git a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth.go b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth.go index 8e0d47f19c..05c3cbd98b 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/sirupsen/logrus" "os" "os/exec" "strings" @@ -14,6 +13,7 @@ import ( "github.com/docker/docker/api/types/registry" dockerregistry "github.com/docker/docker/registry" "github.com/kurtosis-tech/stacktrace" + "github.com/sirupsen/logrus" ) const ( @@ -36,6 +36,8 @@ func loadDockerAuth() (RegistryAuthConfig, error) { configFilePath = configFilePath + "/config.json" } + logrus.Infof("Loading docker auth from config file: %s", configFilePath) + file, err := os.ReadFile(configFilePath) if errors.Is(err, os.ErrNotExist) { // If the auth config doesn't exist, return an empty auth config @@ -50,6 +52,15 @@ func loadDockerAuth() (RegistryAuthConfig, error) { return emptyRegistryAuthConfig(), stacktrace.Propagate(err, "error unmarshalling Docker config file at '%s'", configFilePath) } + // Remove trailing slashes from registry URLs + for registry, auth := range authConfig.Auths { + if strings.HasSuffix(registry, "/") { + delete(authConfig.Auths, registry) + registry = strings.TrimSuffix(registry, "/") + authConfig.Auths[registry] = auth + } + } + return authConfig, nil } @@ -144,14 +155,18 @@ func GetAuthFromDockerConfig(repo string) (*registry.AuthConfig, error) { } // if repo string doesn't contain a repo prefix assume its an official docker library image - if !strings.Contains(repo, "/") { + if !strings.Contains(repo, "/") && !strings.Contains(repo, ".") { repo = "library/" + repo } registryHost := dockerregistry.ConvertToHostname(repo) - if !strings.Contains(registryHost, ".") || registryHost == "docker.io" || registryHost == "registry-1.docker.io" { - registryHost = "https://index.docker.io/v1/" + // Deal with the default Docker Hub registry. + if !strings.Contains(registryHost, ".") || + registryHost == "docker.io" || + registryHost == "registry-1.docker.io" || + registryHost == "index.docker.io" { + registryHost = "https://index.docker.io/v1" } // Check if the URL contains "://", meaning it already has a protocol diff --git a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth_test.go b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth_test.go index 2db8dbc70f..9cbc7bc2a2 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth_test.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_auth_test.go @@ -40,38 +40,83 @@ func TestGetAuthWithNoAuthSetReturnsNilAndNoError(t *testing.T) { } func TestGetAuthConfigForRepoPlain(t *testing.T) { - expectedUser := "user" - expectedPassword := "password" + expectedUserDockerHub := "dhuser" + expectedPasswordDockerHub := "dhpassword" + expectedUserGithub := "ghuser" + expectedPasswordGithub := "ghpassword" - encodedAuth := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", expectedUser, expectedPassword))) + encodedAuthDockerHub := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", expectedUserDockerHub, expectedPasswordDockerHub))) + encodedAuthGithub := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", expectedUserGithub, expectedPasswordGithub))) cfg := fmt.Sprintf(` { "auths": { "https://index.docker.io/v1/": { "auth": "%s" + }, + "https://ghcr.io": { + "auth": "%s" } } - }`, encodedAuth) + }`, encodedAuthDockerHub, encodedAuthGithub) tmpDir := writeStaticConfig(t, cfg) defer os.RemoveAll(tmpDir) - // Test 1: Retrieve auth config for Docker Hub using docker.io domain - authConfig, err := GetAuthFromDockerConfig("docker.io/my-repo/my-image:latest") - assert.NoError(t, err) - assert.Equal(t, encodedAuth, authConfig.Auth, "Auth for Docker Hub should match") + testCases := []struct { + repo string + expectedAuth string + }{ + { + repo: "docker.io/my-repo/my-image:latest", + expectedAuth: encodedAuthDockerHub, + }, + { + repo: "my-repo/my-image:latest", + expectedAuth: encodedAuthDockerHub, + }, + { + repo: "https://registry-1.docker.io/my-repo/my-image:latest", + expectedAuth: encodedAuthDockerHub, + }, + { + repo: "https://index.docker.io/v1/", + expectedAuth: encodedAuthDockerHub, + }, + { + repo: "https://index.docker.io/v1", + expectedAuth: encodedAuthDockerHub, + }, + { + repo: "ghcr.io/my-repo/my-image:latest", + expectedAuth: encodedAuthGithub, + }, + { + repo: "ghcr.io", + expectedAuth: encodedAuthGithub, + }, + { + repo: "ghcr.io/", + expectedAuth: encodedAuthGithub, + }, + } - // Test 2: Retrieve auth config for Docker Hub using no domain - authConfig, err = GetAuthFromDockerConfig("my-repo/my-image:latest") - assert.NoError(t, err) - assert.Equal(t, encodedAuth, authConfig.Auth, "Auth for Docker Hub should match when using no host prefix") + for _, testCase := range testCases { + authConfig, err := GetAuthFromDockerConfig(testCase.repo) + assert.NoError(t, err) + assert.NotNil(t, authConfig, "Auth config should not be nil") + assert.Equal(t, testCase.expectedAuth, authConfig.Auth, "Auth for Docker Hub should match") + } - // Test 3: Retrieve auth config for Docker Hub using full domain and https:// prefix - authConfig, err = GetAuthFromDockerConfig("https://registry-1.docker.io/my-repo/my-image:latest") + authConfig, err := GetAuthFromDockerConfig("something-else.local") assert.NoError(t, err) - assert.Equal(t, encodedAuth, authConfig.Auth, "Auth for Docker Hub should match when using no host prefix") + assert.Nil(t, authConfig, "Auth config should be nil") + registries, err := GetAllRegistriesFromDockerConfig() + assert.NoError(t, err) + assert.Equal(t, 2, len(registries)) + assert.Contains(t, registries, "https://index.docker.io/v1") + assert.Contains(t, registries, "https://ghcr.io") } func TestGetAuthConfigForRepoOSX(t *testing.T) { diff --git a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go index 7a847ae8d4..3d71ea87c4 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go @@ -2297,6 +2297,7 @@ func pullImage(dockerClient *client.Client, imageName string, registrySpec *imag logrus.Warnf("Falling back to pulling image with no auth config.") } else { imagePullOptions.RegistryAuth = authFromConfig + logrus.Infof("Using authentication to pull image: %s", imageName) } }