From 5024e266f5e30a9c27e288d2deda5f75c8137dcd Mon Sep 17 00:00:00 2001
From: Steven Hartland <stevenmhartland@gmail.com>
Date: Fri, 16 Aug 2024 07:27:06 +0100
Subject: [PATCH] fix!: docker authentication setup (#2727)

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     |  39 ++++++++--
 internal/core/images.go |  12 +++-
 6 files changed, 262 insertions(+), 55 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 5650f88612..5f6c415627 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..5b3e1e1067 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,6 +10,7 @@ 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"
@@ -198,7 +200,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 +251,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 +283,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 +371,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 +382,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())
 	}