From 4f525d36a78f0ab6a8495e8bdafc817de30eac51 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Sat, 20 Jul 2024 19:34:58 +0100 Subject: [PATCH] fix!: docker authentication setup Check and return errors in the process of determining authentication configs so that unexpected failures don't occur latter in the process of build an image or creating a container. BuildOptions will now return an empty result on error instead of an incomplete one, to ensure that consumers don't use partial data. Fix builds with different config or override environments failing when the authentication configuration changes, which was introduced by #2646. Report errors from GetRegistryCredentials calls to avoid unexpected failures latter on in the authentication process. Split out the functionality to read a Dockerfile from an io.Reader into ExtractImagesFromReader, as required when processing from a tar archive. Deprecated function ContainerRequest.GetAuthConfigs will now panic if an error occurs, so that callers understand that an failure occurred. Remove unused parameter t from prepareRedisImage. BREAKING CHANGE Add support for determining the required authentication in when building an image from a ContextArchive, this requires ContextArchive support io.Seeker. --- container.go | 90 +++++++++++++++++++---- container_test.go | 15 ++-- docker.go | 6 +- docker_auth.go | 155 +++++++++++++++++++++++++++++++++------- docker_auth_test.go | 40 +++++++++-- internal/core/images.go | 12 +++- 6 files changed, 262 insertions(+), 56 deletions(-) diff --git a/container.go b/container.go index 6f17a7e3ff..8747335a28 100644 --- a/container.go +++ b/container.go @@ -1,6 +1,7 @@ package testcontainers import ( + "archive/tar" "context" "errors" "fmt" @@ -10,6 +11,7 @@ import ( "strings" "time" + "github.com/cpuguy83/dockercfg" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" @@ -85,7 +87,7 @@ type ImageBuildInfo interface { // rather than using a pre-built one type FromDockerfile struct { Context string // the path to the context of the docker build - ContextArchive io.Reader // the tar archive file to send to docker that contains the build context + ContextArchive io.ReadSeeker // the tar archive file to send to docker that contains the build context Dockerfile string // the path from the context to the Dockerfile for the image, defaults to "Dockerfile" Repo string // the repo label for image, defaults to UUID Tag string // the tag label for image, defaults to UUID @@ -305,30 +307,90 @@ func (c *ContainerRequest) GetTag() string { return strings.ToLower(t) } -// Deprecated: Testcontainers will detect registry credentials automatically, and it will be removed in the next major release -// GetAuthConfigs returns the auth configs to be able to pull from an authenticated docker registry +// Deprecated: Testcontainers will detect registry credentials automatically, and it will be removed in the next major release. +// GetAuthConfigs returns the auth configs to be able to pull from an authenticated docker registry. +// Panics if an error occurs. func (c *ContainerRequest) GetAuthConfigs() map[string]registry.AuthConfig { - return getAuthConfigsFromDockerfile(c) + auth, err := getAuthConfigsFromDockerfile(c) + if err != nil { + panic(fmt.Sprintf("failed to get auth configs from Dockerfile: %v", err)) + } + return auth +} + +// dockerFileImages returns the images from the request Dockerfile. +func (c *ContainerRequest) dockerFileImages() ([]string, error) { + if c.ContextArchive == nil { + // Source is a directory, we can read the Dockerfile directly. + images, err := core.ExtractImagesFromDockerfile(filepath.Join(c.Context, c.GetDockerfile()), c.GetBuildArgs()) + if err != nil { + return nil, fmt.Errorf("extract images from Dockerfile: %w", err) + } + + return images, nil + } + + // Source is an archive, we need to read it to get the Dockerfile. + dockerFile := c.GetDockerfile() + tr := tar.NewReader(c.FromDockerfile.ContextArchive) + + for { + hdr, err := tr.Next() + if err != nil { + if errors.Is(err, io.EOF) { + return nil, fmt.Errorf("Dockerfile %q not found in context archive", dockerFile) + } + + return nil, fmt.Errorf("reading tar archive: %w", err) + } + + if hdr.Name != dockerFile { + continue + } + + images, err := core.ExtractImagesFromReader(tr, c.GetBuildArgs()) + if err != nil { + return nil, fmt.Errorf("extract images from Dockerfile: %w", err) + } + + // Reset the archive to the beginning. + if _, err := c.ContextArchive.Seek(0, io.SeekStart); err != nil { + return nil, fmt.Errorf("seek context archive to start: %w", err) + } + + return images, nil + } } // getAuthConfigsFromDockerfile returns the auth configs to be able to pull from an authenticated docker registry -func getAuthConfigsFromDockerfile(c *ContainerRequest) map[string]registry.AuthConfig { - images, err := core.ExtractImagesFromDockerfile(filepath.Join(c.Context, c.GetDockerfile()), c.GetBuildArgs()) +func getAuthConfigsFromDockerfile(c *ContainerRequest) (map[string]registry.AuthConfig, error) { + images, err := c.dockerFileImages() if err != nil { - return map[string]registry.AuthConfig{} + return nil, fmt.Errorf("docker file images: %w", err) + } + + // Get the auth configs once for all images as it can be a time-consuming operation. + configs, err := getDockerAuthConfigs() + if err != nil { + return nil, err } authConfigs := map[string]registry.AuthConfig{} for _, image := range images { - registry, authConfig, err := DockerImageAuth(context.Background(), image) + registry, authConfig, err := dockerImageAuth(context.Background(), image, configs) if err != nil { + if !errors.Is(err, dockercfg.ErrCredentialsNotFound) { + return nil, fmt.Errorf("docker image auth %q: %w", image, err) + } + + // Credentials not found no config to add. continue } authConfigs[registry] = authConfig } - return authConfigs + return authConfigs, nil } func (c *ContainerRequest) ShouldBuildImage() bool { @@ -361,7 +423,10 @@ func (c *ContainerRequest) BuildOptions() (types.ImageBuildOptions, error) { buildOptions.Dockerfile = c.GetDockerfile() // Make sure the auth configs from the Dockerfile are set right after the user-defined build options. - authsFromDockerfile := getAuthConfigsFromDockerfile(c) + authsFromDockerfile, err := getAuthConfigsFromDockerfile(c) + if err != nil { + return types.ImageBuildOptions{}, fmt.Errorf("auth configs from Dockerfile: %w", err) + } if buildOptions.AuthConfigs == nil { buildOptions.AuthConfigs = map[string]registry.AuthConfig{} @@ -378,7 +443,7 @@ func (c *ContainerRequest) BuildOptions() (types.ImageBuildOptions, error) { for _, is := range c.ImageSubstitutors { modifiedTag, err := is.Substitute(tag) if err != nil { - return buildOptions, fmt.Errorf("failed to substitute image %s with %s: %w", tag, is.Description(), err) + return types.ImageBuildOptions{}, fmt.Errorf("failed to substitute image %s with %s: %w", tag, is.Description(), err) } if modifiedTag != tag { @@ -401,8 +466,9 @@ func (c *ContainerRequest) BuildOptions() (types.ImageBuildOptions, error) { // Do this as late as possible to ensure we don't leak the context on error/panic. buildContext, err := c.GetContext() if err != nil { - return buildOptions, err + return types.ImageBuildOptions{}, err } + buildOptions.Context = buildContext return buildOptions, nil diff --git a/container_test.go b/container_test.go index d2a070f2de..3cb14ac296 100644 --- a/container_test.go +++ b/container_test.go @@ -147,7 +147,7 @@ func Test_BuildImageWithContexts(t *testing.T) { type TestCase struct { Name string ContextPath string - ContextArchive func() (io.Reader, error) + ContextArchive func() (io.ReadSeeker, error) ExpectedEchoOutput string Dockerfile string ExpectedError string @@ -157,7 +157,7 @@ func Test_BuildImageWithContexts(t *testing.T) { { Name: "test build from context archive", // fromDockerfileWithContextArchive { - ContextArchive: func() (io.Reader, error) { + ContextArchive: func() (io.ReadSeeker, error) { var buf bytes.Buffer tarWriter := tar.NewWriter(&buf) files := []struct { @@ -202,7 +202,7 @@ func Test_BuildImageWithContexts(t *testing.T) { }, { Name: "test build from context archive and be able to use files in it", - ContextArchive: func() (io.Reader, error) { + ContextArchive: func() (io.ReadSeeker, error) { var buf bytes.Buffer tarWriter := tar.NewWriter(&buf) files := []struct { @@ -255,14 +255,14 @@ func Test_BuildImageWithContexts(t *testing.T) { ContextPath: "./testdata", Dockerfile: "echo.Dockerfile", ExpectedEchoOutput: "this is from the echo test Dockerfile", - ContextArchive: func() (io.Reader, error) { + ContextArchive: func() (io.ReadSeeker, error) { return nil, nil }, }, { Name: "it should error if neither a context nor a context archive are specified", ContextPath: "", - ContextArchive: func() (io.Reader, error) { + ContextArchive: func() (io.ReadSeeker, error) { return nil, nil }, ExpectedError: "create container: you must specify either a build context or an image", @@ -275,9 +275,8 @@ func Test_BuildImageWithContexts(t *testing.T) { t.Parallel() ctx := context.Background() a, err := testCase.ContextArchive() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) + req := testcontainers.ContainerRequest{ FromDockerfile: testcontainers.FromDockerfile{ ContextArchive: a, diff --git a/docker.go b/docker.go index 6bd80c39c4..c54da63044 100644 --- a/docker.go +++ b/docker.go @@ -1114,7 +1114,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque // forward the host ports to the container ports. sshdForwardPortsHook, err := exposeHostPorts(ctx, &req, req.HostAccessPorts...) if err != nil { - return nil, fmt.Errorf("failed to expose host ports: %w", err) + return nil, fmt.Errorf("expose host ports: %w", err) } defaultHooks = append(defaultHooks, sshdForwardPortsHook) @@ -1292,12 +1292,12 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain func (p *DockerProvider) attemptToPullImage(ctx context.Context, tag string, pullOpt image.PullOptions) error { registry, imageAuth, err := DockerImageAuth(ctx, tag) if err != nil { - p.Logger.Printf("Failed to get image auth for %s. Setting empty credentials for the image: %s. Error is:%s", registry, tag, err) + p.Logger.Printf("Failed to get image auth for %s. Setting empty credentials for the image: %s. Error is: %s", registry, tag, err) } else { // see https://github.com/docker/docs/blob/e8e1204f914767128814dca0ea008644709c117f/engine/api/sdk/examples.md?plain=1#L649-L657 encodedJSON, err := json.Marshal(imageAuth) if err != nil { - p.Logger.Printf("Failed to marshal image auth. Setting empty credentials for the image: %s. Error is:%s", tag, err) + p.Logger.Printf("Failed to marshal image auth. Setting empty credentials for the image: %s. Error is: %s", tag, err) } else { pullOpt.RegistryAuth = base64.URLEncoding.EncodeToString(encodedJSON) } diff --git a/docker_auth.go b/docker_auth.go index d45393f325..99e2d2fdba 100644 --- a/docker_auth.go +++ b/docker_auth.go @@ -2,8 +2,13 @@ package testcontainers import ( "context" + "crypto/md5" "encoding/base64" + "encoding/hex" "encoding/json" + "errors" + "fmt" + "io" "net/url" "os" "sync" @@ -21,15 +26,21 @@ var defaultRegistryFn = defaultRegistry // Finally, it will use the credential helpers to extract the information from the docker config file // for that registry, if it exists. func DockerImageAuth(ctx context.Context, image string) (string, registry.AuthConfig, error) { - defaultRegistry := defaultRegistryFn(ctx) - reg := core.ExtractRegistry(image, defaultRegistry) - - cfgs, err := getDockerAuthConfigs() + configs, err := getDockerAuthConfigs() if err != nil { + reg := core.ExtractRegistry(image, defaultRegistryFn(ctx)) return reg, registry.AuthConfig{}, err } - if cfg, ok := getRegistryAuth(reg, cfgs); ok { + return dockerImageAuth(ctx, image, configs) +} + +// dockerImageAuth returns the auth config for the given Docker image. +func dockerImageAuth(ctx context.Context, image string, configs map[string]registry.AuthConfig) (string, registry.AuthConfig, error) { + defaultRegistry := defaultRegistryFn(ctx) + reg := core.ExtractRegistry(image, defaultRegistry) + + if cfg, ok := getRegistryAuth(reg, configs); ok { return reg, cfg, nil } @@ -80,24 +91,93 @@ func defaultRegistry(ctx context.Context) string { return info.IndexServerAddress } -// authConfig represents the details of the auth config for a registry. -type authConfig struct { +// authConfigResult is a result looking up auth details for key. +type authConfigResult struct { key string cfg registry.AuthConfig + err error +} + +// credentialsCache is a cache for registry credentials. +type credentialsCache struct { + entries map[string]credentials + mtx sync.RWMutex +} + +// credentials represents the username and password for a registry. +type credentials struct { + username string + password string +} + +var creds = &credentialsCache{entries: map[string]credentials{}} + +// Get returns the username and password for the given hostname +// as determined by the details in configPath. +func (c *credentialsCache) Get(hostname, configKey string) (string, string, error) { + key := configKey + ":" + hostname + c.mtx.RLock() + entry, ok := c.entries[key] + c.mtx.RUnlock() + + if ok { + return entry.username, entry.password, nil + } + + // No entry found, request and cache. + user, password, err := dockercfg.GetRegistryCredentials(hostname) + if err != nil { + return "", "", fmt.Errorf("getting credentials for %s: %w", hostname, err) + } + + c.mtx.Lock() + c.entries[key] = credentials{username: user, password: password} + c.mtx.Unlock() + + return user, password, nil +} + +// configFileKey returns a key to use for caching credentials based on +// the contents of the currently active config. +func configFileKey() (string, error) { + configPath, err := dockercfg.ConfigPath() + if err != nil { + return "", err + } + + f, err := os.Open(configPath) + if err != nil { + return "", fmt.Errorf("open config file: %w", err) + } + + defer f.Close() + + h := md5.New() + if _, err := io.Copy(h, f); err != nil { + return "", fmt.Errorf("copying config file: %w", err) + } + + return hex.EncodeToString(h.Sum(nil)), nil } // getDockerAuthConfigs returns a map with the auth configs from the docker config file // using the registry as the key -var getDockerAuthConfigs = sync.OnceValues(func() (map[string]registry.AuthConfig, error) { +func getDockerAuthConfigs() (map[string]registry.AuthConfig, error) { cfg, err := getDockerConfig() if err != nil { return nil, err } - cfgs := map[string]registry.AuthConfig{} - results := make(chan authConfig, len(cfg.AuthConfigs)+len(cfg.CredentialHelpers)) + configKey, err := configFileKey() + if err != nil { + return nil, err + } + + size := len(cfg.AuthConfigs) + len(cfg.CredentialHelpers) + cfgs := make(map[string]registry.AuthConfig, size) + results := make(chan authConfigResult, size) var wg sync.WaitGroup - wg.Add(len(cfg.AuthConfigs) + len(cfg.CredentialHelpers)) + wg.Add(size) for k, v := range cfg.AuthConfigs { go func(k string, v dockercfg.AuthConfig) { defer wg.Done() @@ -112,16 +192,23 @@ var getDockerAuthConfigs = sync.OnceValues(func() (map[string]registry.AuthConfi Username: v.Username, } - if v.Username == "" && v.Password == "" { - u, p, _ := dockercfg.GetRegistryCredentials(k) + switch { + case ac.Username == "" && ac.Password == "": + // Look up credentials from the credential store. + u, p, err := creds.Get(k, configKey) + if err != nil { + results <- authConfigResult{err: err} + return + } + ac.Username = u ac.Password = p - } - - if v.Auth == "" { + case ac.Auth == "": + // Create auth from the username and password encoding. ac.Auth = base64.StdEncoding.EncodeToString([]byte(ac.Username + ":" + ac.Password)) } - results <- authConfig{key: k, cfg: ac} + + results <- authConfigResult{key: k, cfg: ac} }(k, v) } @@ -131,11 +218,19 @@ var getDockerAuthConfigs = sync.OnceValues(func() (map[string]registry.AuthConfi go func(k string) { defer wg.Done() - ac := registry.AuthConfig{} - u, p, _ := dockercfg.GetRegistryCredentials(k) - ac.Username = u - ac.Password = p - results <- authConfig{key: k, cfg: ac} + u, p, err := creds.Get(k, configKey) + if err != nil { + results <- authConfigResult{err: err} + return + } + + results <- authConfigResult{ + key: k, + cfg: registry.AuthConfig{ + Username: u, + Password: p, + }, + } }(k) } @@ -144,12 +239,22 @@ var getDockerAuthConfigs = sync.OnceValues(func() (map[string]registry.AuthConfi close(results) }() - for ac := range results { - cfgs[ac.key] = ac.cfg + var errs []error + for result := range results { + if result.err != nil { + errs = append(errs, result.err) + continue + } + + cfgs[result.key] = result.cfg + } + + if len(errs) > 0 { + return nil, errors.Join(errs...) } return cfgs, nil -}) +} // getDockerConfig returns the docker config file. It will internally check, in this particular order: // 1. the DOCKER_AUTH_CONFIG environment variable, unmarshalling it into a dockercfg.Config diff --git a/docker_auth_test.go b/docker_auth_test.go index 1f8df17c09..576a322aca 100644 --- a/docker_auth_test.go +++ b/docker_auth_test.go @@ -2,6 +2,7 @@ package testcontainers import ( "context" + _ "embed" "fmt" "os" "path/filepath" @@ -9,10 +10,10 @@ import ( "github.com/cpuguy83/dockercfg" "github.com/docker/docker/api/types/image" + "github.com/docker/docker/api/types/registry" "github.com/docker/docker/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/testcontainers/testcontainers-go/internal/core" "github.com/testcontainers/testcontainers-go/wait" ) @@ -198,7 +199,7 @@ func TestBuildContainerFromDockerfile(t *testing.T) { WaitingFor: wait.ForLog("Ready to accept connections"), } - redisC, err := prepareRedisImage(ctx, req, t) + redisC, err := prepareRedisImage(ctx, req) require.NoError(t, err) terminateContainerOnEnd(t, ctx, redisC) } @@ -249,7 +250,7 @@ func TestBuildContainerFromDockerfileWithDockerAuthConfig(t *testing.T) { WaitingFor: wait.ForLog("Ready to accept connections"), } - redisC, err := prepareRedisImage(ctx, req, t) + redisC, err := prepareRedisImage(ctx, req) require.NoError(t, err) terminateContainerOnEnd(t, ctx, redisC) } @@ -281,7 +282,7 @@ func TestBuildContainerFromDockerfileShouldFailWithWrongDockerAuthConfig(t *test WaitingFor: wait.ForLog("Ready to accept connections"), } - redisC, err := prepareRedisImage(ctx, req, t) + redisC, err := prepareRedisImage(ctx, req) require.Error(t, err) terminateContainerOnEnd(t, ctx, redisC) } @@ -369,7 +370,7 @@ func prepareLocalRegistryWithAuth(t *testing.T) string { return mp } -func prepareRedisImage(ctx context.Context, req ContainerRequest, t *testing.T) (Container, error) { +func prepareRedisImage(ctx context.Context, req ContainerRequest) (Container, error) { genContainerReq := GenericContainerRequest{ ProviderType: providerType, ContainerRequest: req, @@ -380,3 +381,32 @@ func prepareRedisImage(ctx context.Context, req ContainerRequest, t *testing.T) return redisC, err } + +//go:embed testdata/.docker/config.json +var dockerConfig string + +func Test_getDockerAuthConfigs(t *testing.T) { + t.Run("file", func(t *testing.T) { + got, err := getDockerAuthConfigs() + require.NoError(t, err) + require.NotNil(t, got) + }) + + t.Run("env", func(t *testing.T) { + t.Setenv("DOCKER_AUTH_CONFIG", dockerConfig) + + got, err := getDockerAuthConfigs() + require.NoError(t, err) + + // We can only check the keys as the values are not deterministic. + expected := map[string]registry.AuthConfig{ + "https://index.docker.io/v1/": {}, + "https://example.com": {}, + "https://my.private.registry": {}, + } + for k := range got { + got[k] = registry.AuthConfig{} + } + require.Equal(t, expected, got) + }) +} diff --git a/internal/core/images.go b/internal/core/images.go index 6c7213f12c..2892267e9c 100644 --- a/internal/core/images.go +++ b/internal/core/images.go @@ -2,6 +2,7 @@ package core import ( "bufio" + "io" "net/url" "os" "regexp" @@ -25,17 +26,22 @@ const ( var rxURL = regexp.MustCompile(URL) +// ExtractImagesFromDockerfile extracts images from the Dockerfile sourced from dockerfile. func ExtractImagesFromDockerfile(dockerfile string, buildArgs map[string]*string) ([]string, error) { - var images []string - file, err := os.Open(dockerfile) if err != nil { return nil, err } defer file.Close() + return ExtractImagesFromReader(file, buildArgs) +} + +// ExtractImagesFromReader extracts images from the Dockerfile sourced from r. +func ExtractImagesFromReader(r io.Reader, buildArgs map[string]*string) ([]string, error) { + var images []string var lines []string - scanner := bufio.NewScanner(file) + scanner := bufio.NewScanner(r) for scanner.Scan() { lines = append(lines, scanner.Text()) }