From 6c1b3d8ecea7f2e4f86ace6ab7368bf9d6201cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 2 Oct 2024 16:30:59 +0200 Subject: [PATCH 01/50] feat: support reading Docker context --- internal/core/docker_context.go | 329 +++++++++++++++++++++++++++ internal/core/docker_context_test.go | 55 +++++ internal/core/docker_host.go | 26 ++- internal/core/docker_host_test.go | 29 +++ 4 files changed, 432 insertions(+), 7 deletions(-) create mode 100644 internal/core/docker_context.go create mode 100644 internal/core/docker_context_test.go diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go new file mode 100644 index 0000000000..ed2943d205 --- /dev/null +++ b/internal/core/docker_context.go @@ -0,0 +1,329 @@ +package core + +// The code in this file has been extracted from https://github.com/docker/cli, +// more especifically from https://github.com/docker/cli/blob/master/cli/context/store/metadatastore.go +// with the goal of not consuming the CLI package and all its dependencies. + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "os/user" + "path/filepath" + "reflect" + "runtime" + + "github.com/cpuguy83/dockercfg" + "github.com/docker/docker/client" + "github.com/docker/docker/errdefs" +) + +const ( + // defaultContextName is the name reserved for the default context (config & env based) + defaultContextName = "default" + // envOverrideContext is the name of the environment variable that can be + // used to override the context to use. If set, it overrides the context + // that's set in the CLI's configuration file, but takes no effect if the + // "DOCKER_HOST" env-var is set (which takes precedence. + envOverrideContext = "DOCKER_CONTEXT" + // envOverrideConfigDir is the name of the environment variable that can be + // used to override the location of the client configuration files (~/.docker). + // + // It takes priority over the default, but can be overridden by the "--config" + // command line option. + envOverrideConfigDir = "DOCKER_CONFIG" + // configFileName is the name of the client configuration file inside the + // config-directory. + configFileName = "config.json" + configFileDir = ".docker" + contextsDir = "contexts" + metadataDir = "meta" + metaFile = "meta.json" + // DockerEndpoint is the name of the docker endpoint in a stored context + dockerEndpoint string = "docker" +) + +// dockerContext is a typed representation of what we put in Context metadata +type dockerContext struct { + Description string + AdditionalFields map[string]any +} + +type metadataStore struct { + root string + config contextConfig +} + +// typeGetter is a func used to determine the concrete type of a context or +// endpoint metadata by returning a pointer to an instance of the object +// eg: for a context of type DockerContext, the corresponding typeGetter should return new(DockerContext) +type typeGetter func() any + +// namedTypeGetter is a typeGetter associated with a name +type namedTypeGetter struct { + name string + typeGetter typeGetter +} + +// endpointTypeGetter returns a namedTypeGetter with the specified name and getter +func endpointTypeGetter(name string, getter typeGetter) namedTypeGetter { + return namedTypeGetter{ + name: name, + typeGetter: getter, + } +} + +// endpointMeta contains fields we expect to be common for most context endpoints +type endpointMeta struct { + Host string `json:",omitempty"` + SkipTLSVerify bool +} + +var defaultStoreEndpoints = []namedTypeGetter{ + endpointTypeGetter(dockerEndpoint, func() any { return &endpointMeta{} }), +} + +// contextConfig is used to configure the metadata marshaler of the context ContextStore +type contextConfig struct { + contextType typeGetter + endpointTypes map[string]typeGetter +} + +// newConfig creates a config object +func newConfig(contextType typeGetter, endpoints ...namedTypeGetter) contextConfig { + res := contextConfig{ + contextType: contextType, + endpointTypes: make(map[string]typeGetter), + } + + for _, e := range endpoints { + res.endpointTypes[e.name] = e.typeGetter + } + return res +} + +// metadata contains metadata about a context and its endpoints +type metadata struct { + Name string `json:",omitempty"` + Metadata any `json:",omitempty"` + Endpoints map[string]any `json:",omitempty"` +} + +func (s *metadataStore) contextDir(id contextdir) string { + return filepath.Join(s.root, string(id)) +} + +type untypedContextMetadata struct { + Metadata json.RawMessage `json:"metadata,omitempty"` + Endpoints map[string]json.RawMessage `json:"endpoints,omitempty"` + Name string `json:"name,omitempty"` +} + +func (s *metadataStore) getByID(id contextdir) (metadata, error) { + fileName := filepath.Join(s.contextDir(id), metaFile) + bytes, err := os.ReadFile(fileName) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return metadata{}, errdefs.NotFound(fmt.Errorf("context not found: %w", err)) + } + return metadata{}, err + } + + var untyped untypedContextMetadata + r := metadata{ + Endpoints: make(map[string]any), + } + + if err := json.Unmarshal(bytes, &untyped); err != nil { + return metadata{}, fmt.Errorf("parsing %s: %w", fileName, err) + } + + r.Name = untyped.Name + if r.Metadata, err = parseTypedOrMap(untyped.Metadata, s.config.contextType); err != nil { + return metadata{}, fmt.Errorf("parsing %s: %w", fileName, err) + } + + for k, v := range untyped.Endpoints { + if r.Endpoints[k], err = parseTypedOrMap(v, s.config.endpointTypes[k]); err != nil { + return metadata{}, fmt.Errorf("parsing %s: %w", fileName, err) + } + } + + return r, err +} + +// list returns a list of all Docker contexts +func (s *metadataStore) list() ([]metadata, error) { + ctxDirs, err := listRecursivelyMetadataDirs(s.root) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, err + } + + res := make([]metadata, 0, len(ctxDirs)) + for _, dir := range ctxDirs { + c, err := s.getByID(contextdir(dir)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + continue + } + return nil, fmt.Errorf("read metadata: %w", err) + } + res = append(res, c) + } + + return res, nil +} + +type contextdir string + +func isContextDir(path string) bool { + s, err := os.Stat(filepath.Join(path, metaFile)) + if err != nil { + return false + } + + return !s.IsDir() +} + +func listRecursivelyMetadataDirs(root string) ([]string, error) { + fis, err := os.ReadDir(root) + if err != nil { + return nil, err + } + + var result []string + for _, fi := range fis { + if fi.IsDir() { + if isContextDir(filepath.Join(root, fi.Name())) { + result = append(result, fi.Name()) + } + + subs, err := listRecursivelyMetadataDirs(filepath.Join(root, fi.Name())) + if err != nil { + return nil, err + } + + for _, s := range subs { + result = append(result, filepath.Join(fi.Name(), s)) + } + } + } + + return result, nil +} + +func parseTypedOrMap(payload []byte, getter typeGetter) (any, error) { + if len(payload) == 0 || string(payload) == "null" { + return nil, nil + } + + if getter == nil { + var res map[string]any + if err := json.Unmarshal(payload, &res); err != nil { + return nil, err + } + return res, nil + } + + typed := getter() + if err := json.Unmarshal(payload, typed); err != nil { + return nil, err + } + + return reflect.ValueOf(typed).Elem().Interface(), nil +} + +// getHomeDir returns the home directory of the current user with the help of +// environment variables depending on the target operating system. +// Returned path should be used with "path/filepath" to form new paths. +// +// On non-Windows platforms, it falls back to nss lookups, if the home +// directory cannot be obtained from environment-variables. +// +// If linking statically with cgo enabled against glibc, ensure the +// osusergo build tag is used. +// +// If needing to do nss lookups, do not disable cgo or set osusergo. +// +// getHomeDir is a copy of [pkg/homedir.Get] to prevent adding docker/docker +// as dependency for consumers that only need to read the config-file. +// +// [pkg/homedir.Get]: https://pkg.go.dev/github.com/docker/docker@v26.1.4+incompatible/pkg/homedir#Get +func getHomeDir() string { + home, _ := os.UserHomeDir() + if home == "" && runtime.GOOS != "windows" { + if u, err := user.Current(); err == nil { + return u.HomeDir + } + } + return home +} + +// configurationDir returns the directory the configuration file is stored in +func configurationDir() string { + configDir := os.Getenv(envOverrideConfigDir) + if configDir == "" { + configDir = filepath.Join(getHomeDir(), configFileDir) + } + return configDir +} + +// GetDockerHostFromCurrentContext returns the Docker host from the current Docker context. +// For that, it traverses the directory structure of the Docker configuration directory, +// looking for the current context and its Docker endpoint. +func GetDockerHostFromCurrentContext() (string, error) { + metaRoot := filepath.Join(filepath.Join(configurationDir(), contextsDir), metadataDir) + + ms := &metadataStore{ + root: metaRoot, + config: newConfig(func() any { return &dockerContext{} }, defaultStoreEndpoints...), + } + + md, err := ms.list() + if err != nil { + return "", err + } + + currentContext := currentContext() + + for _, m := range md { + if m.Name == currentContext { + ep, ok := m.Endpoints[dockerEndpoint].(endpointMeta) + if ok { + return ep.Host, nil + } + } + } + + return "", fmt.Errorf("no Docker host found in the current context") +} + +// currentContext returns the current context name, based on +// environment variables and the cli configuration file. It does not +// validate if the given context exists or if it's valid; errors may +// occur when trying to use it. +func currentContext() string { + cfg, err := dockercfg.LoadDefaultConfig() + if err != nil { + return defaultContextName + } + + if os.Getenv(client.EnvOverrideHost) != "" { + return defaultContextName + } + + if ctxName := os.Getenv(envOverrideContext); ctxName != "" { + return ctxName + } + + if cfg.CurrentContext != "" { + // We don't validate if this context exists: errors may occur when trying to use it. + return cfg.CurrentContext + } + + return defaultContextName +} diff --git a/internal/core/docker_context_test.go b/internal/core/docker_context_test.go new file mode 100644 index 0000000000..221128533e --- /dev/null +++ b/internal/core/docker_context_test.go @@ -0,0 +1,55 @@ +package core + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// setupDockerContexts creates a temporary directory structure for testing the Docker context functions. +// It creates the following structure, where $i is the index of the context, starting from 1: +// - $HOME/.docker +// - config.json +// - contexts +// - meta +// - context$i +// - meta.json +// +// The config.json file contains the current context, and the meta.json files contain the metadata for each context. +// It generates the specified number of contexts, setting the current context to the one specified by currentContextIndex. +func setupDockerContexts(t *testing.T, currentContextIndex int, contextsCount int) { + configDir := filepath.Join(getHomeDir(), configFileDir) + + err := createTmpDir(configDir) + require.NoError(t, err) + + configJson := filepath.Join(configDir, "config.json") + + const baseContext = "context" + + configBytes := fmt.Sprintf(`{ + "currentContext": "%s%d" +}`, baseContext, currentContextIndex) + + err = os.WriteFile(configJson, []byte(configBytes), 0o644) + require.NoError(t, err) + + metaDir := filepath.Join(configDir, contextsDir, metadataDir) + + err = createTmpDir(metaDir) + require.NoError(t, err) + + // first index is 1 + for i := 1; i <= contextsCount; i++ { + contextDir := filepath.Join(metaDir, fmt.Sprintf("context%d", i)) + err = createTmpDir(contextDir) + require.NoError(t, err) + + context := fmt.Sprintf(`{"Name":"%s%d","Metadata":{"Description":"Testcontainers Go %d"},"Endpoints":{"docker":{"Host":"tcp://127.0.0.1:%d","SkipTLSVerify":false}}}`, baseContext, i, i, i) + err = os.WriteFile(filepath.Join(contextDir, "meta.json"), []byte(context), 0o644) + require.NoError(t, err) + } +} diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 3088a3742b..c35962d3d3 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -19,13 +19,14 @@ type dockerHostContext string var DockerHostContextKey = dockerHostContext("docker_host") var ( - ErrDockerHostNotSet = errors.New("DOCKER_HOST is not set") - ErrDockerSocketOverrideNotSet = errors.New("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE is not set") - ErrDockerSocketNotSetInContext = errors.New("socket not set in context") - ErrDockerSocketNotSetInProperties = errors.New("socket not set in ~/.testcontainers.properties") - ErrNoUnixSchema = errors.New("URL schema is not unix") - ErrSocketNotFound = errors.New("socket not found") - ErrSocketNotFoundInPath = errors.New("docker socket not found in " + DockerSocketPath) + ErrDockerHostNotSet = errors.New("DOCKER_HOST is not set") + ErrDockerSocketOverrideNotSet = errors.New("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE is not set") + ErrDockerSocketNotSetInContext = errors.New("socket not set in Go context") + ErrDockerSocketNotSetInDockerContext = errors.New("socket not set in Docker context") + ErrDockerSocketNotSetInProperties = errors.New("socket not set in ~/.testcontainers.properties") + ErrNoUnixSchema = errors.New("URL schema is not unix") + ErrSocketNotFound = errors.New("socket not found") + ErrSocketNotFoundInPath = errors.New("docker socket not found in " + DockerSocketPath) // ErrTestcontainersHostNotSetInProperties this error is specific to Testcontainers ErrTestcontainersHostNotSetInProperties = errors.New("tc.host not set in ~/.testcontainers.properties") ) @@ -125,6 +126,7 @@ func extractDockerHost(ctx context.Context) (string, error) { testcontainersHostFromProperties, dockerHostFromEnv, dockerHostFromContext, + dockerHostFromDockerContext, dockerSocketPath, dockerHostFromProperties, rootlessDockerSocketPath, @@ -262,6 +264,16 @@ func dockerHostFromContext(ctx context.Context) (string, error) { return "", ErrDockerSocketNotSetInContext } +// dockerHostFromContext returns the docker host from the Go context, if it's not empty +func dockerHostFromDockerContext(ctx context.Context) (string, error) { + dockerHost, err := GetDockerHostFromCurrentContext() + if err == nil { + return dockerHost, nil + } + + return "", ErrDockerSocketNotSetInDockerContext +} + // dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty func dockerHostFromProperties(ctx context.Context) (string, error) { cfg := config.Read() diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index eceee573fc..cb8647813b 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -154,6 +154,21 @@ func TestExtractDockerHost(t *testing.T) { require.Equal(t, DockerSocketSchema+"/this/is/a/sample.sock", host) }) + t.Run("Docker Host from Docker context", func(tt *testing.T) { + // do not mess with local .testcontainers.properties + tmpDir := tt.TempDir() + tt.Setenv("HOME", tmpDir) + tt.Setenv("USERPROFILE", tmpDir) // Windows support + setupDockerSocketNotFound(tt) + setupRootlessNotFound(tt) + setupTestcontainersProperties(tt, "") + setupDockerContexts(tt, 2, 3) // current context is context2 + + host, err := extractDockerHost(context.Background()) + require.NoError(t, err) + require.Equal(t, "tcp://127.0.0.1:2", host) // from context2 + }) + t.Run("Default Docker socket", func(t *testing.T) { setupRootlessNotFound(t) tmpSocket := setupDockerSocket(t) @@ -299,6 +314,20 @@ func TestExtractDockerHost(t *testing.T) { require.ErrorIs(t, err, ErrSocketNotFoundInPath) assert.Empty(t, socket) }) + + t.Run("extract-from-docker-context/not-found", func(tt *testing.T) { + host, err := dockerHostFromDockerContext(context.Background()) + require.ErrorIs(tt, err, ErrDockerSocketNotSetInDockerContext) + assert.Empty(tt, host) + }) + + t.Run("extract-from-docker-context/found", func(tt *testing.T) { + setupDockerContexts(tt, 2, 3) // current context is context2 + + host, err := dockerHostFromDockerContext(context.Background()) + require.NoError(tt, err) + assert.Equal(tt, "tcp://127.0.0.1:2", host) + }) }) } From 415c896eb807e78ebbc105f4270269ee6fb1f911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 2 Oct 2024 16:35:46 +0200 Subject: [PATCH 02/50] chore: move read Docker config to the core --- docker_auth.go | 24 +---- docker_auth_test.go | 80 ---------------- generic.go | 2 +- internal/core/docker_config.go | 31 ++++++ internal/core/docker_config_test.go | 96 +++++++++++++++++++ internal/core/testdata/.docker/config.json | 8 ++ .../invalid-config/.docker/config.json | 3 + 7 files changed, 140 insertions(+), 104 deletions(-) create mode 100644 internal/core/docker_config.go create mode 100644 internal/core/docker_config_test.go create mode 100644 internal/core/testdata/.docker/config.json create mode 100644 internal/core/testdata/invalid-config/.docker/config.json diff --git a/docker_auth.go b/docker_auth.go index af0d415de9..7298d8dee2 100644 --- a/docker_auth.go +++ b/docker_auth.go @@ -150,7 +150,7 @@ func configKey(cfg *dockercfg.Config) (string, error) { // getDockerAuthConfigs returns a map with the auth configs from the docker config file // using the registry as the key func getDockerAuthConfigs() (map[string]registry.AuthConfig, error) { - cfg, err := getDockerConfig() + cfg, err := core.ReadDockerConfig() if err != nil { if errors.Is(err, os.ErrNotExist) { return map[string]registry.AuthConfig{}, nil @@ -246,25 +246,3 @@ func getDockerAuthConfigs() (map[string]registry.AuthConfig, error) { 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 -// 2. the DOCKER_CONFIG environment variable, as the path to the config file -// 3. else it will load the default config file, which is ~/.docker/config.json -func getDockerConfig() (*dockercfg.Config, error) { - if env := os.Getenv("DOCKER_AUTH_CONFIG"); env != "" { - var cfg dockercfg.Config - if err := json.Unmarshal([]byte(env), &cfg); err != nil { - return nil, fmt.Errorf("unmarshal DOCKER_AUTH_CONFIG: %w", err) - } - - return &cfg, nil - } - - cfg, err := dockercfg.LoadDefaultConfig() - if err != nil { - return nil, fmt.Errorf("load default config: %w", err) - } - - return &cfg, nil -} diff --git a/docker_auth_test.go b/docker_auth_test.go index b8580a08ea..83fcd50020 100644 --- a/docker_auth_test.go +++ b/docker_auth_test.go @@ -16,82 +16,11 @@ import ( "github.com/docker/docker/client" "github.com/stretchr/testify/require" - "github.com/testcontainers/testcontainers-go/internal/core" "github.com/testcontainers/testcontainers-go/wait" ) const exampleAuth = "https://example-auth.com" -func Test_getDockerConfig(t *testing.T) { - expectedConfig := &dockercfg.Config{ - AuthConfigs: map[string]dockercfg.AuthConfig{ - core.IndexDockerIO: {}, - "https://example.com": {}, - "https://my.private.registry": {}, - }, - CredentialsStore: "desktop", - } - t.Run("HOME/valid", func(t *testing.T) { - testDockerConfigHome(t, "testdata") - - cfg, err := getDockerConfig() - require.NoError(t, err) - require.Equal(t, expectedConfig, cfg) - }) - - t.Run("HOME/not-found", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "not-found") - - cfg, err := getDockerConfig() - require.ErrorIs(t, err, os.ErrNotExist) - require.Nil(t, cfg) - }) - - t.Run("HOME/invalid-config", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "invalid-config") - - cfg, err := getDockerConfig() - require.ErrorContains(t, err, "json: cannot unmarshal array") - require.Nil(t, cfg) - }) - - t.Run("DOCKER_AUTH_CONFIG/valid", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "not-found") - t.Setenv("DOCKER_AUTH_CONFIG", dockerConfig) - - cfg, err := getDockerConfig() - require.NoError(t, err) - require.Equal(t, expectedConfig, cfg) - }) - - t.Run("DOCKER_AUTH_CONFIG/invalid-config", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "not-found") - t.Setenv("DOCKER_AUTH_CONFIG", `{"auths": []}`) - - cfg, err := getDockerConfig() - require.ErrorContains(t, err, "json: cannot unmarshal array") - require.Nil(t, cfg) - }) - - t.Run("DOCKER_CONFIG/valid", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "not-found") - t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", ".docker")) - - cfg, err := getDockerConfig() - require.NoError(t, err) - require.Equal(t, expectedConfig, cfg) - }) - - t.Run("DOCKER_CONFIG/invalid-config", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "not-found") - t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", "invalid-config", ".docker")) - - cfg, err := getDockerConfig() - require.ErrorContains(t, err, "json: cannot unmarshal array") - require.Nil(t, cfg) - }) -} - func TestDockerImageAuth(t *testing.T) { t.Run("retrieve auth with DOCKER_AUTH_CONFIG env var", func(t *testing.T) { username, password := "gopher", "secret" @@ -421,15 +350,6 @@ func Test_getDockerAuthConfigs(t *testing.T) { requireValidAuthConfig(t) }) - - t.Run("DOCKER_CONFIG/invalid-config", func(t *testing.T) { - testDockerConfigHome(t, "testdata", "not-found") - t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", "invalid-config", ".docker")) - - cfg, err := getDockerConfig() - require.ErrorContains(t, err, "json: cannot unmarshal array") - require.Nil(t, cfg) - }) } // requireValidAuthConfig checks that the given authConfigs map contains the expected keys. diff --git a/generic.go b/generic.go index 9052287b51..b6ba454d0c 100644 --- a/generic.go +++ b/generic.go @@ -78,7 +78,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain // TODO: Remove this debugging. if strings.Contains(err.Error(), "toomanyrequests") { // Debugging information for rate limiting. - cfg, err := getDockerConfig() + cfg, err := core.ReadDockerConfig() if err == nil { fmt.Printf("XXX: too many requests: %+v", cfg) } diff --git a/internal/core/docker_config.go b/internal/core/docker_config.go new file mode 100644 index 0000000000..b04bc8c14b --- /dev/null +++ b/internal/core/docker_config.go @@ -0,0 +1,31 @@ +package core + +import ( + "encoding/json" + "fmt" + "os" + + "github.com/cpuguy83/dockercfg" +) + +// ReadDockerConfig 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 +// 2. the DOCKER_CONFIG environment variable, as the path to the config file +// 3. else it will load the default config file, which is ~/.docker/config.json +func ReadDockerConfig() (*dockercfg.Config, error) { + if env := os.Getenv("DOCKER_AUTH_CONFIG"); env != "" { + var cfg dockercfg.Config + if err := json.Unmarshal([]byte(env), &cfg); err != nil { + return nil, fmt.Errorf("unmarshal DOCKER_AUTH_CONFIG: %w", err) + } + + return &cfg, nil + } + + cfg, err := dockercfg.LoadDefaultConfig() + if err != nil { + return nil, fmt.Errorf("load default config: %w", err) + } + + return &cfg, nil +} diff --git a/internal/core/docker_config_test.go b/internal/core/docker_config_test.go new file mode 100644 index 0000000000..79507f8506 --- /dev/null +++ b/internal/core/docker_config_test.go @@ -0,0 +1,96 @@ +package core + +import ( + _ "embed" + "os" + "path/filepath" + "testing" + + "github.com/cpuguy83/dockercfg" + "github.com/stretchr/testify/require" +) + +//go:embed testdata/.docker/config.json +var dockerConfig string + +func TestReadDockerConfig(t *testing.T) { + expectedConfig := &dockercfg.Config{ + AuthConfigs: map[string]dockercfg.AuthConfig{ + IndexDockerIO: {}, + "https://example.com": {}, + "https://my.private.registry": {}, + }, + CredentialsStore: "desktop", + } + t.Run("HOME/valid", func(t *testing.T) { + testDockerConfigHome(t, "testdata") + + cfg, err := ReadDockerConfig() + require.NoError(t, err) + require.Equal(t, expectedConfig, cfg) + }) + + t.Run("HOME/not-found", func(t *testing.T) { + testDockerConfigHome(t, "testdata", "not-found") + + cfg, err := ReadDockerConfig() + require.ErrorIs(t, err, os.ErrNotExist) + require.Nil(t, cfg) + }) + + t.Run("HOME/invalid-config", func(t *testing.T) { + testDockerConfigHome(t, "testdata", "invalid-config") + + cfg, err := ReadDockerConfig() + require.ErrorContains(t, err, "json: cannot unmarshal array") + require.Nil(t, cfg) + }) + + t.Run("DOCKER_AUTH_CONFIG/valid", func(t *testing.T) { + testDockerConfigHome(t, "testdata", "not-found") + t.Setenv("DOCKER_AUTH_CONFIG", dockerConfig) + + cfg, err := ReadDockerConfig() + require.NoError(t, err) + require.Equal(t, expectedConfig, cfg) + }) + + t.Run("DOCKER_AUTH_CONFIG/invalid-config", func(t *testing.T) { + testDockerConfigHome(t, "testdata", "not-found") + t.Setenv("DOCKER_AUTH_CONFIG", `{"auths": []}`) + + cfg, err := ReadDockerConfig() + require.ErrorContains(t, err, "json: cannot unmarshal array") + require.Nil(t, cfg) + }) + + t.Run("DOCKER_CONFIG/valid", func(t *testing.T) { + testDockerConfigHome(t, "testdata", "not-found") + t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", ".docker")) + + cfg, err := ReadDockerConfig() + require.NoError(t, err) + require.Equal(t, expectedConfig, cfg) + }) + + t.Run("DOCKER_CONFIG/invalid-config", func(t *testing.T) { + testDockerConfigHome(t, "testdata", "not-found") + t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", "invalid-config", ".docker")) + + cfg, err := ReadDockerConfig() + require.ErrorContains(t, err, "json: cannot unmarshal array") + require.Nil(t, cfg) + }) +} + +// testDockerConfigHome sets the user's home directory to the given path +// and unsets the DOCKER_CONFIG and DOCKER_AUTH_CONFIG environment variables. +func testDockerConfigHome(t *testing.T, dirs ...string) { + t.Helper() + + dir := filepath.Join(dirs...) + t.Setenv("DOCKER_AUTH_CONFIG", "") + t.Setenv("DOCKER_CONFIG", "") + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) // Windows +} diff --git a/internal/core/testdata/.docker/config.json b/internal/core/testdata/.docker/config.json new file mode 100644 index 0000000000..af4b84ef1c --- /dev/null +++ b/internal/core/testdata/.docker/config.json @@ -0,0 +1,8 @@ +{ + "auths": { + "https://index.docker.io/v1/": {}, + "https://example.com": {}, + "https://my.private.registry": {} + }, + "credsStore": "desktop" +} \ No newline at end of file diff --git a/internal/core/testdata/invalid-config/.docker/config.json b/internal/core/testdata/invalid-config/.docker/config.json new file mode 100644 index 0000000000..f0f444f355 --- /dev/null +++ b/internal/core/testdata/invalid-config/.docker/config.json @@ -0,0 +1,3 @@ +{ + "auths": [] +} From 1e620f14365a41dbc6b81639e4f8cff5686ad633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 2 Oct 2024 16:36:34 +0200 Subject: [PATCH 03/50] chore: read docker config more consistently --- internal/core/docker_context.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index ed2943d205..f40a847250 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -14,7 +14,6 @@ import ( "reflect" "runtime" - "github.com/cpuguy83/dockercfg" "github.com/docker/docker/client" "github.com/docker/docker/errdefs" ) @@ -307,7 +306,7 @@ func GetDockerHostFromCurrentContext() (string, error) { // validate if the given context exists or if it's valid; errors may // occur when trying to use it. func currentContext() string { - cfg, err := dockercfg.LoadDefaultConfig() + cfg, err := ReadDockerConfig() if err != nil { return defaultContextName } From ea44c92577aa9c95fddc2d5b7dcad3eec409e3b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 2 Oct 2024 17:15:43 +0200 Subject: [PATCH 04/50] chore: extract to a parse function to detect remote docker hosts --- internal/core/docker_host.go | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index c35962d3d3..83730e28da 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -171,33 +171,35 @@ func extractDockerSocket(ctx context.Context) string { return extractDockerSocketFromClient(ctx, cli) } +// parseDockerSocket satinitises the docker socket path, removing the TCP schema if present, +// or the unix schema if present, returning the path without the schema. +func parseDockerSocket(socket string) string { + // this use case will cover the case when the docker host is a tcp socket + if strings.HasPrefix(socket, TCPSchema) { + return DockerSocketPath + } + + if strings.HasPrefix(socket, DockerSocketSchema) { + return strings.Replace(socket, DockerSocketSchema, "", 1) + } + + return socket +} + // extractDockerSocketFromClient Extracts the docker socket from the different alternatives, without caching the result, // and receiving an instance of the Docker API client interface. // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. // It panics if the Docker Info call errors, or the Docker host is not discovered. func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { - // check that the socket is not a tcp or unix socket - checkDockerSocketFn := func(socket string) string { - // this use case will cover the case when the docker host is a tcp socket - if strings.HasPrefix(socket, TCPSchema) { - return DockerSocketPath - } - - if strings.HasPrefix(socket, DockerSocketSchema) { - return strings.Replace(socket, DockerSocketSchema, "", 1) - } - - return socket - } tcHost, err := testcontainersHostFromProperties(ctx) if err == nil { - return checkDockerSocketFn(tcHost) + return parseDockerSocket(tcHost) } testcontainersDockerSocket, err := dockerSocketOverridePath() if err == nil { - return checkDockerSocketFn(testcontainersDockerSocket) + return parseDockerSocket(testcontainersDockerSocket) } info, err := cli.Info(ctx) @@ -219,7 +221,7 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st panic(err) // Docker host is required to get the Docker socket } - return checkDockerSocketFn(dockerHost) + return parseDockerSocket(dockerHost) } // isHostNotSet returns true if the error is related to the Docker host From 549b1cbfc63c8cac1c76c643e6e32aa05a6ab060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 2 Oct 2024 17:15:56 +0200 Subject: [PATCH 05/50] docs: add Docker context to docs --- docs/features/configuration.md | 10 ++++++---- internal/core/docker_host.go | 11 ++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs/features/configuration.md b/docs/features/configuration.md index ee5b6a4d69..3dc271c4f0 100644 --- a/docs/features/configuration.md +++ b/docs/features/configuration.md @@ -75,17 +75,19 @@ See [Docker environment variables](https://docs.docker.com/engine/reference/comm 3. Read the Go context for the **DOCKER_HOST** key. E.g. `ctx.Value("DOCKER_HOST")`. This is used internally for the library to pass the Docker host to the resource reaper. -4. Read the default Docker socket path, without the unix schema. E.g. `/var/run/docker.sock` +4. Read the host endpoint for the current Docker context in the Docker configuration file. E.g. `~/.docker/config.json`. -5. Read the **docker.host** property in the `~/.testcontainers.properties` file. E.g. `docker.host=tcp://my.docker.host:1234` +5. Read the default Docker socket path, without the unix schema. E.g. `/var/run/docker.sock` -6. Read the rootless Docker socket path, checking in the following alternative locations: +6. Read the **docker.host** property in the `~/.testcontainers.properties` file. E.g. `docker.host=tcp://my.docker.host:1234` + +7. Read the rootless Docker socket path, checking in the following alternative locations: 1. `${XDG_RUNTIME_DIR}/.docker/run/docker.sock`. 2. `${HOME}/.docker/run/docker.sock`. 3. `${HOME}/.docker/desktop/docker.sock`. 4. `/run/user/${UID}/docker.sock`, where `${UID}` is the user ID of the current user. -7. The library panics if none of the above are set, meaning that the Docker host was not detected. +8. The library panics if none of the above are set, meaning that the Docker host was not detected. ## Docker socket path detection diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 83730e28da..e4b4ae303c 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -80,11 +80,12 @@ var dockerHostCheck = func(ctx context.Context, host string) error { // // 1. Docker host from the "tc.host" property in the ~/.testcontainers.properties file. // 2. DOCKER_HOST environment variable. -// 3. Docker host from context. -// 4. Docker host from the default docker socket path, without the unix schema. -// 5. Docker host from the "docker.host" property in the ~/.testcontainers.properties file. -// 6. Rootless docker socket path. -// 7. Else, because the Docker host is not set, it panics. +// 3. Docker host from Go context. +// 4. Docker host from the current Docker context, without the unix schema. +// 5. Docker host from the default docker socket path, without the unix schema. +// 6. Docker host from the "docker.host" property in the ~/.testcontainers.properties file. +// 7. Rootless docker socket path. +// 8. Else, because the Docker host is not set, it panics. func MustExtractDockerHost(ctx context.Context) string { dockerHostOnce.Do(func() { cache, err := extractDockerHost(ctx) From fbefbdf6b53d75779d277acfa09b63cc14142b5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 2 Oct 2024 17:30:24 +0200 Subject: [PATCH 06/50] fix: lint --- internal/core/docker_host.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index e4b4ae303c..4d5fa38b87 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -192,7 +192,6 @@ func parseDockerSocket(socket string) string { // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. // It panics if the Docker Info call errors, or the Docker host is not discovered. func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { - tcHost, err := testcontainersHostFromProperties(ctx) if err == nil { return parseDockerSocket(tcHost) From 1ca3f63d804c0564412cd885dea88510f5036544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 13:12:07 +0200 Subject: [PATCH 07/50] chore: simplify container provider initialisation No need to have podman-specific code, as it will be read from the docker context --- docker.go | 3 +- docker_auth_test.go | 2 - docker_test.go | 85 +++++------------------------------------ from_dockerfile_test.go | 1 - generic_test.go | 4 -- lifecycle_test.go | 1 - logconsumer_test.go | 3 -- provider.go | 30 +++------------ provider_test.go | 81 --------------------------------------- reaper_test.go | 5 --- 10 files changed, 16 insertions(+), 199 deletions(-) delete mode 100644 provider_test.go diff --git a/docker.go b/docker.go index 9319c630dd..ec1edb9988 100644 --- a/docker.go +++ b/docker.go @@ -43,8 +43,7 @@ import ( var _ Container = (*DockerContainer)(nil) const ( - Bridge = "bridge" // Bridge network name (as well as driver) - Podman = "podman" + Bridge = "bridge" // Bridge network driver and name ReaperDefault = "reaper_default" // Default network name when bridge is not available packagePath = "github.com/testcontainers/testcontainers-go" ) diff --git a/docker_auth_test.go b/docker_auth_test.go index 83fcd50020..97638cceb6 100644 --- a/docker_auth_test.go +++ b/docker_auth_test.go @@ -220,7 +220,6 @@ func prepareLocalRegistryWithAuth(t *testing.T) string { // } genContainerReq := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, } @@ -245,7 +244,6 @@ func prepareLocalRegistryWithAuth(t *testing.T) string { func prepareRedisImage(ctx context.Context, req ContainerRequest) (Container, error) { genContainerReq := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, } diff --git a/docker_test.go b/docker_test.go index bbbe519c28..054fb130a9 100644 --- a/docker_test.go +++ b/docker_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/wait" ) @@ -38,14 +39,6 @@ const ( daemonMaxVersion = "1.41" ) -var providerType = ProviderDocker - -func init() { - if strings.Contains(os.Getenv("DOCKER_HOST"), "podman.sock") { - providerType = ProviderPodman - } -} - func TestContainerWithHostNetworkOptions(t *testing.T) { if os.Getenv("XDG_RUNTIME_DIR") != "" { t.Skip("Skipping test that requires host network access when running in a container") @@ -60,7 +53,6 @@ func TestContainerWithHostNetworkOptions(t *testing.T) { } gcr := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, Files: []ContainerFile{ @@ -145,7 +137,6 @@ func TestContainerWithNetworkModeAndNetworkTogether(t *testing.T) { // } gcr := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxImage, Networks: []string{"new-network"}, @@ -178,7 +169,6 @@ func TestContainerWithHostNetwork(t *testing.T) { } gcr := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, WaitingFor: wait.ForHTTP("/").WithPort(nginxHighPort), @@ -224,7 +214,6 @@ func TestContainerWithHostNetwork(t *testing.T) { func TestContainerReturnItsContainerID(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -253,7 +242,6 @@ func TestContainerTerminationResetsState(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -280,7 +268,6 @@ func TestContainerTerminationResetsState(t *testing.T) { func TestContainerStateAfterTermination(t *testing.T) { createContainerFn := func(ctx context.Context) (Container, error) { return GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -340,7 +327,6 @@ func TestContainerTerminationRemovesDockerImage(t *testing.T) { defer dockerClient.Close() ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -377,7 +363,6 @@ func TestContainerTerminationRemovesDockerImage(t *testing.T) { WaitingFor: wait.ForLog("Ready to accept connections"), } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -407,7 +392,6 @@ func TestContainerTerminationRemovesDockerImage(t *testing.T) { func TestTwoContainersExposingTheSamePort(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -421,7 +405,6 @@ func TestTwoContainersExposingTheSamePort(t *testing.T) { require.NoError(t, err) nginxB, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -467,7 +450,6 @@ func TestContainerCreation(t *testing.T) { ctx := context.Background() nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -516,11 +498,15 @@ func TestContainerCreation(t *testing.T) { func TestContainerCreationWithName(t *testing.T) { ctx := context.Background() + cfg := config.Read() + + // use the user-defined bridge network, as it could have been changed by the user + bridge := cfg.TestcontainersBridgeName + creationName := fmt.Sprintf("%s_%d", "test_container", time.Now().Unix()) expectedName := "/" + creationName // inspect adds '/' in the beginning nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -528,7 +514,7 @@ func TestContainerCreationWithName(t *testing.T) { }, WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort), Name: creationName, - Networks: []string{"bridge"}, + Networks: []string{bridge}, }, Started: true, }) @@ -552,15 +538,8 @@ func TestContainerCreationWithName(t *testing.T) { t.Errorf("Expected networks 1. Got '%d'.", len(networks)) } network := networks[0] - switch providerType { - case ProviderDocker: - if network != Bridge { - t.Errorf("Expected network name '%s'. Got '%s'.", Bridge, network) - } - case ProviderPodman: - if network != Podman { - t.Errorf("Expected network name '%s'. Got '%s'.", Podman, network) - } + if network != bridge { + t.Errorf("Expected network name '%s'. Got '%s'.", bridge, network) } endpoint, err := nginxC.PortEndpoint(ctx, nginxDefaultPort, "http") @@ -582,7 +561,6 @@ func TestContainerCreationAndWaitForListeningPortLongEnough(t *testing.T) { // delayed-nginx will wait 2s before opening port nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxDelayedImage, ExposedPorts: []string{ @@ -614,7 +592,6 @@ func TestContainerCreationTimesOut(t *testing.T) { ctx := context.Background() // delayed-nginx will wait 2s before opening port nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxDelayedImage, ExposedPorts: []string{ @@ -635,7 +612,6 @@ func TestContainerRespondsWithHttp200ForIndex(t *testing.T) { ctx := context.Background() nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -667,7 +643,6 @@ func TestContainerCreationTimesOutWithHttp(t *testing.T) { ctx := context.Background() // delayed-nginx will wait 2s before opening port nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxDelayedImage, ExposedPorts: []string{ @@ -693,7 +668,6 @@ func TestContainerCreationWaitsForLogContextTimeout(t *testing.T) { WaitingFor: wait.ForLog("test context timeout").WithStartupTimeout(1 * time.Second), } c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -715,7 +689,6 @@ func TestContainerCreationWaitsForLog(t *testing.T) { WaitingFor: wait.ForLog("port: 3306 MySQL Community Server - GPL"), } mysqlC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -742,7 +715,6 @@ func Test_BuildContainerFromDockerfileWithBuildArgs(t *testing.T) { // } genContainerReq := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, } @@ -787,7 +759,6 @@ func Test_BuildContainerFromDockerfileWithBuildLog(t *testing.T) { // } genContainerReq := GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, } @@ -824,7 +795,6 @@ func TestContainerCreationWaitsForLogAndPortContextTimeout(t *testing.T) { ), } c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -844,7 +814,6 @@ func TestContainerCreationWaitingForHostPort(t *testing.T) { } // } nginx, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -860,7 +829,6 @@ func TestContainerCreationWaitingForHostPortWithoutBashThrowsAnError(t *testing. WaitingFor: wait.ForListeningPort(nginxDefaultPort), } nginx, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -886,7 +854,6 @@ func TestCMD(t *testing.T) { } c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -912,7 +879,6 @@ func TestEntrypoint(t *testing.T) { } c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -939,7 +905,6 @@ func TestWorkingDir(t *testing.T) { } c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -1145,7 +1110,6 @@ func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) { // Create the container that writes into the mounted volume. bashC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: "docker.io/bash", Files: []ContainerFile{ @@ -1173,7 +1137,6 @@ func TestContainerWithTmpFs(t *testing.T) { } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -1240,7 +1203,6 @@ func TestContainerNonExistentImage(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: "docker.io/postgres:12", WaitingFor: wait.ForLog("log"), @@ -1255,16 +1217,12 @@ func TestContainerNonExistentImage(t *testing.T) { } func TestContainerCustomPlatformImage(t *testing.T) { - if providerType == ProviderPodman { - t.Skip("Incompatible Docker API version for Podman") - } t.Run("error with a non-existent platform", func(t *testing.T) { t.Parallel() nonExistentPlatform := "windows/arm12" ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: "docker.io/redis:latest", ImagePlatform: nonExistentPlatform, @@ -1280,7 +1238,6 @@ func TestContainerCustomPlatformImage(t *testing.T) { ctx := context.Background() c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: "docker.io/mysql:8.0.36", ImagePlatform: "linux/amd64", @@ -1314,7 +1271,6 @@ func TestContainerWithCustomHostname(t *testing.T) { Hostname: hostname, } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -1379,7 +1335,6 @@ func TestDockerContainerCopyFileToContainer(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1406,7 +1361,6 @@ func TestDockerContainerCopyDirToContainer(t *testing.T) { ctx := context.Background() nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1591,7 +1545,6 @@ func TestDockerContainerCopyToContainer(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1629,7 +1582,6 @@ func TestDockerContainerCopyFileFromContainer(t *testing.T) { ctx := context.Background() nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1667,7 +1619,6 @@ func TestDockerContainerCopyEmptyFileFromContainer(t *testing.T) { ctx := context.Background() nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1702,9 +1653,6 @@ func TestDockerContainerCopyEmptyFileFromContainer(t *testing.T) { } func TestDockerContainerResources(t *testing.T) { - if providerType == ProviderPodman { - t.Skip("Rootless Podman does not support setting rlimit") - } if os.Getenv("XDG_RUNTIME_DIR") != "" { t.Skip("Rootless Docker does not support setting rlimit") } @@ -1725,7 +1673,6 @@ func TestDockerContainerResources(t *testing.T) { } nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1754,16 +1701,11 @@ func TestDockerContainerResources(t *testing.T) { } func TestContainerCapAdd(t *testing.T) { - if providerType == ProviderPodman { - t.Skip("Rootless Podman does not support setting cap-add/cap-drop") - } - ctx := context.Background() expected := "IPC_LOCK" nginx, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{nginxDefaultPort}, @@ -1822,7 +1764,6 @@ func TestContainerWithUserID(t *testing.T) { WaitingFor: wait.ForExit(), } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -1850,7 +1791,6 @@ func TestContainerWithNoUserID(t *testing.T) { WaitingFor: wait.ForExit(), } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) @@ -1873,7 +1813,7 @@ func TestContainerWithNoUserID(t *testing.T) { func TestGetGatewayIP(t *testing.T) { // When using docker compose with DinD mode, and using host port or http wait strategy // It's need to invoke GetGatewayIP for get the host - provider, err := providerType.GetProvider(WithLogger(TestLogger(t))) + provider, err := ProviderDocker.GetProvider(WithLogger(TestLogger(t))) if err != nil { t.Fatal(err) } @@ -1891,7 +1831,6 @@ func TestGetGatewayIP(t *testing.T) { func TestNetworkModeWithContainerReference(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, }, @@ -1902,7 +1841,6 @@ func TestNetworkModeWithContainerReference(t *testing.T) { networkMode := fmt.Sprintf("container:%v", nginxA.GetContainerID()) nginxB, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, HostConfigModifier: func(hc *container.HostConfig) { @@ -1967,7 +1905,6 @@ func TestDockerProviderFindContainerByName(t *testing.T) { defer provider.Close() c1, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Name: "test", Image: "nginx:1.17.6", @@ -1985,7 +1922,6 @@ func TestDockerProviderFindContainerByName(t *testing.T) { c1Name := c1Inspect.Name c2, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Name: "test2", Image: "nginx:1.17.6", @@ -2020,7 +1956,6 @@ func TestImageBuiltFromDockerfile_KeepBuiltImage(t *testing.T) { cli := provider.Client() // Create container. c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ FromDockerfile: FromDockerfile{ Context: "testdata", diff --git a/from_dockerfile_test.go b/from_dockerfile_test.go index f6f7512ae3..a800877822 100644 --- a/from_dockerfile_test.go +++ b/from_dockerfile_test.go @@ -103,7 +103,6 @@ func TestBuildImageFromDockerfile_BuildError(t *testing.T) { }, } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Started: true, }) diff --git a/generic_test.go b/generic_test.go index 99c13f5bfa..4a9a9a96b6 100644 --- a/generic_test.go +++ b/generic_test.go @@ -27,7 +27,6 @@ func TestGenericReusableContainer(t *testing.T) { reusableContainerName := reusableContainerName + "_" + time.Now().Format("20060102150405") n1, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{nginxDefaultPort}, @@ -84,7 +83,6 @@ func TestGenericReusableContainer(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { n2, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{nginxDefaultPort}, @@ -114,7 +112,6 @@ func TestGenericContainerShouldReturnRefOnError(t *testing.T) { defer cancel() c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, WaitingFor: wait.ForLog("this string should not be present in the logs"), @@ -184,7 +181,6 @@ func TestHelperContainerStarterProcess(t *testing.T) { ctx := context.Background() nginxC, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxDelayedImage, ExposedPorts: []string{nginxDefaultPort}, diff --git a/lifecycle_test.go b/lifecycle_test.go index 66c4ad8a8a..04da1df835 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -897,7 +897,6 @@ func TestPrintContainerLogsOnError(t *testing.T) { } ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: req, Logger: &arrayOfLinesLogger, Started: true, diff --git a/logconsumer_test.go b/logconsumer_test.go index 9f4b0b61f9..4cb826aced 100644 --- a/logconsumer_test.go +++ b/logconsumer_test.go @@ -240,9 +240,6 @@ func TestContainerLogWithErrClosed(t *testing.T) { config.Reset() }) - if providerType == ProviderPodman { - t.Skip("Docker-in-Docker does not work with rootless Podman") - } // First spin up a docker-in-docker container, then spin up an inner container within that dind container // Logs are being read from the inner container via the dind container's tcp port, which can be briefly // closed to test behaviour in connection-closed situations. diff --git a/provider.go b/provider.go index b5e5ffa997..eb97e9ee1a 100644 --- a/provider.go +++ b/provider.go @@ -2,10 +2,7 @@ package testcontainers import ( "context" - "errors" "fmt" - "os" - "strings" "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/internal/core" @@ -15,7 +12,6 @@ import ( const ( ProviderDefault ProviderType = iota // default will auto-detect provider from DOCKER_HOST environment variable ProviderDocker - ProviderPodman ) type ( @@ -103,28 +99,12 @@ func (t ProviderType) GetProvider(opts ...GenericProviderOption) (GenericProvide o.ApplyGenericTo(opt) } - pt := t - if pt == ProviderDefault && strings.Contains(os.Getenv("DOCKER_HOST"), "podman.sock") { - pt = ProviderPodman - } - - switch pt { - case ProviderDefault, ProviderDocker: - providerOptions := append(Generic2DockerOptions(opts...), WithDefaultBridgeNetwork(Bridge)) - provider, err := NewDockerProvider(providerOptions...) - if err != nil { - return nil, fmt.Errorf("%w, failed to create Docker provider", err) - } - return provider, nil - case ProviderPodman: - providerOptions := append(Generic2DockerOptions(opts...), WithDefaultBridgeNetwork(Podman)) - provider, err := NewDockerProvider(providerOptions...) - if err != nil { - return nil, fmt.Errorf("%w, failed to create Docker provider", err) - } - return provider, nil + providerOptions := append(Generic2DockerOptions(opts...), WithDefaultBridgeNetwork(config.Read().TestcontainersBridgeName)) + provider, err := NewDockerProvider(providerOptions...) + if err != nil { + return nil, fmt.Errorf("%w, failed to create Docker provider", err) } - return nil, errors.New("unknown provider") + return provider, nil } // NewDockerProvider creates a Docker provider with the EnvClient diff --git a/provider_test.go b/provider_test.go deleted file mode 100644 index 097c83a02c..0000000000 --- a/provider_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package testcontainers - -import ( - "context" - "testing" - - "github.com/testcontainers/testcontainers-go/internal/core" -) - -func TestProviderTypeGetProviderAutodetect(t *testing.T) { - dockerHost := core.MustExtractDockerHost(context.Background()) - const podmanSocket = "unix://$XDG_RUNTIME_DIR/podman/podman.sock" - - tests := []struct { - name string - tr ProviderType - DockerHost string - want string - wantErr bool - }{ - { - name: "default provider without podman.socket", - tr: ProviderDefault, - DockerHost: dockerHost, - want: Bridge, - }, - { - name: "default provider with podman.socket", - tr: ProviderDefault, - DockerHost: podmanSocket, - want: Podman, - }, - { - name: "docker provider without podman.socket", - tr: ProviderDocker, - DockerHost: dockerHost, - want: Bridge, - }, - { - // Explicitly setting Docker provider should not be overridden by auto-detect - name: "docker provider with podman.socket", - tr: ProviderDocker, - DockerHost: podmanSocket, - want: Bridge, - }, - { - name: "Podman provider without podman.socket", - tr: ProviderPodman, - DockerHost: dockerHost, - want: Podman, - }, - { - name: "Podman provider with podman.socket", - tr: ProviderPodman, - DockerHost: podmanSocket, - want: Podman, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.tr == ProviderPodman && core.IsWindows() { - t.Skip("Podman provider is not implemented for Windows") - } - - t.Setenv("DOCKER_HOST", tt.DockerHost) - - got, err := tt.tr.GetProvider() - if (err != nil) != tt.wantErr { - t.Errorf("ProviderType.GetProvider() error = %v, wantErr %v", err, tt.wantErr) - return - } - provider, ok := got.(*DockerProvider) - if !ok { - t.Fatalf("ProviderType.GetProvider() = %T, want %T", got, &DockerProvider{}) - } - if provider.defaultBridgeNetworkName != tt.want { - t.Errorf("ProviderType.GetProvider() = %v, want %v", provider.defaultBridgeNetworkName, tt.want) - } - }) - } -} diff --git a/reaper_test.go b/reaper_test.go index f421c2686d..e285600465 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -120,7 +120,6 @@ func TestContainerStartsWithoutTheReaper(t *testing.T) { ctx := context.Background() ctr, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -153,7 +152,6 @@ func TestContainerStartsWithTheReaper(t *testing.T) { ctx := context.Background() c, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -188,7 +186,6 @@ func TestContainerStopWithReaper(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -235,7 +232,6 @@ func TestContainerTerminationWithReaper(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ @@ -268,7 +264,6 @@ func TestContainerTerminationWithoutReaper(t *testing.T) { ctx := context.Background() nginxA, err := GenericContainer(ctx, GenericContainerRequest{ - ProviderType: providerType, ContainerRequest: ContainerRequest{ Image: nginxAlpineImage, ExposedPorts: []string{ From 7e1e4b5d47baa3e949de4eb9246676b89b564d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:15:24 +0200 Subject: [PATCH 08/50] feat: support configuring the bridge network name --- internal/config/config.go | 10 +++ internal/config/config_test.go | 152 +++++++++++++++++---------------- 2 files changed, 87 insertions(+), 75 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index a172fa3a16..f1dff643a8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -85,6 +85,11 @@ type Config struct { // // Environment variable: TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE TestcontainersHost string `properties:"tc.host,default="` + + // TestcontainersBridgeName is the name of the bridge network used by Testcontainers. + // + // Environment variable: TESTCONTAINERS_BRIDGE_NAME + TestcontainersBridgeName string `properties:"tc.bridge.name,default=bridge"` } // } @@ -141,6 +146,11 @@ func read() Config { config.RyukConnectionTimeout = timeout } + testcontainersBridgeName := os.Getenv("TESTCONTAINERS_BRIDGE_NAME") + if testcontainersBridgeName != "" { + config.TestcontainersBridgeName = testcontainersBridgeName + } + return config } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 319deb85b7..5d38b5bd44 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -6,7 +6,9 @@ import ( "testing" "time" + "dario.cat/mergo" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -24,6 +26,7 @@ func resetTestEnv(t *testing.T) { t.Setenv("TESTCONTAINERS_RYUK_VERBOSE", "") t.Setenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT", "") t.Setenv("TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT", "") + t.Setenv("TESTCONTAINERS_BRIDGE_NAME", "") } func TestReadConfig(t *testing.T) { @@ -126,15 +129,17 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("TESTCONTAINERS_RYUK_VERBOSE", "true") t.Setenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT", "13s") t.Setenv("TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT", "12s") + t.Setenv("TESTCONTAINERS_BRIDGE_NAME", "testbridge") config := read() expected := Config{ - HubImageNamePrefix: defaultHubPrefix, - RyukDisabled: true, - RyukPrivileged: true, - RyukVerbose: true, - RyukReconnectionTimeout: 13 * time.Second, - RyukConnectionTimeout: 12 * time.Second, + HubImageNamePrefix: defaultHubPrefix, + RyukDisabled: true, + RyukPrivileged: true, + RyukVerbose: true, + RyukReconnectionTimeout: 13 * time.Second, + RyukConnectionTimeout: 12 * time.Second, + TestcontainersBridgeName: "testbridge", } assert.Equal(t, expected, config) @@ -144,8 +149,9 @@ func TestReadTCConfig(t *testing.T) { defaultRyukConnectionTimeout := 60 * time.Second defaultRyukReconnectionTimeout := 10 * time.Second defaultConfig := Config{ - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukConnectionTimeout: defaultRyukConnectionTimeout, + RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + TestcontainersBridgeName: "bridge", } tests := []struct { @@ -159,9 +165,7 @@ func TestReadTCConfig(t *testing.T) { "docker.host = " + tcpDockerHost33293, map[string]string{}, Config{ - Host: tcpDockerHost33293, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + Host: tcpDockerHost33293, }, }, { @@ -171,9 +175,7 @@ func TestReadTCConfig(t *testing.T) { `, map[string]string{}, Config{ - Host: tcpDockerHost4711, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + Host: tcpDockerHost4711, }, }, { @@ -185,20 +187,15 @@ func TestReadTCConfig(t *testing.T) { `, map[string]string{}, Config{ - Host: tcpDockerHost1234, - TLSVerify: 1, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + Host: tcpDockerHost1234, + TLSVerify: 1, }, }, { "Empty file", "", map[string]string{}, - Config{ - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, - }, + Config{}, }, { "Non-valid properties are ignored", @@ -207,9 +204,7 @@ func TestReadTCConfig(t *testing.T) { `, map[string]string{}, Config{ - Host: tcpDockerHost1234, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + Host: tcpDockerHost1234, }, }, { @@ -217,9 +212,7 @@ func TestReadTCConfig(t *testing.T) { "docker.host=" + tcpDockerHost33293, map[string]string{}, Config{ - Host: tcpDockerHost33293, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + Host: tcpDockerHost33293, }, }, { @@ -236,10 +229,8 @@ func TestReadTCConfig(t *testing.T) { docker.cert.path=/tmp/certs`, map[string]string{}, Config{ - Host: tcpDockerHost1234, - CertPath: "/tmp/certs", - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + Host: tcpDockerHost1234, + CertPath: "/tmp/certs", }, }, { @@ -247,9 +238,7 @@ func TestReadTCConfig(t *testing.T) { `ryuk.disabled=true`, map[string]string{}, Config{ - RyukDisabled: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukDisabled: true, }, }, { @@ -257,9 +246,7 @@ func TestReadTCConfig(t *testing.T) { `ryuk.container.privileged=true`, map[string]string{}, Config{ - RyukPrivileged: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukPrivileged: true, }, }, { @@ -302,9 +289,7 @@ func TestReadTCConfig(t *testing.T) { `ryuk.verbose=true`, map[string]string{}, Config{ - RyukVerbose: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukVerbose: true, }, }, { @@ -314,9 +299,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_DISABLED": "true", }, Config{ - RyukDisabled: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukDisabled: true, }, }, { @@ -326,9 +309,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, Config{ - RyukPrivileged: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukPrivileged: true, }, }, { @@ -338,9 +319,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_DISABLED": "true", }, Config{ - RyukDisabled: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukDisabled: true, }, }, { @@ -350,9 +329,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_DISABLED": "true", }, Config{ - RyukDisabled: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukDisabled: true, }, }, { @@ -378,9 +355,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_VERBOSE": "true", }, Config{ - RyukVerbose: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukVerbose: true, }, }, { @@ -390,9 +365,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_VERBOSE": "true", }, Config{ - RyukVerbose: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukVerbose: true, }, }, { @@ -418,9 +391,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, Config{ - RyukPrivileged: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukPrivileged: true, }, }, { @@ -430,9 +401,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, Config{ - RyukPrivileged: true, - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + RyukPrivileged: true, }, }, { @@ -485,9 +454,7 @@ func TestReadTCConfig(t *testing.T) { `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, map[string]string{}, Config{ - HubImageNamePrefix: defaultHubPrefix + "/props/", - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + HubImageNamePrefix: defaultHubPrefix + "/props/", }, }, { @@ -497,9 +464,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX": defaultHubPrefix + "/env/", }, Config{ - HubImageNamePrefix: defaultHubPrefix + "/env/", - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + HubImageNamePrefix: defaultHubPrefix + "/env/", }, }, { @@ -509,9 +474,35 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX": defaultHubPrefix + "/env/", }, Config{ - HubImageNamePrefix: defaultHubPrefix + "/env/", - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, + HubImageNamePrefix: defaultHubPrefix + "/env/", + }, + }, + { + "bridge-name/property", + `tc.bridge.name=props`, + map[string]string{}, + Config{ + TestcontainersBridgeName: "props", + }, + }, + { + "bridge-name/env-var", + ``, + map[string]string{ + "TESTCONTAINERS_BRIDGE_NAME": "env", + }, + Config{ + TestcontainersBridgeName: "env", + }, + }, + { + "bridge-name/env-var-wins", + `tc.bridge.name=props`, + map[string]string{ + "TESTCONTAINERS_BRIDGE_NAME": "env", + }, + Config{ + TestcontainersBridgeName: "env", }, }, } @@ -531,7 +522,18 @@ func TestReadTCConfig(t *testing.T) { // config := read() - assert.Equal(t, tt.expected, config, "Configuration doesn't not match") + // Merge the returned config, and the expected one, with the default config + // to avoid setting all the fields in the expected config. + // In the case of decoding errors in the properties file, the read config + // needs to be merged with the default config to avoid setting the fields + // that are not set in the properties file. + err := mergo.Merge(&config, defaultConfig) + require.NoError(t, err) + + err = mergo.Merge(&tt.expected, defaultConfig) + require.NoError(t, err) + + require.Equal(t, tt.expected, config, "Configuration doesn't not match") }) } }) From 2decbe3bb91f963f90680a792eaca883696eac0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:16:26 +0200 Subject: [PATCH 09/50] chore: rename function --- docker_auth.go | 2 +- generic.go | 2 +- internal/core/docker_config.go | 4 ++-- internal/core/docker_config_test.go | 14 +++++++------- internal/core/docker_context.go | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/docker_auth.go b/docker_auth.go index 7298d8dee2..8188943984 100644 --- a/docker_auth.go +++ b/docker_auth.go @@ -150,7 +150,7 @@ func configKey(cfg *dockercfg.Config) (string, error) { // getDockerAuthConfigs returns a map with the auth configs from the docker config file // using the registry as the key func getDockerAuthConfigs() (map[string]registry.AuthConfig, error) { - cfg, err := core.ReadDockerConfig() + cfg, err := core.DockerConfig() if err != nil { if errors.Is(err, os.ErrNotExist) { return map[string]registry.AuthConfig{}, nil diff --git a/generic.go b/generic.go index b6ba454d0c..9222ab584c 100644 --- a/generic.go +++ b/generic.go @@ -78,7 +78,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain // TODO: Remove this debugging. if strings.Contains(err.Error(), "toomanyrequests") { // Debugging information for rate limiting. - cfg, err := core.ReadDockerConfig() + cfg, err := core.DockerConfig() if err == nil { fmt.Printf("XXX: too many requests: %+v", cfg) } diff --git a/internal/core/docker_config.go b/internal/core/docker_config.go index b04bc8c14b..2b352701d2 100644 --- a/internal/core/docker_config.go +++ b/internal/core/docker_config.go @@ -8,11 +8,11 @@ import ( "github.com/cpuguy83/dockercfg" ) -// ReadDockerConfig returns the docker config file. It will internally check, in this particular order: +// DockerConfig 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 // 2. the DOCKER_CONFIG environment variable, as the path to the config file // 3. else it will load the default config file, which is ~/.docker/config.json -func ReadDockerConfig() (*dockercfg.Config, error) { +func DockerConfig() (*dockercfg.Config, error) { if env := os.Getenv("DOCKER_AUTH_CONFIG"); env != "" { var cfg dockercfg.Config if err := json.Unmarshal([]byte(env), &cfg); err != nil { diff --git a/internal/core/docker_config_test.go b/internal/core/docker_config_test.go index 79507f8506..b8aba2b54f 100644 --- a/internal/core/docker_config_test.go +++ b/internal/core/docker_config_test.go @@ -25,7 +25,7 @@ func TestReadDockerConfig(t *testing.T) { t.Run("HOME/valid", func(t *testing.T) { testDockerConfigHome(t, "testdata") - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.NoError(t, err) require.Equal(t, expectedConfig, cfg) }) @@ -33,7 +33,7 @@ func TestReadDockerConfig(t *testing.T) { t.Run("HOME/not-found", func(t *testing.T) { testDockerConfigHome(t, "testdata", "not-found") - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.ErrorIs(t, err, os.ErrNotExist) require.Nil(t, cfg) }) @@ -41,7 +41,7 @@ func TestReadDockerConfig(t *testing.T) { t.Run("HOME/invalid-config", func(t *testing.T) { testDockerConfigHome(t, "testdata", "invalid-config") - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.ErrorContains(t, err, "json: cannot unmarshal array") require.Nil(t, cfg) }) @@ -50,7 +50,7 @@ func TestReadDockerConfig(t *testing.T) { testDockerConfigHome(t, "testdata", "not-found") t.Setenv("DOCKER_AUTH_CONFIG", dockerConfig) - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.NoError(t, err) require.Equal(t, expectedConfig, cfg) }) @@ -59,7 +59,7 @@ func TestReadDockerConfig(t *testing.T) { testDockerConfigHome(t, "testdata", "not-found") t.Setenv("DOCKER_AUTH_CONFIG", `{"auths": []}`) - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.ErrorContains(t, err, "json: cannot unmarshal array") require.Nil(t, cfg) }) @@ -68,7 +68,7 @@ func TestReadDockerConfig(t *testing.T) { testDockerConfigHome(t, "testdata", "not-found") t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", ".docker")) - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.NoError(t, err) require.Equal(t, expectedConfig, cfg) }) @@ -77,7 +77,7 @@ func TestReadDockerConfig(t *testing.T) { testDockerConfigHome(t, "testdata", "not-found") t.Setenv("DOCKER_CONFIG", filepath.Join("testdata", "invalid-config", ".docker")) - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() require.ErrorContains(t, err, "json: cannot unmarshal array") require.Nil(t, cfg) }) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index f40a847250..1c2265d242 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -306,7 +306,7 @@ func GetDockerHostFromCurrentContext() (string, error) { // validate if the given context exists or if it's valid; errors may // occur when trying to use it. func currentContext() string { - cfg, err := ReadDockerConfig() + cfg, err := DockerConfig() if err != nil { return defaultContextName } From c596e376b4b3c157096e9d30e4c56db1a78803d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:17:51 +0200 Subject: [PATCH 10/50] fix: remove wrong copy&paste --- internal/core/docker_context.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index 1c2265d242..9dc46c4654 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -29,8 +29,7 @@ const ( // envOverrideConfigDir is the name of the environment variable that can be // used to override the location of the client configuration files (~/.docker). // - // It takes priority over the default, but can be overridden by the "--config" - // command line option. + // It takes priority over the default. envOverrideConfigDir = "DOCKER_CONFIG" // configFileName is the name of the client configuration file inside the // config-directory. From ece6ea5e633ba11f2a305f8c128ea8099d694474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:18:30 +0200 Subject: [PATCH 11/50] chore: simplify --- internal/core/docker_context.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index 9dc46c4654..3567b2798c 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -265,8 +265,9 @@ func getHomeDir() string { func configurationDir() string { configDir := os.Getenv(envOverrideConfigDir) if configDir == "" { - configDir = filepath.Join(getHomeDir(), configFileDir) + return filepath.Join(getHomeDir(), configFileDir) } + return configDir } From a775c4b07b114634d4bb8d9c6b7e3b99dc0d4834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:20:36 +0200 Subject: [PATCH 12/50] chore: simplify error --- internal/core/docker_context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index 3567b2798c..ed4732d7f5 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -298,7 +298,7 @@ func GetDockerHostFromCurrentContext() (string, error) { } } - return "", fmt.Errorf("no Docker host found in the current context") + return "", errors.New("no Docker host found in the current context") } // currentContext returns the current context name, based on From 5ac2263281f1f4cbdd582c4629b50871e9ed3f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:25:03 +0200 Subject: [PATCH 13/50] chore: simple naming in test --- internal/core/docker_host_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index cb8647813b..4c655020b3 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -154,7 +154,7 @@ func TestExtractDockerHost(t *testing.T) { require.Equal(t, DockerSocketSchema+"/this/is/a/sample.sock", host) }) - t.Run("Docker Host from Docker context", func(tt *testing.T) { + t.Run("docker-context/docker-host", func(tt *testing.T) { // do not mess with local .testcontainers.properties tmpDir := tt.TempDir() tt.Setenv("HOME", tmpDir) From 932fce205d35026d5f08e5cd8e0d0f483a5b1c4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:25:14 +0200 Subject: [PATCH 14/50] fix: do not lose the original error --- internal/core/docker_host.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 4d5fa38b87..7ddc73896f 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -273,7 +273,7 @@ func dockerHostFromDockerContext(ctx context.Context) (string, error) { return dockerHost, nil } - return "", ErrDockerSocketNotSetInDockerContext + return "", errors.Join(ErrDockerSocketNotSetInDockerContext, err) } // dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty From 9c048f7bf2064d643c0c058394fedeeb8e55d57d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 3 Oct 2024 14:25:39 +0200 Subject: [PATCH 15/50] fix: line endings --- internal/core/testdata/.docker/config.json | 2 +- testdata/.docker/config.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/testdata/.docker/config.json b/internal/core/testdata/.docker/config.json index af4b84ef1c..5ce110622d 100644 --- a/internal/core/testdata/.docker/config.json +++ b/internal/core/testdata/.docker/config.json @@ -5,4 +5,4 @@ "https://my.private.registry": {} }, "credsStore": "desktop" -} \ No newline at end of file +} diff --git a/testdata/.docker/config.json b/testdata/.docker/config.json index af4b84ef1c..5ce110622d 100644 --- a/testdata/.docker/config.json +++ b/testdata/.docker/config.json @@ -5,4 +5,4 @@ "https://my.private.registry": {} }, "credsStore": "desktop" -} \ No newline at end of file +} From 77cdda6fa9b91d67487a16f72ce02dc99bda65b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 4 Oct 2024 12:27:21 +0200 Subject: [PATCH 16/50] fix: case insensitive regex Different container runtimes could produce "Step" or "STEP" --- docker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker_test.go b/docker_test.go index 054fb130a9..0d5f0cd792 100644 --- a/docker_test.go +++ b/docker_test.go @@ -775,7 +775,7 @@ func Test_BuildContainerFromDockerfileWithBuildLog(t *testing.T) { temp := strings.Split(string(out), "\n") - if !regexp.MustCompile(`^Step\s*1/\d+\s*:\s*FROM docker.io/alpine$`).MatchString(temp[0]) { + if !regexp.MustCompile(`^(?i:Step)\s*1/\d+\s*:\s*FROM docker.io/alpine$`).MatchString(temp[0]) { t.Errorf("Expected stdout first line to be %s. Got '%s'.", "Step 1/* : FROM docker.io/alpine", temp[0]) } } From 7eab89f622e71a4ab21ea8791d9849e3c06f027a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 4 Oct 2024 12:58:30 +0200 Subject: [PATCH 17/50] chore: deprecate reaper_network and always use the bridge one --- docker.go | 4 ++-- docker_test.go | 14 ++++++-------- provider.go | 2 +- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/docker.go b/docker.go index ec1edb9988..dd9dbfc224 100644 --- a/docker.go +++ b/docker.go @@ -44,7 +44,7 @@ var _ Container = (*DockerContainer)(nil) const ( Bridge = "bridge" // Bridge network driver and name - ReaperDefault = "reaper_default" // Default network name when bridge is not available + ReaperDefault = "reaper_default" // Deprecated: use Bridge instead. Default network name when bridge is not available packagePath = "github.com/testcontainers/testcontainers-go" ) @@ -1572,7 +1572,7 @@ func (p *DockerProvider) getDefaultNetwork(ctx context.Context, cli client.APICl return "", err } - reaperNetwork := ReaperDefault + reaperNetwork := Bridge reaperNetworkExists := false diff --git a/docker_test.go b/docker_test.go index 0d5f0cd792..11e260df70 100644 --- a/docker_test.go +++ b/docker_test.go @@ -498,11 +498,6 @@ func TestContainerCreation(t *testing.T) { func TestContainerCreationWithName(t *testing.T) { ctx := context.Background() - cfg := config.Read() - - // use the user-defined bridge network, as it could have been changed by the user - bridge := cfg.TestcontainersBridgeName - creationName := fmt.Sprintf("%s_%d", "test_container", time.Now().Unix()) expectedName := "/" + creationName // inspect adds '/' in the beginning @@ -514,7 +509,7 @@ func TestContainerCreationWithName(t *testing.T) { }, WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort), Name: creationName, - Networks: []string{bridge}, + Networks: []string{Bridge}, }, Started: true, }) @@ -537,9 +532,12 @@ func TestContainerCreationWithName(t *testing.T) { if len(networks) != 1 { t.Errorf("Expected networks 1. Got '%d'.", len(networks)) } + + expectedBridge := config.Read().TestcontainersBridgeName + network := networks[0] - if network != bridge { - t.Errorf("Expected network name '%s'. Got '%s'.", bridge, network) + if network != expectedBridge { + t.Errorf("Expected network name '%s'. Got '%s'.", expectedBridge, network) } endpoint, err := nginxC.PortEndpoint(ctx, nginxDefaultPort, "http") diff --git a/provider.go b/provider.go index eb97e9ee1a..f2fefdeb17 100644 --- a/provider.go +++ b/provider.go @@ -99,7 +99,7 @@ func (t ProviderType) GetProvider(opts ...GenericProviderOption) (GenericProvide o.ApplyGenericTo(opt) } - providerOptions := append(Generic2DockerOptions(opts...), WithDefaultBridgeNetwork(config.Read().TestcontainersBridgeName)) + providerOptions := append(Generic2DockerOptions(opts...), WithDefaultBridgeNetwork(Bridge)) provider, err := NewDockerProvider(providerOptions...) if err != nil { return nil, fmt.Errorf("%w, failed to create Docker provider", err) From 1e58328876fe9bb030fd8685b976a793936cecbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 4 Oct 2024 13:04:14 +0200 Subject: [PATCH 18/50] chore: support replacing the bridge network in the endpoint modifier --- lifecycle.go | 33 +++++++++++++++- lifecycle_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/lifecycle.go b/lifecycle.go index ff1472d043..0fc2b49d9e 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/docker/go-connections/nat" + "github.com/testcontainers/testcontainers-go/internal/config" ) // ContainerRequestHook is a hook that will be called before a container is created. @@ -464,7 +465,11 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain // prepare mounts hostConfig.Mounts = mapToDockerMounts(req.Mounts) - endpointSettings := map[string]*network.EndpointSettings{} + endpointSettings := networkingConfig.EndpointsConfig + if endpointSettings == nil { + // sanity check for nil map + endpointSettings = make(map[string]*network.EndpointSettings) + } // #248: Docker allows only one network to be specified during container creation // If there is more than one network specified in the request container should be attached to them @@ -503,6 +508,16 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain networkingConfig.EndpointsConfig = endpointSettings + // Move the bridge network to the user-defined bridge network, if configured. + // This is needed because different container runtimes might use different a bridge network name. + // As an example, when listing the networks, the Docker client always retrives the default + // bridge network as "bridge", but when attaching a container to that bridge network, + // the container runtime can fail because the bridge network is not named "bridge". + // For that reason we need to move the bridge network to the user-defined bridge network, + // that's why we are offering an extension point to configure the bridge network name + // at the Testcontainers configuration level. + bridgeNetworModifier(networkingConfig.EndpointsConfig) + exposedPorts := req.ExposedPorts // this check must be done after the pre-creation Modifiers are called, so the network mode is already set if len(exposedPorts) == 0 && !hostConfig.NetworkMode.IsContainer() { @@ -624,3 +639,19 @@ func defaultHostConfigModifier(req ContainerRequest) func(hostConfig *container. hostConfig.Resources = req.Resources } } + +// bridgeNetworModifier moves the bridge network to the user-defined bridge network, if configured. +func bridgeNetworModifier(endpointSettings map[string]*network.EndpointSettings) { + userDefinedBridge := config.Read().TestcontainersBridgeName + + if userDefinedBridge == Bridge { + return + } + + // If the map contains a bridge network, use the configured bridge network. + if _, ok := endpointSettings[Bridge]; ok { + nw := endpointSettings[Bridge] + delete(endpointSettings, Bridge) + endpointSettings[userDefinedBridge] = nw + } +} diff --git a/lifecycle_test.go b/lifecycle_test.go index 04da1df835..44c11a334d 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -4,6 +4,8 @@ import ( "bufio" "context" "fmt" + "os" + "path/filepath" "strings" "testing" "time" @@ -16,6 +18,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/wait" ) @@ -294,6 +297,102 @@ func TestPreCreateModifierHook(t *testing.T) { ) }) + t.Run("endpoint-settings-modifier/custom-bridge-network", func(t *testing.T) { + // set testcontainers properties including a custom bridge network name + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) // windows + + const bridgeNetworkName = "test-bridge" + content := "tc.bridge.name=" + bridgeNetworkName + if err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(content), 0o600); err != nil { + t.Errorf("Failed to create the file: %v", err) + return + } + config.Reset() // reset the config to reload the properties for testing purposes + + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{ + EndpointsConfig: map[string]*network.EndpointSettings{ + "bridge": { + MacAddress: "bridge-mac", + Aliases: []string{"bridge-alias"}, + }, + }, + } + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.NoError(t, err) + + // assertions + + require.Equal( + t, + "bridge-mac", + inputNetworkingConfig.EndpointsConfig[bridgeNetworkName].MacAddress, + ) + require.Equal( + t, + []string{"bridge-alias"}, + inputNetworkingConfig.EndpointsConfig[bridgeNetworkName].Aliases, + ) + }) + + t.Run("endpoint-settings-modifier/default-bridge-network", func(t *testing.T) { + // set testcontainers properties including a custom bridge network name + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) // windows + + if err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(""), 0o600); err != nil { + t.Errorf("Failed to create the file: %v", err) + return + } + config.Reset() // reset the config to reload the properties for testing purposes + + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{ + EndpointsConfig: map[string]*network.EndpointSettings{ + Bridge: { + MacAddress: "bridge-mac", + Aliases: []string{"bridge-alias"}, + }, + }, + } + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.NoError(t, err) + + // assertions + + require.Equal( + t, + "bridge-mac", + inputNetworkingConfig.EndpointsConfig[Bridge].MacAddress, + ) + require.Equal( + t, + []string{"bridge-alias"}, + inputNetworkingConfig.EndpointsConfig[Bridge].Aliases, + ) + }) + t.Run("Request contains exposed port modifiers without protocol", func(t *testing.T) { req := ContainerRequest{ Image: nginxAlpineImage, // alpine image does expose port 80 From 8abfeba53682891a4c65940fc5b19b4b4930fa07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 4 Oct 2024 13:11:20 +0200 Subject: [PATCH 19/50] fix: lint --- lifecycle.go | 1 + 1 file changed, 1 insertion(+) diff --git a/lifecycle.go b/lifecycle.go index 0fc2b49d9e..c49d8a60f0 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/docker/go-connections/nat" + "github.com/testcontainers/testcontainers-go/internal/config" ) From 203d76293c369988dbdbec114c85671a7cbb2699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 4 Oct 2024 16:05:23 +0200 Subject: [PATCH 20/50] chore: assume the container runtime uses the default network by default --- docker.go | 80 +------------------------------------------------- docker_test.go | 2 +- provider.go | 9 +++--- reaper.go | 5 ---- 4 files changed, 6 insertions(+), 90 deletions(-) diff --git a/docker.go b/docker.go index dd9dbfc224..a50b4ed92e 100644 --- a/docker.go +++ b/docker.go @@ -985,32 +985,6 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque // defer the close of the Docker client connection the soonest defer p.Close() - // Make sure that bridge network exists - // In case it is disabled we will create reaper_default network - if p.DefaultNetwork == "" { - p.DefaultNetwork, err = p.getDefaultNetwork(ctx, p.client) - if err != nil { - return nil, err - } - } - - // If default network is not bridge make sure it is attached to the request - // as container won't be attached to it automatically - // in case of Podman the bridge network is called 'podman' as 'bridge' would conflict - if p.DefaultNetwork != p.defaultBridgeNetworkName { - isAttached := false - for _, net := range req.Networks { - if net == p.DefaultNetwork { - isAttached = true - break - } - } - - if !isAttached { - req.Networks = append(req.Networks, p.DefaultNetwork) - } - } - imageName := req.Image env := []string{} @@ -1459,14 +1433,6 @@ func (p *DockerProvider) CreateNetwork(ctx context.Context, req NetworkRequest) // defer the close of the Docker client connection the soonest defer p.Close() - // Make sure that bridge network exists - // In case it is disabled we will create reaper_default network - if p.DefaultNetwork == "" { - if p.DefaultNetwork, err = p.getDefaultNetwork(ctx, p.client); err != nil { - return nil, err - } - } - if req.Labels == nil { req.Labels = make(map[string]string) } @@ -1538,15 +1504,7 @@ func (p *DockerProvider) GetNetwork(ctx context.Context, req NetworkRequest) (ne } func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) { - // Use a default network as defined in the DockerProvider - if p.DefaultNetwork == "" { - var err error - p.DefaultNetwork, err = p.getDefaultNetwork(ctx, p.client) - if err != nil { - return "", err - } - } - nw, err := p.GetNetwork(ctx, NetworkRequest{Name: p.DefaultNetwork}) + nw, err := p.GetNetwork(ctx, NetworkRequest{Name: config.Read().TestcontainersBridgeName}) if err != nil { return "", err } @@ -1565,42 +1523,6 @@ func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) { return ip, nil } -func (p *DockerProvider) getDefaultNetwork(ctx context.Context, cli client.APIClient) (string, error) { - // Get list of available networks - networkResources, err := cli.NetworkList(ctx, network.ListOptions{}) - if err != nil { - return "", err - } - - reaperNetwork := Bridge - - reaperNetworkExists := false - - for _, net := range networkResources { - if net.Name == p.defaultBridgeNetworkName { - return p.defaultBridgeNetworkName, nil - } - - if net.Name == reaperNetwork { - reaperNetworkExists = true - } - } - - // Create a bridge network for the container communications - if !reaperNetworkExists { - _, err = cli.NetworkCreate(ctx, reaperNetwork, network.CreateOptions{ - Driver: Bridge, - Attachable: true, - Labels: core.DefaultLabels(core.SessionID()), - }) - if err != nil { - return "", err - } - } - - return reaperNetwork, nil -} - // containerFromDockerResponse builds a Docker container struct from the response of the Docker API func containerFromDockerResponse(ctx context.Context, response types.Container) (*DockerContainer, error) { provider, err := NewDockerProvider() diff --git a/docker_test.go b/docker_test.go index 11e260df70..281d010bd9 100644 --- a/docker_test.go +++ b/docker_test.go @@ -509,7 +509,7 @@ func TestContainerCreationWithName(t *testing.T) { }, WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort), Name: creationName, - Networks: []string{Bridge}, + Networks: []string{config.Read().TestcontainersBridgeName}, }, Started: true, }) diff --git a/provider.go b/provider.go index f2fefdeb17..49c1aee7c7 100644 --- a/provider.go +++ b/provider.go @@ -21,7 +21,7 @@ type ( // GenericProviderOptions defines options applicable to all providers GenericProviderOptions struct { Logger Logging - DefaultNetwork string + DefaultNetwork string // Deprecated: it will be removed in the next major version } // GenericProviderOption defines a common interface to modify GenericProviderOptions @@ -35,7 +35,6 @@ type ( // DockerProviderOptions defines options applicable to DockerProvider DockerProviderOptions struct { - defaultBridgeNetworkName string *GenericProviderOptions } @@ -69,9 +68,10 @@ func Generic2DockerOptions(opts ...GenericProviderOption) []DockerProviderOption return converted } +// Deprecated: WithDefaultNetwork is deprecated and will be removed in the next major version func WithDefaultBridgeNetwork(bridgeNetworkName string) DockerProviderOption { return DockerProviderOptionFunc(func(opts *DockerProviderOptions) { - opts.defaultBridgeNetworkName = bridgeNetworkName + // NOOP }) } @@ -99,8 +99,7 @@ func (t ProviderType) GetProvider(opts ...GenericProviderOption) (GenericProvide o.ApplyGenericTo(opt) } - providerOptions := append(Generic2DockerOptions(opts...), WithDefaultBridgeNetwork(Bridge)) - provider, err := NewDockerProvider(providerOptions...) + provider, err := NewDockerProvider(Generic2DockerOptions(opts...)...) if err != nil { return nil, fmt.Errorf("%w, failed to create Docker provider", err) } diff --git a/reaper.go b/reaper.go index c41520b5b7..d0132fb809 100644 --- a/reaper.go +++ b/reaper.go @@ -272,11 +272,6 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider) ( req.Labels[core.LabelReaper] = "true" req.Labels[core.LabelRyuk] = "true" - // Attach reaper container to a requested network if it is specified - if p, ok := provider.(*DockerProvider); ok { - req.Networks = append(req.Networks, p.DefaultNetwork) - } - c, err := provider.RunContainer(ctx, req) if err != nil { // We need to check whether the error is caused by a container with the same name From 464bc9b28880d1cf0e2293554f481c536df36ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Sat, 5 Oct 2024 07:21:20 +0200 Subject: [PATCH 21/50] chore: rename variable to avoid shading package --- internal/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index f1dff643a8..f02c3f37ea 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -161,12 +161,12 @@ func read() Config { tcProp := filepath.Join(home, ".testcontainers.properties") // init from a file - properties, err := properties.LoadFile(tcProp, properties.UTF8) + props, err := properties.LoadFile(tcProp, properties.UTF8) if err != nil { return applyEnvironmentConfiguration(config) } - if err := properties.Decode(&config); err != nil { + if err := props.Decode(&config); err != nil { fmt.Printf("invalid testcontainers properties file, returning an empty Testcontainers configuration: %v\n", err) return applyEnvironmentConfiguration(config) } From 37a8142e1cc21caeccf1cff095e47a399d708a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Sat, 5 Oct 2024 07:31:30 +0200 Subject: [PATCH 22/50] fix: if the properties file does not exist, use default config --- internal/config/config.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index f02c3f37ea..505f43f6c5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -160,8 +160,9 @@ func read() Config { } tcProp := filepath.Join(home, ".testcontainers.properties") - // init from a file - props, err := properties.LoadFile(tcProp, properties.UTF8) + // Init from a file, ignore if it doesn't existm which is the case for most users. + // The properties library will return the default values for the struct. + props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) if err != nil { return applyEnvironmentConfiguration(config) } From 07003ffcfe80ee5b564dcbfbe80d87f9562581ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Sat, 5 Oct 2024 07:31:58 +0200 Subject: [PATCH 23/50] fix: typo --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 505f43f6c5..ef890fc298 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -160,7 +160,7 @@ func read() Config { } tcProp := filepath.Join(home, ".testcontainers.properties") - // Init from a file, ignore if it doesn't existm which is the case for most users. + // Init from a file, ignore if it doesn't exist, which is the case for most users. // The properties library will return the default values for the struct. props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) if err != nil { From adf0ba1731001cf2a93928ceee8cd1055c2a16b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 7 Oct 2024 14:18:48 +0200 Subject: [PATCH 24/50] fix: use default config in tests --- internal/config/config.go | 123 +++++++++++++++++---------------- internal/config/config_test.go | 28 ++++---- 2 files changed, 79 insertions(+), 72 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index ef890fc298..36f5e36991 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -94,66 +94,10 @@ type Config struct { // } -// Read reads from testcontainers properties file, if it exists -// it is possible that certain values get overridden when set as environment variables -func Read() Config { - tcConfigOnce.Do(func() { - tcConfig = read() - }) - - return tcConfig -} - -// Reset resets the singleton instance of the Config struct, -// allowing to read the configuration again. -// Handy for testing, so do not use it in production code -// This function is not thread-safe -func Reset() { - tcConfigOnce = new(sync.Once) -} - -func read() Config { +// defaultConfig +func defaultConfig() Config { config := Config{} - applyEnvironmentConfiguration := func(config Config) Config { - ryukDisabledEnv := os.Getenv("TESTCONTAINERS_RYUK_DISABLED") - if parseBool(ryukDisabledEnv) { - config.RyukDisabled = ryukDisabledEnv == "true" - } - - hubImageNamePrefix := os.Getenv("TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX") - if hubImageNamePrefix != "" { - config.HubImageNamePrefix = hubImageNamePrefix - } - - ryukPrivilegedEnv := os.Getenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED") - if parseBool(ryukPrivilegedEnv) { - config.RyukPrivileged = ryukPrivilegedEnv == "true" - } - - ryukVerboseEnv := os.Getenv("TESTCONTAINERS_RYUK_VERBOSE") - if parseBool(ryukVerboseEnv) { - config.RyukVerbose = ryukVerboseEnv == "true" - } - - ryukReconnectionTimeoutEnv := os.Getenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT") - if timeout, err := time.ParseDuration(ryukReconnectionTimeoutEnv); err == nil { - config.RyukReconnectionTimeout = timeout - } - - ryukConnectionTimeoutEnv := os.Getenv("TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT") - if timeout, err := time.ParseDuration(ryukConnectionTimeoutEnv); err == nil { - config.RyukConnectionTimeout = timeout - } - - testcontainersBridgeName := os.Getenv("TESTCONTAINERS_BRIDGE_NAME") - if testcontainersBridgeName != "" { - config.TestcontainersBridgeName = testcontainersBridgeName - } - - return config - } - home, err := os.UserHomeDir() if err != nil { return applyEnvironmentConfiguration(config) @@ -172,6 +116,69 @@ func read() Config { return applyEnvironmentConfiguration(config) } + return config +} + +func applyEnvironmentConfiguration(config Config) Config { + ryukDisabledEnv := os.Getenv("TESTCONTAINERS_RYUK_DISABLED") + if parseBool(ryukDisabledEnv) { + config.RyukDisabled = ryukDisabledEnv == "true" + } + + hubImageNamePrefix := os.Getenv("TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX") + if hubImageNamePrefix != "" { + config.HubImageNamePrefix = hubImageNamePrefix + } + + ryukPrivilegedEnv := os.Getenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED") + if parseBool(ryukPrivilegedEnv) { + config.RyukPrivileged = ryukPrivilegedEnv == "true" + } + + ryukVerboseEnv := os.Getenv("TESTCONTAINERS_RYUK_VERBOSE") + if parseBool(ryukVerboseEnv) { + config.RyukVerbose = ryukVerboseEnv == "true" + } + + ryukReconnectionTimeoutEnv := os.Getenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT") + if timeout, err := time.ParseDuration(ryukReconnectionTimeoutEnv); err == nil { + config.RyukReconnectionTimeout = timeout + } + + ryukConnectionTimeoutEnv := os.Getenv("TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT") + if timeout, err := time.ParseDuration(ryukConnectionTimeoutEnv); err == nil { + config.RyukConnectionTimeout = timeout + } + + testcontainersBridgeName := os.Getenv("TESTCONTAINERS_BRIDGE_NAME") + if testcontainersBridgeName != "" { + config.TestcontainersBridgeName = testcontainersBridgeName + } + + return config +} + +// Read reads from testcontainers properties file, if it exists +// it is possible that certain values get overridden when set as environment variables +func Read() Config { + tcConfigOnce.Do(func() { + tcConfig = read() + }) + + return tcConfig +} + +// Reset resets the singleton instance of the Config struct, +// allowing to read the configuration again. +// Handy for testing, so do not use it in production code +// This function is not thread-safe +func Reset() { + tcConfigOnce = new(sync.Once) +} + +func read() Config { + config := defaultConfig() + return applyEnvironmentConfiguration(config) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5d38b5bd44..2a683b8fd7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -102,7 +102,7 @@ func TestReadTCConfig(t *testing.T) { config := read() - expected := Config{} + expected := defaultConfig() assert.Equal(t, expected, config) }) @@ -114,7 +114,7 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("DOCKER_HOST", tcpDockerHost33293) config := read() - expected := Config{} // the config does not read DOCKER_HOST, that's why it's empty + expected := defaultConfig() // the config does not read DOCKER_HOST, that's why it's empty assert.Equal(t, expected, config) }) @@ -148,7 +148,7 @@ func TestReadTCConfig(t *testing.T) { t.Run("HOME contains TC properties file", func(t *testing.T) { defaultRyukConnectionTimeout := 60 * time.Second defaultRyukReconnectionTimeout := 10 * time.Second - defaultConfig := Config{ + defaultCfg := Config{ RyukConnectionTimeout: defaultRyukConnectionTimeout, RyukReconnectionTimeout: defaultRyukReconnectionTimeout, TestcontainersBridgeName: "bridge", @@ -219,7 +219,7 @@ func TestReadTCConfig(t *testing.T) { "Comments are ignored", `#docker.host=` + tcpDockerHost33293, map[string]string{}, - defaultConfig, + defaultCfg, }, { "Multiple docker host entries, last one wins, with TLS and cert path", @@ -338,7 +338,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "false", }, - defaultConfig, + defaultCfg, }, { "With Ryuk disabled using an env var and properties. Env var wins (3)", @@ -346,7 +346,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "false", }, - defaultConfig, + defaultCfg, }, { "With Ryuk verbose using an env var and properties. Env var wins (0)", @@ -374,7 +374,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_VERBOSE": "false", }, - defaultConfig, + defaultCfg, }, { "With Ryuk verbose using an env var and properties. Env var wins (3)", @@ -382,7 +382,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_VERBOSE": "false", }, - defaultConfig, + defaultCfg, }, { "With Ryuk container privileged using an env var and properties. Env var wins (0)", @@ -410,7 +410,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false", }, - defaultConfig, + defaultCfg, }, { "With Ryuk container privileged using an env var and properties. Env var wins (3)", @@ -418,7 +418,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false", }, - defaultConfig, + defaultCfg, }, { "With TLS verify using properties when value is wrong", @@ -439,7 +439,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "foo", }, - defaultConfig, + defaultCfg, }, { "With Ryuk container privileged using an env var and properties. Env var does not win because it's not a boolean value", @@ -447,7 +447,7 @@ func TestReadTCConfig(t *testing.T) { map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "foo", }, - defaultConfig, + defaultCfg, }, { "With Hub image name prefix set as a property", @@ -527,10 +527,10 @@ func TestReadTCConfig(t *testing.T) { // In the case of decoding errors in the properties file, the read config // needs to be merged with the default config to avoid setting the fields // that are not set in the properties file. - err := mergo.Merge(&config, defaultConfig) + err := mergo.Merge(&config, defaultCfg) require.NoError(t, err) - err = mergo.Merge(&tt.expected, defaultConfig) + err = mergo.Merge(&tt.expected, defaultCfg) require.NoError(t, err) require.Equal(t, tt.expected, config, "Configuration doesn't not match") From b31da78f7268a75fdd61c503a4f7e850f0ebad69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 14 Oct 2024 11:07:47 +0200 Subject: [PATCH 25/50] docs: simplify --- docs/features/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/configuration.md b/docs/features/configuration.md index 3dc271c4f0..d634f940cb 100644 --- a/docs/features/configuration.md +++ b/docs/features/configuration.md @@ -77,7 +77,7 @@ See [Docker environment variables](https://docs.docker.com/engine/reference/comm 4. Read the host endpoint for the current Docker context in the Docker configuration file. E.g. `~/.docker/config.json`. -5. Read the default Docker socket path, without the unix schema. E.g. `/var/run/docker.sock` +5. Assume the default Docker socket path as `/var/run/docker.sock`. 6. Read the **docker.host** property in the `~/.testcontainers.properties` file. E.g. `docker.host=tcp://my.docker.host:1234` From a38a34591248c2d0205eb1a6ef60ef1686f25520 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 14 Oct 2024 11:08:06 +0200 Subject: [PATCH 26/50] chore: simplify error when docker context is not found --- internal/core/docker_context.go | 2 +- internal/core/docker_host.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index ed4732d7f5..b8c691c16c 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -298,7 +298,7 @@ func GetDockerHostFromCurrentContext() (string, error) { } } - return "", errors.New("no Docker host found in the current context") + return "", ErrDockerSocketNotSetInDockerContext } // currentContext returns the current context name, based on diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 7ddc73896f..73214b3988 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -273,7 +273,7 @@ func dockerHostFromDockerContext(ctx context.Context) (string, error) { return dockerHost, nil } - return "", errors.Join(ErrDockerSocketNotSetInDockerContext, err) + return "", err } // dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty From 14aa3dd421db4beb5a517f3de7a02406916573e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 16 Oct 2024 12:34:17 +0200 Subject: [PATCH 27/50] fix: check if the docker socket is listening --- internal/core/docker_host.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 73214b3988..c641c6f848 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -194,12 +194,16 @@ func parseDockerSocket(socket string) string { func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { tcHost, err := testcontainersHostFromProperties(ctx) if err == nil { - return parseDockerSocket(tcHost) + if err = dockerHostCheck(ctx, tcHost); err == nil { + return parseDockerSocket(tcHost) + } } testcontainersDockerSocket, err := dockerSocketOverridePath() if err == nil { - return parseDockerSocket(testcontainersDockerSocket) + if err = dockerHostCheck(ctx, testcontainersDockerSocket); err == nil { + return parseDockerSocket(testcontainersDockerSocket) + } } info, err := cli.Info(ctx) From b327978962698f4abe722dbffee1d59f2b98da92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 16 Oct 2024 12:46:40 +0200 Subject: [PATCH 28/50] docs: document the docker context support --- docs/system_requirements/docker.md | 68 ++++++++++++++++++++ docs/system_requirements/rancher.md | 26 -------- docs/system_requirements/using_colima.md | 47 -------------- docs/system_requirements/using_podman.md | 81 ------------------------ mkdocs.yml | 3 - 5 files changed, 68 insertions(+), 157 deletions(-) delete mode 100644 docs/system_requirements/rancher.md delete mode 100644 docs/system_requirements/using_colima.md delete mode 100644 docs/system_requirements/using_podman.md diff --git a/docs/system_requirements/docker.md b/docs/system_requirements/docker.md index e791b748fb..7a6e8af27f 100644 --- a/docs/system_requirements/docker.md +++ b/docs/system_requirements/docker.md @@ -9,3 +9,71 @@ However, these are not actively tested in the main development workflow, so not If you have further questions about configuration details for your setup or whether it supports running Testcontainers-based tests, please contact the Testcontainers team and other users from the Testcontainers community on [Slack](https://slack.testcontainers.org/). + +## Using different container runtimes + +_Testcontainers for Go_ automatically detects the selected Docker context and use it to run the tests on that container runtime. You can check the selected context by running: + +```sh +docker context ls +NAME DESCRIPTION DOCKER ENDPOINT ERROR +colima colima unix:///Users/mdelapenya/.colima/default/docker.sock +default Current DOCKER_HOST based configuration unix:///var/run/docker.sock +desktop-linux * Docker Desktop unix:///Users/mdelapenya/.docker/run/docker.sock +orbstack OrbStack unix:///Users/mdelapenya/.orbstack/run/docker.sock +podman podman context unix:///var/folders/_j/nhbgdck523n3008dd3zlsm5m0000gn/T/podman/podman-machine-default-api.sock +tcd Testcontainers Desktop tcp://127.0.0.1:59908 +``` + +It is possible to use any container runtime to satisfy the system requirements instead of Docker, as long as it is 100% Docker-API compatible, and a Docker context is created for it. + +### Colima + +Colima creates its own Docker context when it is installed. This context is called `colima`. You can set this as the active context by running: + +```sh +docker context use colima +``` + +### Orbstack + +Orbstack creates its own Docker context when it is installed. This context is called `orbstack`. You can set this as the active context by running: + +```sh +docker context use orbstack +``` + +### Podman + +Podman does not create its own Docker context when it is installed so, after starting a `podman-machine`, please create it with the following command: + +```sh +docker context create podman --description "podman context" --docker "host=unix:///var/folders/_j/nhbgdck523n3008dd3zlsm5m0000gn/T/podman/podman-machine-default-api.sock" +``` + +!!! note + The UNIX socket path could be different in your machine. You can find it by running `podman machine inspect`. + +Then you can set this as the active context by running: + +```sh +docker context use podman +``` + +### Rancher Desktop + +Rancher Desktop creates its own Docker context when it is installed. This context is called `rancher-desktop`. You can set this as the active context by running: + +```sh +docker context use rancher-desktop +``` + +### Testcontainers Desktop + +Testcontainers Desktop creates its own Docker context when it is installed. This context is called `tcd`. You can set this as the active context by running: + +```sh +docker context use tcd +``` + +Testcontainers Desktop allows you to switch between different container runtimes, such as Docker, Podman, and Colima, by just using its simple GUI. You can also run the containers in the cloud, using Docker's Testcontainers Cloud. diff --git a/docs/system_requirements/rancher.md b/docs/system_requirements/rancher.md deleted file mode 100644 index 581b3a9cca..0000000000 --- a/docs/system_requirements/rancher.md +++ /dev/null @@ -1,26 +0,0 @@ -# Using Rancher Desktop - -It is possible to use Rancher Desktop to satisfy the system requirements instead of Docker. - -**IMPORTANT**: Please ensure you are running an up-to-date version of Rancher Desktop. There were some key fixes made in earlier versions (especially around v1.6). It is highly unlikely you will be able to get Rancher Desktop working with testcontainers if you are on an old version. - -The instructions below are written on the assumption that: - -1. you wish to run Rancher Desktop without administrative permissions (i.e. without granting `sudo` access a.k.a *"Administrative Access"* setting tickbox in Rancher Desktop is *unticked*). -2. you are running Rancher Desktop on an Apple-silicon device a.k.a M-series processor. - -Steps are as follows: - -1. In Rancher Desktop change engine from `containerd` to `dockerd (moby)`. -2. In Rancher Desktop set `VZ mode` networking. -3. On macOS CLI (e.g. `Terminal` app), set the following environment variables: - -```sh -export DOCKER_HOST=unix://$HOME/.rd/docker.sock -export TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE=/var/run/docker.sock -export TESTCONTAINERS_HOST_OVERRIDE=$(rdctl shell ip a show vznat | awk '/inet / {sub("/.*",""); print $2}') -``` - -As always, remember that environment variables are not persisted unless you add them to the relevant file for your default shell e.g. `~/.zshrc`. - -Credit: Thank you to @pdrosos on GitHub. diff --git a/docs/system_requirements/using_colima.md b/docs/system_requirements/using_colima.md deleted file mode 100644 index 6cb853adde..0000000000 --- a/docs/system_requirements/using_colima.md +++ /dev/null @@ -1,47 +0,0 @@ -# Using Colima with Docker - -[Colima](https://github.com/abiosoft/colima) is a container runtime which -integrates with Docker's tooling and can be configured in various ways. - -As of Colima v0.4.0 it's recommended to set the active Docker context to use -Colima. After the context is set _Testcontainers for Go_ will automatically be -configured to use Colima. - -```bash -$ docker context ls -NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR -colima colima unix:///Users/foobar/.colima/default/docker.sock -default * Current DOCKER_HOST based configuration unix:///Users/foobar/.colima/docker.sock - -$ docker context use colima -colima -Current context is now "colima" - -$ docker context ls -NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR -colima * colima unix:///Users/foobar/.colima/default/docker.sock -default Current DOCKER_HOST based configuration unix:///var/run/docker.sock -``` - -If you're using an older version of Colima or have other applications that are -unaware of Docker context the following workaround is available: - -- Locate your Docker Socket, see: [Colima's FAQ - Docker Socket Location](https://github.com/abiosoft/colima/blob/main/docs/FAQ.md#docker-socket-location) - -- Create a symbolic link from the default Docker Socket to the expected location, and restart Colima with the `--network-address` flag. - -``` - sudo ln -sf $HOME/.colima/default/docker.sock /var/run/docker.sock - colima stop - colima start --network-address -``` - -- Set the `DOCKER_HOST` environment variable to match the located Docker Socket - - * Example: `export DOCKER_HOST="unix://${HOME}/.colima/default/docker.sock"` - -- As of testcontainers-go v0.14.0 set `TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE` - to `/var/run/docker.sock` as the default value refers to your `DOCKER_HOST` - environment variable. - - * Example: `export TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE="/var/run/docker.sock"` diff --git a/docs/system_requirements/using_podman.md b/docs/system_requirements/using_podman.md deleted file mode 100644 index cf65792bbd..0000000000 --- a/docs/system_requirements/using_podman.md +++ /dev/null @@ -1,81 +0,0 @@ -# Using Podman instead of Docker - -_Testcontainers for Go_ supports the use of Podman (rootless or rootful) instead of Docker. - -In most scenarios no special setup is required in _Testcontainers for Go_. -_Testcontainers for Go_ will automatically discover the socket based on the `DOCKER_HOST` environment variables. -Alternatively you can configure the host with a `.testcontainers.properties` file. -The discovered Docker host is taken into account when starting a reaper container. -The discovered socket is used to detect the use of Podman. - -By default _Testcontainers for Go_ takes advantage of the default network settings both Docker and Podman are applying to newly created containers. -It only intervenes in scenarios where a `ContainerRequest` specifies networks and does not include the default network of the current container provider. -Unfortunately the default network for Docker is called _bridge_ where the default network in Podman is called _podman_. - -In complex container network scenarios it may be required to explicitly make use of the `ProviderPodman` like so: - -```go - -package some_test - -import ( - "testing" - tc "github.com/testcontainers/testcontainers-go" -) - -func TestSomething(t *testing.T) { - req := tc.GenericContainerRequest{ - ProviderType: tc.ProviderPodman, - ContainerRequest: tc.ContainerRequest{ - Image: "docker.io/nginx:alpine" - }, - } - - // ... -} -``` - -The `ProviderPodman` configures the `DockerProvider` with the correct default network for Podman to ensure complex network scenarios are working as with Docker. - -## Podman socket activation - -The reaper container needs to connect to the docker daemon to reap containers, so the podman socket service must be started: -```shell -> systemctl --user start podman.socket -``` - -## Fedora - -`DOCKER_HOST` environment variable must be set - -``` -> export DOCKER_HOST=unix://$XDG_RUNTIME_DIR/podman/podman.sock -``` - -SELinux may require a custom policy be applied to allow the reaper container to connect to and write to a socket. Once you experience the se-linux error, you can run the following commands to create and install a custom policy. - -``` -> sudo ausearch -c 'app' --raw | audit2allow -M my-podman -> sudo semodule -i my-podman.pp -``` - -The resulting my-podman.te file should look something like this: -``` -module my-podman2 1.0; - -require { - type user_tmp_t; - type container_runtime_t; - type container_t; - class sock_file write; - class unix_stream_socket connectto; -} - -#============= container_t ============== -allow container_t container_runtime_t:unix_stream_socket connectto; -allow container_t user_tmp_t:sock_file write; - -``` - -**NOTE: It will take two rounds of installing a policy, then experiencing the next se-linux issue, install new policy, etc...** - diff --git a/mkdocs.yml b/mkdocs.yml index 824f5091e1..2afab905f7 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -129,9 +129,6 @@ nav: - system_requirements/ci/gitlab_ci.md - system_requirements/ci/tekton.md - system_requirements/ci/travis.md - - system_requirements/using_colima.md - - system_requirements/using_podman.md - - system_requirements/rancher.md - Contributing: - contributing.md - contributing_docs.md From 33528c667838d2bb926960510427478014fd134c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 16 Oct 2024 13:12:01 +0200 Subject: [PATCH 29/50] chore: remove bridge network custom configuration It's not needed at all, as we rely on the underlying container runtime --- docker.go | 6 +-- docker_test.go | 14 ++--- internal/config/config.go | 10 ---- internal/config/config_test.go | 47 +++------------- lifecycle.go | 28 ---------- lifecycle_test.go | 99 ---------------------------------- reaper.go | 2 +- reaper_test.go | 2 +- 8 files changed, 17 insertions(+), 191 deletions(-) diff --git a/docker.go b/docker.go index a50b4ed92e..395dc169ce 100644 --- a/docker.go +++ b/docker.go @@ -43,8 +43,8 @@ import ( var _ Container = (*DockerContainer)(nil) const ( - Bridge = "bridge" // Bridge network driver and name - ReaperDefault = "reaper_default" // Deprecated: use Bridge instead. Default network name when bridge is not available + Bridge = "bridge" // Deprecated, it will removed in the next major release. Bridge network driver and name + ReaperDefault = "reaper_default" // Deprecated: it will removed in the next major release. Default network name when bridge is not available packagePath = "github.com/testcontainers/testcontainers-go" ) @@ -1504,7 +1504,7 @@ func (p *DockerProvider) GetNetwork(ctx context.Context, req NetworkRequest) (ne } func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) { - nw, err := p.GetNetwork(ctx, NetworkRequest{Name: config.Read().TestcontainersBridgeName}) + nw, err := p.GetNetwork(ctx, NetworkRequest{Name: Bridge}) if err != nil { return "", err } diff --git a/docker_test.go b/docker_test.go index 281d010bd9..ee60b7e957 100644 --- a/docker_test.go +++ b/docker_test.go @@ -25,7 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/wait" ) @@ -509,7 +508,9 @@ func TestContainerCreationWithName(t *testing.T) { }, WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort), Name: creationName, - Networks: []string{config.Read().TestcontainersBridgeName}, + // no network means it will be connected to the default bridge network + // of any given container runtime + Networks: []string{}, }, Started: true, }) @@ -530,14 +531,7 @@ func TestContainerCreationWithName(t *testing.T) { t.Fatal(err) } if len(networks) != 1 { - t.Errorf("Expected networks 1. Got '%d'.", len(networks)) - } - - expectedBridge := config.Read().TestcontainersBridgeName - - network := networks[0] - if network != expectedBridge { - t.Errorf("Expected network name '%s'. Got '%s'.", expectedBridge, network) + t.Errorf("Expected networks 1, the default bridge network. Got '%d'.", len(networks)) } endpoint, err := nginxC.PortEndpoint(ctx, nginxDefaultPort, "http") diff --git a/internal/config/config.go b/internal/config/config.go index 36f5e36991..4dcf766d58 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -85,11 +85,6 @@ type Config struct { // // Environment variable: TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE TestcontainersHost string `properties:"tc.host,default="` - - // TestcontainersBridgeName is the name of the bridge network used by Testcontainers. - // - // Environment variable: TESTCONTAINERS_BRIDGE_NAME - TestcontainersBridgeName string `properties:"tc.bridge.name,default=bridge"` } // } @@ -150,11 +145,6 @@ func applyEnvironmentConfiguration(config Config) Config { config.RyukConnectionTimeout = timeout } - testcontainersBridgeName := os.Getenv("TESTCONTAINERS_BRIDGE_NAME") - if testcontainersBridgeName != "" { - config.TestcontainersBridgeName = testcontainersBridgeName - } - return config } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2a683b8fd7..341b62c550 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -129,17 +129,15 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("TESTCONTAINERS_RYUK_VERBOSE", "true") t.Setenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT", "13s") t.Setenv("TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT", "12s") - t.Setenv("TESTCONTAINERS_BRIDGE_NAME", "testbridge") config := read() expected := Config{ - HubImageNamePrefix: defaultHubPrefix, - RyukDisabled: true, - RyukPrivileged: true, - RyukVerbose: true, - RyukReconnectionTimeout: 13 * time.Second, - RyukConnectionTimeout: 12 * time.Second, - TestcontainersBridgeName: "testbridge", + HubImageNamePrefix: defaultHubPrefix, + RyukDisabled: true, + RyukPrivileged: true, + RyukVerbose: true, + RyukReconnectionTimeout: 13 * time.Second, + RyukConnectionTimeout: 12 * time.Second, } assert.Equal(t, expected, config) @@ -149,9 +147,8 @@ func TestReadTCConfig(t *testing.T) { defaultRyukConnectionTimeout := 60 * time.Second defaultRyukReconnectionTimeout := 10 * time.Second defaultCfg := Config{ - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, - TestcontainersBridgeName: "bridge", + RyukConnectionTimeout: defaultRyukConnectionTimeout, + RyukReconnectionTimeout: defaultRyukReconnectionTimeout, } tests := []struct { @@ -477,34 +474,6 @@ func TestReadTCConfig(t *testing.T) { HubImageNamePrefix: defaultHubPrefix + "/env/", }, }, - { - "bridge-name/property", - `tc.bridge.name=props`, - map[string]string{}, - Config{ - TestcontainersBridgeName: "props", - }, - }, - { - "bridge-name/env-var", - ``, - map[string]string{ - "TESTCONTAINERS_BRIDGE_NAME": "env", - }, - Config{ - TestcontainersBridgeName: "env", - }, - }, - { - "bridge-name/env-var-wins", - `tc.bridge.name=props`, - map[string]string{ - "TESTCONTAINERS_BRIDGE_NAME": "env", - }, - Config{ - TestcontainersBridgeName: "env", - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/lifecycle.go b/lifecycle.go index c49d8a60f0..9270a790b2 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -12,8 +12,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/docker/go-connections/nat" - - "github.com/testcontainers/testcontainers-go/internal/config" ) // ContainerRequestHook is a hook that will be called before a container is created. @@ -509,16 +507,6 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain networkingConfig.EndpointsConfig = endpointSettings - // Move the bridge network to the user-defined bridge network, if configured. - // This is needed because different container runtimes might use different a bridge network name. - // As an example, when listing the networks, the Docker client always retrives the default - // bridge network as "bridge", but when attaching a container to that bridge network, - // the container runtime can fail because the bridge network is not named "bridge". - // For that reason we need to move the bridge network to the user-defined bridge network, - // that's why we are offering an extension point to configure the bridge network name - // at the Testcontainers configuration level. - bridgeNetworModifier(networkingConfig.EndpointsConfig) - exposedPorts := req.ExposedPorts // this check must be done after the pre-creation Modifiers are called, so the network mode is already set if len(exposedPorts) == 0 && !hostConfig.NetworkMode.IsContainer() { @@ -640,19 +628,3 @@ func defaultHostConfigModifier(req ContainerRequest) func(hostConfig *container. hostConfig.Resources = req.Resources } } - -// bridgeNetworModifier moves the bridge network to the user-defined bridge network, if configured. -func bridgeNetworModifier(endpointSettings map[string]*network.EndpointSettings) { - userDefinedBridge := config.Read().TestcontainersBridgeName - - if userDefinedBridge == Bridge { - return - } - - // If the map contains a bridge network, use the configured bridge network. - if _, ok := endpointSettings[Bridge]; ok { - nw := endpointSettings[Bridge] - delete(endpointSettings, Bridge) - endpointSettings[userDefinedBridge] = nw - } -} diff --git a/lifecycle_test.go b/lifecycle_test.go index 44c11a334d..04da1df835 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -4,8 +4,6 @@ import ( "bufio" "context" "fmt" - "os" - "path/filepath" "strings" "testing" "time" @@ -18,7 +16,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/wait" ) @@ -297,102 +294,6 @@ func TestPreCreateModifierHook(t *testing.T) { ) }) - t.Run("endpoint-settings-modifier/custom-bridge-network", func(t *testing.T) { - // set testcontainers properties including a custom bridge network name - tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) - t.Setenv("USERPROFILE", tmpDir) // windows - - const bridgeNetworkName = "test-bridge" - content := "tc.bridge.name=" + bridgeNetworkName - if err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(content), 0o600); err != nil { - t.Errorf("Failed to create the file: %v", err) - return - } - config.Reset() // reset the config to reload the properties for testing purposes - - req := ContainerRequest{ - Image: nginxAlpineImage, // alpine image does expose port 80 - } - - // define empty inputs to be overwritten by the pre create hook - inputConfig := &container.Config{ - Image: req.Image, - } - inputHostConfig := &container.HostConfig{} - inputNetworkingConfig := &network.NetworkingConfig{ - EndpointsConfig: map[string]*network.EndpointSettings{ - "bridge": { - MacAddress: "bridge-mac", - Aliases: []string{"bridge-alias"}, - }, - }, - } - - err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) - require.NoError(t, err) - - // assertions - - require.Equal( - t, - "bridge-mac", - inputNetworkingConfig.EndpointsConfig[bridgeNetworkName].MacAddress, - ) - require.Equal( - t, - []string{"bridge-alias"}, - inputNetworkingConfig.EndpointsConfig[bridgeNetworkName].Aliases, - ) - }) - - t.Run("endpoint-settings-modifier/default-bridge-network", func(t *testing.T) { - // set testcontainers properties including a custom bridge network name - tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) - t.Setenv("USERPROFILE", tmpDir) // windows - - if err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(""), 0o600); err != nil { - t.Errorf("Failed to create the file: %v", err) - return - } - config.Reset() // reset the config to reload the properties for testing purposes - - req := ContainerRequest{ - Image: nginxAlpineImage, // alpine image does expose port 80 - } - - // define empty inputs to be overwritten by the pre create hook - inputConfig := &container.Config{ - Image: req.Image, - } - inputHostConfig := &container.HostConfig{} - inputNetworkingConfig := &network.NetworkingConfig{ - EndpointsConfig: map[string]*network.EndpointSettings{ - Bridge: { - MacAddress: "bridge-mac", - Aliases: []string{"bridge-alias"}, - }, - }, - } - - err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) - require.NoError(t, err) - - // assertions - - require.Equal( - t, - "bridge-mac", - inputNetworkingConfig.EndpointsConfig[Bridge].MacAddress, - ) - require.Equal( - t, - []string{"bridge-alias"}, - inputNetworkingConfig.EndpointsConfig[Bridge].Aliases, - ) - }) - t.Run("Request contains exposed port modifiers without protocol", func(t *testing.T) { req := ContainerRequest{ Image: nginxAlpineImage, // alpine image does expose port 80 diff --git a/reaper.go b/reaper.go index d0132fb809..9d74f573e2 100644 --- a/reaper.go +++ b/reaper.go @@ -254,7 +254,7 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider) ( HostConfigModifier: func(hc *container.HostConfig) { hc.AutoRemove = true hc.Binds = []string{dockerHostMount + ":/var/run/docker.sock"} - hc.NetworkMode = Bridge + hc.NetworkMode = "bridge" }, Env: map[string]string{}, } diff --git a/reaper_test.go b/reaper_test.go index e285600465..cd0e1ee7d8 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -438,7 +438,7 @@ func Test_NewReaper(t *testing.T) { assert.Equal(t, test.req.Env, provider.req.Env, "expected env doesn't match the submitted request") // checks for reaper's preCreationCallback fields - assert.Equal(t, container.NetworkMode(Bridge), provider.hostConfig.NetworkMode, "expected networkMode doesn't match the submitted request") + assert.Equal(t, container.NetworkMode("bridge"), provider.hostConfig.NetworkMode, "expected networkMode doesn't match the submitted request") assert.True(t, provider.hostConfig.AutoRemove, "expected networkMode doesn't match the submitted request") }) } From a313dc566e64a2c793f9f7be1196215407af34aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 21 Oct 2024 18:30:33 +0200 Subject: [PATCH 30/50] chore: use t.Helper --- internal/core/docker_context_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/core/docker_context_test.go b/internal/core/docker_context_test.go index 221128533e..a294bd020f 100644 --- a/internal/core/docker_context_test.go +++ b/internal/core/docker_context_test.go @@ -21,6 +21,8 @@ import ( // The config.json file contains the current context, and the meta.json files contain the metadata for each context. // It generates the specified number of contexts, setting the current context to the one specified by currentContextIndex. func setupDockerContexts(t *testing.T, currentContextIndex int, contextsCount int) { + t.Helper() + configDir := filepath.Join(getHomeDir(), configFileDir) err := createTmpDir(configDir) From bc2f04b571e0dec4be475a48551e3020c48c638f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 19 Nov 2024 16:26:59 +0100 Subject: [PATCH 31/50] chore: proper deprecation path --- docker.go | 1 + provider.go | 1 + 2 files changed, 2 insertions(+) diff --git a/docker.go b/docker.go index e781ff2a31..1230b55119 100644 --- a/docker.go +++ b/docker.go @@ -44,6 +44,7 @@ var _ Container = (*DockerContainer)(nil) const ( Bridge = "bridge" // Deprecated, it will removed in the next major release. Bridge network driver and name + Podman = "podman" // Deprecated: Podman is supported through the current Docker context ReaperDefault = "reaper_default" // Deprecated: it will removed in the next major release. Default network name when bridge is not available packagePath = "github.com/testcontainers/testcontainers-go" ) diff --git a/provider.go b/provider.go index 9939178d94..69765baab5 100644 --- a/provider.go +++ b/provider.go @@ -12,6 +12,7 @@ import ( const ( ProviderDefault ProviderType = iota // default will auto-detect provider from DOCKER_HOST environment variable ProviderDocker + ProviderPodman // Deprecated: Podman is supported through the current Docker context ) type ( From ad79b43bed83c625fc53026f8948b4e4e3e352f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 15:01:57 +0100 Subject: [PATCH 32/50] chore: comment out empty field used for documentation --- docker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker_test.go b/docker_test.go index 70fa9e356a..43a716a293 100644 --- a/docker_test.go +++ b/docker_test.go @@ -440,7 +440,7 @@ func TestContainerCreationWithName(t *testing.T) { Name: creationName, // no network means it will be connected to the default bridge network // of any given container runtime - Networks: []string{}, + // Networks: []string{}, }, Started: true, }) From 4d4769fedf6e294311d1efb3c397b82d115bde78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 15:04:22 +0100 Subject: [PATCH 33/50] chore: use require --- docker_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker_test.go b/docker_test.go index 43a716a293..7e4c94b613 100644 --- a/docker_test.go +++ b/docker_test.go @@ -456,7 +456,7 @@ func TestContainerCreationWithName(t *testing.T) { require.NoError(t, err) require.Lenf(t, networks, 1, "Expected networks 1. Got '%d'.", len(networks)) network := networks[0] - assert.Equalf(t, Bridge, network, "Expected network name '%s'. Got '%s'.", Bridge, network) + require.Equal(t, Bridge, network) endpoint, err := nginxC.PortEndpoint(ctx, nginxDefaultPort, "http") require.NoError(t, err) @@ -671,7 +671,7 @@ func Test_BuildContainerFromDockerfileWithBuildLog(t *testing.T) { temp := strings.Split(string(out), "\n") require.NotEmpty(t, temp) - assert.Regexpf(t, `^(?i:Step)\s*1/\d+\s*:\s*FROM alpine$`, temp[0], "Expected stdout first line to be %s. Got '%s'.", "Step 1/* : FROM alpine", temp[0]) + require.Regexp(t, `^(?i:Step)\s*1/\d+\s*:\s*FROM alpine$`, temp[0]) } func TestContainerCreationWaitsForLogAndPortContextTimeout(t *testing.T) { From 56d6774dcf2cc804d960d24c38ea3e8f4f4527dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 16:16:53 +0100 Subject: [PATCH 34/50] docs: update podman commands --- docs/system_requirements/docker.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/system_requirements/docker.md b/docs/system_requirements/docker.md index 7a6e8af27f..ea99134be0 100644 --- a/docs/system_requirements/docker.md +++ b/docs/system_requirements/docker.md @@ -45,10 +45,10 @@ docker context use orbstack ### Podman -Podman does not create its own Docker context when it is installed so, after starting a `podman-machine`, please create it with the following command: +Podman does not create its own Docker context when it is installed so, after starting a `podman-machine` in **rootful mode**, please create the context with the following command: ```sh -docker context create podman --description "podman context" --docker "host=unix:///var/folders/_j/nhbgdck523n3008dd3zlsm5m0000gn/T/podman/podman-machine-default-api.sock" +podman context create podman --description "podman context" --docker "host=unix:///var/folders/_j/nhbgdck523n3008dd3zlsm5m0000gn/T/podman/podman-machine-default-api.sock" ``` !!! note @@ -57,7 +57,7 @@ docker context create podman --description "podman context" --docker "host=unix: Then you can set this as the active context by running: ```sh -docker context use podman +podman context use podman ``` ### Rancher Desktop From 1273e02da75a3063c6bcdddc9a6d00c0666af16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 16:20:14 +0100 Subject: [PATCH 35/50] chore: readability in var comments --- internal/core/docker_context.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index b8c691c16c..3f582b6a33 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -21,16 +21,19 @@ import ( const ( // defaultContextName is the name reserved for the default context (config & env based) defaultContextName = "default" + // envOverrideContext is the name of the environment variable that can be // used to override the context to use. If set, it overrides the context // that's set in the CLI's configuration file, but takes no effect if the // "DOCKER_HOST" env-var is set (which takes precedence. envOverrideContext = "DOCKER_CONTEXT" + // envOverrideConfigDir is the name of the environment variable that can be // used to override the location of the client configuration files (~/.docker). // // It takes priority over the default. envOverrideConfigDir = "DOCKER_CONFIG" + // configFileName is the name of the client configuration file inside the // config-directory. configFileName = "config.json" @@ -38,6 +41,7 @@ const ( contextsDir = "contexts" metadataDir = "meta" metaFile = "meta.json" + // DockerEndpoint is the name of the docker endpoint in a stored context dockerEndpoint string = "docker" ) From e7dd83c2e2ee793e13ec292ad263d4600f866205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 16:25:54 +0100 Subject: [PATCH 36/50] chore: unique parsing error messages --- internal/core/docker_context.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index 3f582b6a33..bd1f9ae99b 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -138,17 +138,17 @@ func (s *metadataStore) getByID(id contextdir) (metadata, error) { } if err := json.Unmarshal(bytes, &untyped); err != nil { - return metadata{}, fmt.Errorf("parsing %s: %w", fileName, err) + return metadata{}, fmt.Errorf("parsing %s (metadata): %w", fileName, err) } r.Name = untyped.Name if r.Metadata, err = parseTypedOrMap(untyped.Metadata, s.config.contextType); err != nil { - return metadata{}, fmt.Errorf("parsing %s: %w", fileName, err) + return metadata{}, fmt.Errorf("parsing %s (context type): %w", fileName, err) } for k, v := range untyped.Endpoints { if r.Endpoints[k], err = parseTypedOrMap(v, s.config.endpointTypes[k]); err != nil { - return metadata{}, fmt.Errorf("parsing %s: %w", fileName, err) + return metadata{}, fmt.Errorf("parsing %s (endpoint types): %w", fileName, err) } } From 584f6da5997ec7e4e3b7879d0e6e04c81cdd0594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 16:30:20 +0100 Subject: [PATCH 37/50] docs: readability as function comments --- internal/core/docker_context.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index bd1f9ae99b..c7afc8897a 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -122,6 +122,7 @@ type untypedContextMetadata struct { Name string `json:"name,omitempty"` } +// getByID returns the metadata for a context by its ID func (s *metadataStore) getByID(id contextdir) (metadata, error) { fileName := filepath.Join(s.contextDir(id), metaFile) bytes, err := os.ReadFile(fileName) @@ -180,8 +181,11 @@ func (s *metadataStore) list() ([]metadata, error) { return res, nil } +// contextdir is a type used to represent a context directory type contextdir string +// isContextDir checks if the given path is a context directory, +// which means it contains a meta.json file. func isContextDir(path string) bool { s, err := os.Stat(filepath.Join(path, metaFile)) if err != nil { @@ -191,26 +195,27 @@ func isContextDir(path string) bool { return !s.IsDir() } +// listRecursivelyMetadataDirs lists all directories that contain a meta.json file func listRecursivelyMetadataDirs(root string) ([]string, error) { - fis, err := os.ReadDir(root) + fileEntries, err := os.ReadDir(root) if err != nil { return nil, err } var result []string - for _, fi := range fis { - if fi.IsDir() { - if isContextDir(filepath.Join(root, fi.Name())) { - result = append(result, fi.Name()) + for _, fileEntry := range fileEntries { + if fileEntry.IsDir() { + if isContextDir(filepath.Join(root, fileEntry.Name())) { + result = append(result, fileEntry.Name()) } - subs, err := listRecursivelyMetadataDirs(filepath.Join(root, fi.Name())) + subs, err := listRecursivelyMetadataDirs(filepath.Join(root, fileEntry.Name())) if err != nil { return nil, err } for _, s := range subs { - result = append(result, filepath.Join(fi.Name(), s)) + result = append(result, filepath.Join(fileEntry.Name(), s)) } } } @@ -218,6 +223,7 @@ func listRecursivelyMetadataDirs(root string) ([]string, error) { return result, nil } +// parseTypedOrMap parses a JSON payload into a typed object or a map func parseTypedOrMap(payload []byte, getter typeGetter) (any, error) { if len(payload) == 0 || string(payload) == "null" { return nil, nil From 3b590860b504e860143f28f5d010ca110367263a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 16:43:08 +0100 Subject: [PATCH 38/50] chore: idiomatic var declaration --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 3875d12b81..2a5a447fb3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -91,7 +91,7 @@ type Config struct { // defaultConfig func defaultConfig() Config { - config := Config{} + var config Config home, err := os.UserHomeDir() if err != nil { From 1c93fe8d45dd28eb2d929950542a210fa0fbec3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 16:52:01 +0100 Subject: [PATCH 39/50] chore: extract file name to constant --- internal/config/config.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 2a5a447fb3..8fead11c36 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,7 +11,13 @@ import ( "github.com/magiconair/properties" ) -const ReaperDefaultImage = "testcontainers/ryuk:0.11.0" +const ( + // ReaperDefaultImage is the default image used for Ryuk, the resource reaper. + ReaperDefaultImage = "testcontainers/ryuk:0.11.0" + + // TestcontainersProperties is the name of the properties file used to configure Testcontainers. + TestcontainersProperties = ".testcontainers.properties" +) var ( tcConfig Config @@ -98,7 +104,7 @@ func defaultConfig() Config { return applyEnvironmentConfiguration(config) } - tcProp := filepath.Join(home, ".testcontainers.properties") + tcProp := filepath.Join(home, TestcontainersProperties) // Init from a file, ignore if it doesn't exist, which is the case for most users. // The properties library will return the default values for the struct. props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) From 92d306ffb831801ef806f083d401bdc55126e868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 18:54:20 +0100 Subject: [PATCH 40/50] chore: simplify default config --- internal/config/config.go | 45 ++++++++++++++-------------------- internal/config/config_test.go | 15 ++++++++++-- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 8fead11c36..ea2d128c72 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -95,31 +95,6 @@ type Config struct { // } -// defaultConfig -func defaultConfig() Config { - var config Config - - home, err := os.UserHomeDir() - if err != nil { - return applyEnvironmentConfiguration(config) - } - - tcProp := filepath.Join(home, TestcontainersProperties) - // Init from a file, ignore if it doesn't exist, which is the case for most users. - // The properties library will return the default values for the struct. - props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) - if err != nil { - return applyEnvironmentConfiguration(config) - } - - if err := props.Decode(&config); err != nil { - fmt.Printf("invalid testcontainers properties file, returning an empty Testcontainers configuration: %v\n", err) - return applyEnvironmentConfiguration(config) - } - - return config -} - func applyEnvironmentConfiguration(config Config) Config { ryukDisabledEnv := os.Getenv("TESTCONTAINERS_RYUK_DISABLED") if parseBool(ryukDisabledEnv) { @@ -173,7 +148,25 @@ func Reset() { } func read() Config { - config := defaultConfig() + var config Config + + home, err := os.UserHomeDir() + if err != nil { + return applyEnvironmentConfiguration(config) + } + + tcProp := filepath.Join(home, TestcontainersProperties) + // Init from a file, ignore if it doesn't exist, which is the case for most users. + // The properties library will return the default values for the struct. + props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) + if err != nil { + return applyEnvironmentConfiguration(config) + } + + if err := props.Decode(&config); err != nil { + fmt.Printf("invalid testcontainers properties file, returning an empty Testcontainers configuration: %v\n", err) + return applyEnvironmentConfiguration(config) + } return applyEnvironmentConfiguration(config) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b94a6ac289..626d8ff53b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -102,7 +102,11 @@ func TestReadTCConfig(t *testing.T) { config := read() - expected := defaultConfig() + // The time fields are set to the default values. + expected := Config{ + RyukReconnectionTimeout: 10 * time.Second, + RyukConnectionTimeout: time.Minute, + } assert.Equal(t, expected, config) }) @@ -114,7 +118,14 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("DOCKER_HOST", tcpDockerHost33293) config := read() - expected := defaultConfig() // the config does not read DOCKER_HOST, that's why it's empty + + // The time fields are set to the default values, + // and the config does not read DOCKER_HOST, + // that's why it's empty + expected := Config{ + RyukReconnectionTimeout: 10 * time.Second, + RyukConnectionTimeout: time.Minute, + } assert.Equal(t, expected, config) }) From 499a05d8857d769134852db6e7ee4d23224170ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 18:58:51 +0100 Subject: [PATCH 41/50] chore: make errors private --- internal/core/docker_context.go | 2 +- internal/core/docker_host.go | 44 +++++++++++++++---------------- internal/core/docker_host_test.go | 14 +++++----- internal/core/docker_rootless.go | 2 +- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/internal/core/docker_context.go b/internal/core/docker_context.go index c7afc8897a..bddde30d82 100644 --- a/internal/core/docker_context.go +++ b/internal/core/docker_context.go @@ -308,7 +308,7 @@ func GetDockerHostFromCurrentContext() (string, error) { } } - return "", ErrDockerSocketNotSetInDockerContext + return "", errDockerSocketNotSetInDockerContext } // currentContext returns the current context name, based on diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index c641c6f848..fa41924148 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -19,16 +19,16 @@ type dockerHostContext string var DockerHostContextKey = dockerHostContext("docker_host") var ( - ErrDockerHostNotSet = errors.New("DOCKER_HOST is not set") - ErrDockerSocketOverrideNotSet = errors.New("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE is not set") - ErrDockerSocketNotSetInContext = errors.New("socket not set in Go context") - ErrDockerSocketNotSetInDockerContext = errors.New("socket not set in Docker context") - ErrDockerSocketNotSetInProperties = errors.New("socket not set in ~/.testcontainers.properties") - ErrNoUnixSchema = errors.New("URL schema is not unix") - ErrSocketNotFound = errors.New("socket not found") - ErrSocketNotFoundInPath = errors.New("docker socket not found in " + DockerSocketPath) - // ErrTestcontainersHostNotSetInProperties this error is specific to Testcontainers - ErrTestcontainersHostNotSetInProperties = errors.New("tc.host not set in ~/.testcontainers.properties") + errDockerHostNotSet = errors.New("DOCKER_HOST is not set") + errDockerSocketOverrideNotSet = errors.New("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE is not set") + errDockerSocketNotSetInContext = errors.New("socket not set in Go context") + errDockerSocketNotSetInDockerContext = errors.New("socket not set in Docker context") + errDockerSocketNotSetInProperties = errors.New("socket not set in ~/.testcontainers.properties") + errNoUnixSchema = errors.New("URL schema is not unix") + errSocketNotFound = errors.New("socket not found") + errSocketNotFoundInPath = errors.New("docker socket not found in " + DockerSocketPath) + // errTestcontainersHostNotSetInProperties this error is specific to Testcontainers + errTestcontainersHostNotSetInProperties = errors.New("tc.host not set in ~/.testcontainers.properties") ) var ( @@ -155,7 +155,7 @@ func extractDockerHost(ctx context.Context) (string, error) { return "", errors.Join(errs...) } - return "", ErrSocketNotFound + return "", errSocketNotFound } // extractDockerSocket Extracts the docker socket from the different alternatives, without caching the result. @@ -232,11 +232,11 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st // not being set, false otherwise. func isHostNotSet(err error) bool { switch { - case errors.Is(err, ErrTestcontainersHostNotSetInProperties), - errors.Is(err, ErrDockerHostNotSet), - errors.Is(err, ErrDockerSocketNotSetInContext), - errors.Is(err, ErrDockerSocketNotSetInProperties), - errors.Is(err, ErrSocketNotFoundInPath), + case errors.Is(err, errTestcontainersHostNotSetInProperties), + errors.Is(err, errDockerHostNotSet), + errors.Is(err, errDockerSocketNotSetInContext), + errors.Is(err, errDockerSocketNotSetInProperties), + errors.Is(err, errSocketNotFoundInPath), errors.Is(err, ErrXDGRuntimeDirNotSet), errors.Is(err, ErrRootlessDockerNotFoundHomeRunDir), errors.Is(err, ErrRootlessDockerNotFoundHomeDesktopDir), @@ -253,7 +253,7 @@ func dockerHostFromEnv(ctx context.Context) (string, error) { return dockerHostPath, nil } - return "", ErrDockerHostNotSet + return "", errDockerHostNotSet } // dockerHostFromContext returns the docker host from the Go context, if it's not empty @@ -267,7 +267,7 @@ func dockerHostFromContext(ctx context.Context) (string, error) { return parsed, nil } - return "", ErrDockerSocketNotSetInContext + return "", errDockerSocketNotSetInContext } // dockerHostFromContext returns the docker host from the Go context, if it's not empty @@ -288,7 +288,7 @@ func dockerHostFromProperties(ctx context.Context) (string, error) { return socketPath, nil } - return "", ErrDockerSocketNotSetInProperties + return "", errDockerSocketNotSetInProperties } // dockerSocketOverridePath returns the docker socket from the TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE environment variable, @@ -298,7 +298,7 @@ func dockerSocketOverridePath() (string, error) { return dockerHostPath, nil } - return "", ErrDockerSocketOverrideNotSet + return "", errDockerSocketOverrideNotSet } // dockerSocketPath returns the docker socket from the default docker socket path, if it's not empty @@ -308,7 +308,7 @@ func dockerSocketPath(ctx context.Context) (string, error) { return DockerSocketPathWithSchema, nil } - return "", ErrSocketNotFoundInPath + return "", errSocketNotFoundInPath } // testcontainersHostFromProperties returns the testcontainers host from the ~/.testcontainers.properties file, if it's not empty @@ -324,7 +324,7 @@ func testcontainersHostFromProperties(ctx context.Context) (string, error) { return parsed, nil } - return "", ErrTestcontainersHostNotSetInProperties + return "", errTestcontainersHostNotSetInProperties } // InAContainer returns true if the code is running inside a container diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 8f557599ee..6c1b67a345 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -207,7 +207,7 @@ func TestExtractDockerHost(t *testing.T) { setupTestcontainersProperties(t, content) socket, err := testcontainersHostFromProperties(context.Background()) - require.ErrorIs(t, err, ErrTestcontainersHostNotSetInProperties) + require.ErrorIs(t, err, errTestcontainersHostNotSetInProperties) require.Empty(t, socket) }) @@ -227,7 +227,7 @@ func TestExtractDockerHost(t *testing.T) { t.Setenv("DOCKER_HOST", "") socket, err := dockerHostFromEnv(context.Background()) - require.ErrorIs(t, err, ErrDockerHostNotSet) + require.ErrorIs(t, err, errDockerHostNotSet) require.Empty(t, socket) }) @@ -251,7 +251,7 @@ func TestExtractDockerHost(t *testing.T) { os.Unsetenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") socket, err := dockerSocketOverridePath() - require.ErrorIs(t, err, ErrDockerSocketOverrideNotSet) + require.ErrorIs(t, err, errDockerSocketOverrideNotSet) require.Empty(t, socket) }) @@ -275,7 +275,7 @@ func TestExtractDockerHost(t *testing.T) { ctx := context.Background() socket, err := dockerHostFromContext(context.WithValue(ctx, DockerHostContextKey, "http://example.com/docker.sock")) - require.ErrorIs(t, err, ErrNoUnixSchema) + require.ErrorIs(t, err, errNoUnixSchema) require.Empty(t, socket) }) @@ -304,7 +304,7 @@ func TestExtractDockerHost(t *testing.T) { setupTestcontainersProperties(t, content) socket, err := dockerHostFromProperties(context.Background()) - require.ErrorIs(t, err, ErrDockerSocketNotSetInProperties) + require.ErrorIs(t, err, errDockerSocketNotSetInProperties) require.Empty(t, socket) }) @@ -312,13 +312,13 @@ func TestExtractDockerHost(t *testing.T) { setupDockerSocketNotFound(t) socket, err := dockerSocketPath(context.Background()) - require.ErrorIs(t, err, ErrSocketNotFoundInPath) + require.ErrorIs(t, err, errSocketNotFoundInPath) require.Empty(t, socket) }) t.Run("extract-from-docker-context/not-found", func(tt *testing.T) { host, err := dockerHostFromDockerContext(context.Background()) - require.ErrorIs(tt, err, ErrDockerSocketNotSetInDockerContext) + require.ErrorIs(tt, err, errDockerSocketNotSetInDockerContext) assert.Empty(tt, host) }) diff --git a/internal/core/docker_rootless.go b/internal/core/docker_rootless.go index 70cdebf240..7f5082e975 100644 --- a/internal/core/docker_rootless.go +++ b/internal/core/docker_rootless.go @@ -93,7 +93,7 @@ func parseURL(s string) (string, error) { // return the original URL, as it is a valid TCP URL return s, nil default: - return "", ErrNoUnixSchema + return "", errNoUnixSchema } } From cdb874d4d893619a4bb32fbdc685e006b1680850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 19:16:17 +0100 Subject: [PATCH 42/50] chore: simplify function --- internal/core/docker_host.go | 4 +++- internal/core/docker_host_test.go | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index fa41924148..0e9971c4a7 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -127,7 +127,9 @@ func extractDockerHost(ctx context.Context) (string, error) { testcontainersHostFromProperties, dockerHostFromEnv, dockerHostFromContext, - dockerHostFromDockerContext, + func(_ context.Context) (string, error) { + return GetDockerHostFromCurrentContext() + }, dockerSocketPath, dockerHostFromProperties, rootlessDockerSocketPath, diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 6c1b67a345..7d3a571de8 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -317,7 +317,7 @@ func TestExtractDockerHost(t *testing.T) { }) t.Run("extract-from-docker-context/not-found", func(tt *testing.T) { - host, err := dockerHostFromDockerContext(context.Background()) + host, err := GetDockerHostFromCurrentContext() require.ErrorIs(tt, err, errDockerSocketNotSetInDockerContext) assert.Empty(tt, host) }) @@ -325,7 +325,7 @@ func TestExtractDockerHost(t *testing.T) { t.Run("extract-from-docker-context/found", func(tt *testing.T) { setupDockerContexts(tt, 2, 3) // current context is context2 - host, err := dockerHostFromDockerContext(context.Background()) + host, err := GetDockerHostFromCurrentContext() require.NoError(tt, err) assert.Equal(tt, "tcp://127.0.0.1:2", host) }) From 1933f4282fe817db8702c63247fe4902418d6063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 25 Nov 2024 23:33:43 +0100 Subject: [PATCH 43/50] fix: remove unused func --- internal/core/docker_host.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 0e9971c4a7..cb2d22813f 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -272,16 +272,6 @@ func dockerHostFromContext(ctx context.Context) (string, error) { return "", errDockerSocketNotSetInContext } -// dockerHostFromContext returns the docker host from the Go context, if it's not empty -func dockerHostFromDockerContext(ctx context.Context) (string, error) { - dockerHost, err := GetDockerHostFromCurrentContext() - if err == nil { - return dockerHost, nil - } - - return "", err -} - // dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty func dockerHostFromProperties(ctx context.Context) (string, error) { cfg := config.Read() From f4783a7bbe02fc582b58b0080f45c7f9bf1b01e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 4 Dec 2024 17:44:21 +0100 Subject: [PATCH 44/50] chore: bubble up config errors when when config is incorrect --- config.go | 6 +- internal/config/config.go | 18 +- internal/config/config_test.go | 315 ++++++++++++++++++--------------- internal/core/client.go | 6 +- internal/core/docker_host.go | 12 +- internal/core/labels.go | 8 +- logconsumer_test.go | 5 +- provider.go | 7 +- reaper.go | 14 +- 9 files changed, 227 insertions(+), 164 deletions(-) diff --git a/config.go b/config.go index 91a333107d..8c74d22c37 100644 --- a/config.go +++ b/config.go @@ -17,7 +17,11 @@ type TestcontainersConfig struct { // ReadConfig reads from testcontainers properties file, storing the result in a singleton instance // of the TestcontainersConfig struct func ReadConfig() TestcontainersConfig { - cfg := config.Read() + cfg, err := config.Read() + if err != nil { + return TestcontainersConfig{} + } + return TestcontainersConfig{ Host: cfg.Host, TLSVerify: cfg.TLSVerify, diff --git a/internal/config/config.go b/internal/config/config.go index ea2d128c72..6914128a2d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -131,12 +131,13 @@ func applyEnvironmentConfiguration(config Config) Config { // Read reads from testcontainers properties file, if it exists // it is possible that certain values get overridden when set as environment variables -func Read() Config { +func Read() (Config, error) { + var err error tcConfigOnce.Do(func() { - tcConfig = read() + tcConfig, err = read() }) - return tcConfig + return tcConfig, err } // Reset resets the singleton instance of the Config struct, @@ -147,12 +148,12 @@ func Reset() { tcConfigOnce = new(sync.Once) } -func read() Config { +func read() (Config, error) { var config Config home, err := os.UserHomeDir() if err != nil { - return applyEnvironmentConfiguration(config) + return applyEnvironmentConfiguration(config), nil } tcProp := filepath.Join(home, TestcontainersProperties) @@ -160,15 +161,14 @@ func read() Config { // The properties library will return the default values for the struct. props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) if err != nil { - return applyEnvironmentConfiguration(config) + return applyEnvironmentConfiguration(config), nil } if err := props.Decode(&config); err != nil { - fmt.Printf("invalid testcontainers properties file, returning an empty Testcontainers configuration: %v\n", err) - return applyEnvironmentConfiguration(config) + return applyEnvironmentConfiguration(config), fmt.Errorf("invalid testcontainers properties file: %w", err) } - return applyEnvironmentConfiguration(config) + return applyEnvironmentConfiguration(config), nil } func parseBool(input string) bool { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 626d8ff53b..9ebdaee0c9 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -40,7 +40,8 @@ func TestReadConfig(t *testing.T) { t.Setenv("DOCKER_HOST", "") t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true") - config := Read() + config, err := Read() + require.NoError(t, err) expected := Config{ RyukDisabled: true, @@ -51,7 +52,8 @@ func TestReadConfig(t *testing.T) { t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "false") - config = Read() + config, err = Read() + require.NoError(t, err) assert.Equal(t, expected, config) }) } @@ -65,8 +67,8 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("HOME", "") t.Setenv("USERPROFILE", "") // Windows support - config := read() - + config, err := read() + require.NoError(t, err) expected := Config{} assert.Equal(t, expected, config) @@ -81,7 +83,8 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("RYUK_RECONNECTION_TIMEOUT", "13s") t.Setenv("RYUK_CONNECTION_TIMEOUT", "12s") - config := read() + config, err := read() + require.NoError(t, err) expected := Config{ HubImageNamePrefix: defaultHubPrefix, @@ -100,7 +103,8 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("HOME", tmpDir) t.Setenv("USERPROFILE", tmpDir) // Windows support - config := read() + config, err := read() + require.NoError(t, err) // The time fields are set to the default values. expected := Config{ @@ -117,7 +121,8 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("USERPROFILE", tmpDir) // Windows support t.Setenv("DOCKER_HOST", tcpDockerHost33293) - config := read() + config, err := read() + require.NoError(t, err) // The time fields are set to the default values, // and the config does not read DOCKER_HOST, @@ -141,7 +146,9 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("RYUK_RECONNECTION_TIMEOUT", "13s") t.Setenv("RYUK_CONNECTION_TIMEOUT", "12s") - config := read() + config, err := read() + require.NoError(t, err) + expected := Config{ HubImageNamePrefix: defaultHubPrefix, RyukDisabled: true, @@ -167,321 +174,333 @@ func TestReadTCConfig(t *testing.T) { content string env map[string]string expected Config + wantErr bool }{ { - "Single Docker host with spaces", - "docker.host = " + tcpDockerHost33293, - map[string]string{}, - Config{ + name: "Single Docker host with spaces", + content: "docker.host = " + tcpDockerHost33293, + env: map[string]string{}, + expected: Config{ Host: tcpDockerHost33293, }, }, { - "Multiple docker host entries, last one wins", - `docker.host = ` + tcpDockerHost33293 + ` + name: "Multiple docker host entries, last one wins", + content: `docker.host = ` + tcpDockerHost33293 + ` docker.host = ` + tcpDockerHost4711 + ` `, - map[string]string{}, - Config{ + env: map[string]string{}, + expected: Config{ Host: tcpDockerHost4711, }, }, { - "Multiple docker host entries, last one wins, with TLS", - `docker.host = ` + tcpDockerHost33293 + ` + name: "Multiple docker host entries, last one wins, with TLS", + content: `docker.host = ` + tcpDockerHost33293 + ` docker.host = ` + tcpDockerHost4711 + ` docker.host = ` + tcpDockerHost1234 + ` docker.tls.verify = 1 `, - map[string]string{}, - Config{ + env: map[string]string{}, + expected: Config{ Host: tcpDockerHost1234, TLSVerify: 1, }, }, { - "Empty file", - "", - map[string]string{}, - Config{}, + name: "Empty file", + content: "", + env: map[string]string{}, + expected: Config{}, + wantErr: false, }, { - "Non-valid properties are ignored", - `foo = bar + name: "Non-valid properties are ignored", + content: `foo = bar docker.host = ` + tcpDockerHost1234 + ` `, - map[string]string{}, - Config{ + env: map[string]string{}, + expected: Config{ Host: tcpDockerHost1234, }, }, { - "Single Docker host without spaces", - "docker.host=" + tcpDockerHost33293, - map[string]string{}, - Config{ + name: "Single Docker host without spaces", + content: "docker.host=" + tcpDockerHost33293, + env: map[string]string{}, + expected: Config{ Host: tcpDockerHost33293, }, }, { - "Comments are ignored", - `#docker.host=` + tcpDockerHost33293, - map[string]string{}, - defaultCfg, + name: "Comments are ignored", + content: `#docker.host=` + tcpDockerHost33293, + env: map[string]string{}, + expected: defaultCfg, + wantErr: false, }, { - "Multiple docker host entries, last one wins, with TLS and cert path", - `#docker.host = ` + tcpDockerHost33293 + ` + name: "Multiple docker host entries, last one wins, with TLS and cert path", + content: `#docker.host = ` + tcpDockerHost33293 + ` docker.host = ` + tcpDockerHost4711 + ` docker.host = ` + tcpDockerHost1234 + ` docker.cert.path=/tmp/certs`, - map[string]string{}, - Config{ + env: map[string]string{}, + expected: Config{ Host: tcpDockerHost1234, CertPath: "/tmp/certs", }, }, { - "With Ryuk disabled using properties", - `ryuk.disabled=true`, - map[string]string{}, - Config{ + name: "With Ryuk disabled using properties", + content: `ryuk.disabled=true`, + env: map[string]string{}, + expected: Config{ RyukDisabled: true, }, }, { - "With Ryuk container privileged using properties", - `ryuk.container.privileged=true`, - map[string]string{}, - Config{ + name: "With Ryuk container privileged using properties", + content: `ryuk.container.privileged=true`, + env: map[string]string{}, + expected: Config{ RyukPrivileged: true, }, }, { - "With Ryuk container timeouts configured using properties", - `ryuk.connection.timeout=12s + name: "With Ryuk container timeouts configured using properties", + content: `ryuk.connection.timeout=12s ryuk.reconnection.timeout=13s`, - map[string]string{}, - Config{ + env: map[string]string{}, + expected: Config{ RyukReconnectionTimeout: 13 * time.Second, RyukConnectionTimeout: 12 * time.Second, }, }, { - "With Ryuk container timeouts configured using env vars", - ``, - map[string]string{ + name: "With Ryuk container timeouts configured using env vars", + content: ``, + env: map[string]string{ "RYUK_RECONNECTION_TIMEOUT": "13s", "RYUK_CONNECTION_TIMEOUT": "12s", }, - Config{ + expected: Config{ RyukReconnectionTimeout: 13 * time.Second, RyukConnectionTimeout: 12 * time.Second, }, }, { - "With Ryuk container timeouts configured using env vars and properties. Env var wins", - `ryuk.connection.timeout=22s + name: "With Ryuk container timeouts configured using env vars and properties. Env var wins", + content: `ryuk.connection.timeout=22s ryuk.reconnection.timeout=23s`, - map[string]string{ + env: map[string]string{ "RYUK_RECONNECTION_TIMEOUT": "13s", "RYUK_CONNECTION_TIMEOUT": "12s", }, - Config{ + expected: Config{ RyukReconnectionTimeout: 13 * time.Second, RyukConnectionTimeout: 12 * time.Second, }, }, { - "With Ryuk verbose configured using properties", - `ryuk.verbose=true`, - map[string]string{}, - Config{ + name: "With Ryuk verbose configured using properties", + content: `ryuk.verbose=true`, + env: map[string]string{}, + expected: Config{ RyukVerbose: true, }, }, { - "With Ryuk disabled using an env var", - ``, - map[string]string{ + name: "With Ryuk disabled using an env var", + content: ``, + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", }, - Config{ + expected: Config{ RyukDisabled: true, }, }, { - "With Ryuk container privileged using an env var", - ``, - map[string]string{ + name: "With Ryuk container privileged using an env var", + content: ``, + env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, - Config{ + expected: Config{ RyukPrivileged: true, }, }, { - "With Ryuk disabled using an env var and properties. Env var wins (0)", - `ryuk.disabled=true`, - map[string]string{ + name: "With Ryuk disabled using an env var and properties. Env var wins (0)", + content: `ryuk.disabled=true`, + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", }, - Config{ + expected: Config{ RyukDisabled: true, }, }, { - "With Ryuk disabled using an env var and properties. Env var wins (1)", - `ryuk.disabled=false`, - map[string]string{ + name: "With Ryuk disabled using an env var and properties. Env var wins (1)", + content: `ryuk.disabled=false`, + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", }, - Config{ + expected: Config{ RyukDisabled: true, }, }, { - "With Ryuk disabled using an env var and properties. Env var wins (2)", - `ryuk.disabled=true`, - map[string]string{ + name: "With Ryuk disabled using an env var and properties. Env var wins (2)", + content: `ryuk.disabled=true`, + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "false", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Ryuk disabled using an env var and properties. Env var wins (3)", - `ryuk.disabled=false`, - map[string]string{ + name: "With Ryuk disabled using an env var and properties. Env var wins (3)", + content: `ryuk.disabled=false`, + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "false", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Ryuk verbose using an env var and properties. Env var wins (0)", - `ryuk.verbose=true`, - map[string]string{ + name: "With Ryuk verbose using an env var and properties. Env var wins (0)", + content: `ryuk.verbose=true`, + env: map[string]string{ "RYUK_VERBOSE": "true", }, - Config{ + expected: Config{ RyukVerbose: true, }, }, { - "With Ryuk verbose using an env var and properties. Env var wins (1)", - `ryuk.verbose=false`, - map[string]string{ + name: "With Ryuk verbose using an env var and properties. Env var wins (1)", + content: `ryuk.verbose=false`, + env: map[string]string{ "RYUK_VERBOSE": "true", }, - Config{ + expected: Config{ RyukVerbose: true, }, }, { - "With Ryuk verbose using an env var and properties. Env var wins (2)", - `ryuk.verbose=true`, - map[string]string{ + name: "With Ryuk verbose using an env var and properties. Env var wins (2)", + content: `ryuk.verbose=true`, + env: map[string]string{ "RYUK_VERBOSE": "false", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Ryuk verbose using an env var and properties. Env var wins (3)", - `ryuk.verbose=false`, - map[string]string{ + name: "With Ryuk verbose using an env var and properties. Env var wins (3)", + content: `ryuk.verbose=false`, + env: map[string]string{ "RYUK_VERBOSE": "false", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Ryuk container privileged using an env var and properties. Env var wins (0)", - `ryuk.container.privileged=true`, - map[string]string{ + name: "With Ryuk container privileged using an env var and properties. Env var wins (0)", + content: `ryuk.container.privileged=true`, + env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, - Config{ + expected: Config{ RyukPrivileged: true, }, }, { - "With Ryuk container privileged using an env var and properties. Env var wins (1)", - `ryuk.container.privileged=false`, - map[string]string{ + name: "With Ryuk container privileged using an env var and properties. Env var wins (1)", + content: `ryuk.container.privileged=false`, + env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, - Config{ + expected: Config{ RyukPrivileged: true, }, }, { - "With Ryuk container privileged using an env var and properties. Env var wins (2)", - `ryuk.container.privileged=true`, - map[string]string{ + name: "With Ryuk container privileged using an env var and properties. Env var wins (2)", + content: `ryuk.container.privileged=true`, + env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Ryuk container privileged using an env var and properties. Env var wins (3)", - `ryuk.container.privileged=false`, - map[string]string{ + name: "With Ryuk container privileged using an env var and properties. Env var wins (3)", + content: `ryuk.container.privileged=false`, + env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With TLS verify using properties when value is wrong", - `ryuk.container.privileged=false + name: "With TLS verify using properties when value is wrong", + content: `ryuk.container.privileged=false docker.tls.verify = ERROR`, - map[string]string{ + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", }, - Config{ + expected: Config{ RyukDisabled: true, RyukPrivileged: true, }, + wantErr: true, }, { - "With Ryuk disabled using an env var and properties. Env var does not win because it's not a boolean value", - `ryuk.disabled=false`, - map[string]string{ + name: "With Ryuk disabled using an env var and properties. Env var does not win because it's not a boolean value", + content: `ryuk.disabled=false`, + env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "foo", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Ryuk container privileged using an env var and properties. Env var does not win because it's not a boolean value", - `ryuk.container.privileged=false`, - map[string]string{ + name: "With Ryuk container privileged using an env var and properties. Env var does not win because it's not a boolean value", + content: `ryuk.container.privileged=false`, + env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "foo", }, - defaultCfg, + expected: defaultCfg, + wantErr: false, }, { - "With Hub image name prefix set as a property", - `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, - map[string]string{}, - Config{ + name: "With Hub image name prefix set as a property", + content: `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, + env: map[string]string{}, + expected: Config{ HubImageNamePrefix: defaultHubPrefix + "/props/", }, }, { - "With Hub image name prefix set as env var", - ``, - map[string]string{ + name: "With Hub image name prefix set as env var", + content: ``, + env: map[string]string{ "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX": defaultHubPrefix + "/env/", }, - Config{ + expected: Config{ HubImageNamePrefix: defaultHubPrefix + "/env/", }, }, { - "With Hub image name prefix set as env var and properties: Env var wins", - `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, - map[string]string{ + name: "With Hub image name prefix set as env var and properties: Env var wins", + content: `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, + env: map[string]string{ "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX": defaultHubPrefix + "/env/", }, - Config{ + expected: Config{ HubImageNamePrefix: defaultHubPrefix + "/env/", }, }, @@ -497,8 +516,12 @@ func TestReadTCConfig(t *testing.T) { err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(tt.content), 0o600) require.NoErrorf(t, err, "Failed to create the file") - // - config := read() + config, err := read() + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) // Merge the returned config, and the expected one, with the default config // to avoid setting all the fields in the expected config. diff --git a/internal/core/client.go b/internal/core/client.go index 04a54bcbc5..3a9737db4f 100644 --- a/internal/core/client.go +++ b/internal/core/client.go @@ -2,6 +2,7 @@ package core import ( "context" + "fmt" "path/filepath" "github.com/docker/docker/client" @@ -12,7 +13,10 @@ import ( // NewClient returns a new docker client extracting the docker host from the different alternatives func NewClient(ctx context.Context, ops ...client.Opt) (*client.Client, error) { - tcConfig := config.Read() + tcConfig, err := config.Read() + if err != nil { + return nil, fmt.Errorf("read config: %w", err) + } dockerHost := MustExtractDockerHost(ctx) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index cb2d22813f..eff06bf65a 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -274,7 +274,11 @@ func dockerHostFromContext(ctx context.Context) (string, error) { // dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty func dockerHostFromProperties(ctx context.Context) (string, error) { - cfg := config.Read() + cfg, err := config.Read() + if err != nil { + return "", fmt.Errorf("read config: %w", err) + } + socketPath := cfg.Host if socketPath != "" { return socketPath, nil @@ -305,7 +309,11 @@ func dockerSocketPath(ctx context.Context) (string, error) { // testcontainersHostFromProperties returns the testcontainers host from the ~/.testcontainers.properties file, if it's not empty func testcontainersHostFromProperties(ctx context.Context) (string, error) { - cfg := config.Read() + cfg, err := config.Read() + if err != nil { + return "", fmt.Errorf("read config: %w", err) + } + testcontainersHost := cfg.TestcontainersHost if testcontainersHost != "" { parsed, err := parseURL(testcontainersHost) diff --git a/internal/core/labels.go b/internal/core/labels.go index 0814924234..6739ea6ec1 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -42,7 +42,13 @@ func DefaultLabels(sessionID string) map[string]string { LabelSessionID: sessionID, } - if !config.Read().RyukDisabled { + cfg, err := config.Read() + if err != nil { + // do not apply labels from the config if it's not available + return labels + } + + if !cfg.RyukDisabled { labels[LabelReap] = "true" } diff --git a/logconsumer_test.go b/logconsumer_test.go index dbbedb3adc..b24eaadefb 100644 --- a/logconsumer_test.go +++ b/logconsumer_test.go @@ -283,9 +283,12 @@ func TestContainerLogWithErrClosed(t *testing.T) { require.NoError(t, err) defer dockerClient.Close() + cfg, err := config.Read() + require.NoError(t, err) + provider := &DockerProvider{ client: dockerClient, - config: config.Read(), + config: cfg, DockerProviderOptions: &DockerProviderOptions{ GenericProviderOptions: &GenericProviderOptions{ Logger: TestLogger(t), diff --git a/provider.go b/provider.go index 69765baab5..66afb224ad 100644 --- a/provider.go +++ b/provider.go @@ -125,10 +125,15 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error return nil, err } + cfg, err := config.Read() + if err != nil { + return nil, fmt.Errorf("read config: %w", err) + } + return &DockerProvider{ DockerProviderOptions: o, host: core.MustExtractDockerHost(ctx), client: c, - config: config.Read(), + config: cfg, }, nil } diff --git a/reaper.go b/reaper.go index 2080e52f87..6920f3e38b 100644 --- a/reaper.go +++ b/reaper.go @@ -260,7 +260,12 @@ func (r *reaperSpawner) retryError(err error) error { // // Safe for concurrent calls. func (r *reaperSpawner) reaper(ctx context.Context, sessionID string, provider ReaperProvider) (*Reaper, error) { - if config.Read().RyukDisabled { + cfg, err := config.Read() + if err != nil { + return nil, fmt.Errorf("read config: %w", err) + } + + if cfg.RyukDisabled { return nil, errReaperDisabled } @@ -445,7 +450,12 @@ type Reaper struct { // It returns a channel that can be closed to terminate the connection. // Returns an error if config.RyukDisabled is true. func (r *Reaper) Connect() (chan bool, error) { - if config.Read().RyukDisabled { + cfg, err := config.Read() + if err != nil { + return nil, fmt.Errorf("read config: %w", err) + } + + if cfg.RyukDisabled { return nil, errReaperDisabled } From 28f080251e5e7114cfc75e98b855a43746635dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 4 Dec 2024 17:53:41 +0100 Subject: [PATCH 45/50] chore: use better test names --- internal/config/config_test.go | 68 +++++++++++++++++----------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9ebdaee0c9..ad3fa07889 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -177,7 +177,7 @@ func TestReadTCConfig(t *testing.T) { wantErr bool }{ { - name: "Single Docker host with spaces", + name: "single-docker-host/with-spaces", content: "docker.host = " + tcpDockerHost33293, env: map[string]string{}, expected: Config{ @@ -185,7 +185,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "Multiple docker host entries, last one wins", + name: "multiple-docker-hosts/last-one-wins", content: `docker.host = ` + tcpDockerHost33293 + ` docker.host = ` + tcpDockerHost4711 + ` `, @@ -195,7 +195,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "Multiple docker host entries, last one wins, with TLS", + name: "multiple-docker-hosts/last-one-wins/with-tls", content: `docker.host = ` + tcpDockerHost33293 + ` docker.host = ` + tcpDockerHost4711 + ` docker.host = ` + tcpDockerHost1234 + ` @@ -208,14 +208,14 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "Empty file", + name: "properties/empty", content: "", env: map[string]string{}, expected: Config{}, wantErr: false, }, { - name: "Non-valid properties are ignored", + name: "non-valid-properties-are-ignored", content: `foo = bar docker.host = ` + tcpDockerHost1234 + ` `, @@ -225,7 +225,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "Single Docker host without spaces", + name: "single-docker-host/without-spaces", content: "docker.host=" + tcpDockerHost33293, env: map[string]string{}, expected: Config{ @@ -233,14 +233,14 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "Comments are ignored", + name: "comments-are-ignored", content: `#docker.host=` + tcpDockerHost33293, env: map[string]string{}, expected: defaultCfg, wantErr: false, }, { - name: "Multiple docker host entries, last one wins, with TLS and cert path", + name: "multiple-docker-hosts/last-one-wins/with-tls/cert-path", content: `#docker.host = ` + tcpDockerHost33293 + ` docker.host = ` + tcpDockerHost4711 + ` docker.host = ` + tcpDockerHost1234 + ` @@ -252,7 +252,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk disabled using properties", + name: "using-properties/ryuk-disabled", content: `ryuk.disabled=true`, env: map[string]string{}, expected: Config{ @@ -260,7 +260,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container privileged using properties", + name: "properties/ryuk-container-privileged", content: `ryuk.container.privileged=true`, env: map[string]string{}, expected: Config{ @@ -268,7 +268,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container timeouts configured using properties", + name: "properties/ryuk-container-timeouts", content: `ryuk.connection.timeout=12s ryuk.reconnection.timeout=13s`, env: map[string]string{}, @@ -278,7 +278,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container timeouts configured using env vars", + name: "env-vars/ryuk-container-timeouts", content: ``, env: map[string]string{ "RYUK_RECONNECTION_TIMEOUT": "13s", @@ -290,7 +290,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container timeouts configured using env vars and properties. Env var wins", + name: "env-vars/ryuk-container-timeouts/env-var-wins", content: `ryuk.connection.timeout=22s ryuk.reconnection.timeout=23s`, env: map[string]string{ @@ -303,7 +303,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk verbose configured using properties", + name: "properties/ryuk-verbose", content: `ryuk.verbose=true`, env: map[string]string{}, expected: Config{ @@ -311,7 +311,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk disabled using an env var", + name: "env-vars/ryuk-disabled", content: ``, env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", @@ -321,7 +321,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container privileged using an env var", + name: "env-vars/ryuk-container-privileged", content: ``, env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", @@ -331,7 +331,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk disabled using an env var and properties. Env var wins (0)", + name: "env-vars/properties/ryuk-disabled/env-var-wins-0", content: `ryuk.disabled=true`, env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", @@ -341,7 +341,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk disabled using an env var and properties. Env var wins (1)", + name: "env-vars/properties/ryuk-disabled/env-var-wins-1", content: `ryuk.disabled=false`, env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "true", @@ -351,7 +351,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk disabled using an env var and properties. Env var wins (2)", + name: "env-vars/properties/ryuk-disabled/env-var-wins-2", content: `ryuk.disabled=true`, env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "false", @@ -360,7 +360,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Ryuk disabled using an env var and properties. Env var wins (3)", + name: "env-vars/properties/ryuk-disabled/env-var-wins-3", content: `ryuk.disabled=false`, env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "false", @@ -369,7 +369,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Ryuk verbose using an env var and properties. Env var wins (0)", + name: "env-vars/properties/ryuk-verbose/env-var-wins-0", content: `ryuk.verbose=true`, env: map[string]string{ "RYUK_VERBOSE": "true", @@ -379,7 +379,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk verbose using an env var and properties. Env var wins (1)", + name: "env-vars/properties/ryuk-verbose/env-var-wins-1", content: `ryuk.verbose=false`, env: map[string]string{ "RYUK_VERBOSE": "true", @@ -389,7 +389,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk verbose using an env var and properties. Env var wins (2)", + name: "env-vars/properties/ryuk-verbose/env-var-wins-2", content: `ryuk.verbose=true`, env: map[string]string{ "RYUK_VERBOSE": "false", @@ -398,7 +398,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Ryuk verbose using an env var and properties. Env var wins (3)", + name: "env-vars/properties/ryuk-verbose/env-var-wins-3", content: `ryuk.verbose=false`, env: map[string]string{ "RYUK_VERBOSE": "false", @@ -407,7 +407,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Ryuk container privileged using an env var and properties. Env var wins (0)", + name: "env-vars/properties/ryuk-container-privileged/env-var-wins-0", content: `ryuk.container.privileged=true`, env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", @@ -417,7 +417,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container privileged using an env var and properties. Env var wins (1)", + name: "env-vars/properties/ryuk-container-privileged/env-var-wins-1", content: `ryuk.container.privileged=false`, env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true", @@ -427,7 +427,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Ryuk container privileged using an env var and properties. Env var wins (2)", + name: "env-vars/properties/ryuk-container-privileged/env-var-wins-2", content: `ryuk.container.privileged=true`, env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false", @@ -436,7 +436,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Ryuk container privileged using an env var and properties. Env var wins (3)", + name: "env-vars/properties/ryuk-container-privileged/env-var-wins-3", content: `ryuk.container.privileged=false`, env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false", @@ -445,7 +445,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With TLS verify using properties when value is wrong", + name: "properties/tls-verify/wrong-value", content: `ryuk.container.privileged=false docker.tls.verify = ERROR`, env: map[string]string{ @@ -459,7 +459,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: true, }, { - name: "With Ryuk disabled using an env var and properties. Env var does not win because it's not a boolean value", + name: "env-vars/properties/ryuk-disabled/env-var-does-not-win/wrong-boolean", content: `ryuk.disabled=false`, env: map[string]string{ "TESTCONTAINERS_RYUK_DISABLED": "foo", @@ -468,7 +468,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Ryuk container privileged using an env var and properties. Env var does not win because it's not a boolean value", + name: "env-vars/properties/ryuk-container-privileged/env-var-does-not-win/wrong-boolean", content: `ryuk.container.privileged=false`, env: map[string]string{ "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "foo", @@ -477,7 +477,7 @@ func TestReadTCConfig(t *testing.T) { wantErr: false, }, { - name: "With Hub image name prefix set as a property", + name: "properties/hub-image-name-prefix", content: `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, env: map[string]string{}, expected: Config{ @@ -485,7 +485,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Hub image name prefix set as env var", + name: "env-vars/hub-image-name-prefix", content: ``, env: map[string]string{ "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX": defaultHubPrefix + "/env/", @@ -495,7 +495,7 @@ func TestReadTCConfig(t *testing.T) { }, }, { - name: "With Hub image name prefix set as env var and properties: Env var wins", + name: "env-vars/properties/hub-image-name-prefix/env-var-wins", content: `hub.image.name.prefix=` + defaultHubPrefix + `/props/`, env: map[string]string{ "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX": defaultHubPrefix + "/env/", From f8d24c7afe5069b894fb52859a56f71b1b64fde4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 5 Dec 2024 02:03:44 +0100 Subject: [PATCH 46/50] chore: deprecate ReadConfig in favour of NewConfig --- config.go | 12 +++++++++++- docker.go | 1 + logconsumer_test.go | 2 +- modules/compose/compose_api.go | 7 ++++++- modules/compose/compose_api_test.go | 8 ++++++-- provider.go | 5 +++-- reaper.go | 5 ++++- reaper_test.go | 3 ++- 8 files changed, 34 insertions(+), 9 deletions(-) diff --git a/config.go b/config.go index 8c74d22c37..52ad3b8400 100644 --- a/config.go +++ b/config.go @@ -4,6 +4,7 @@ import ( "github.com/testcontainers/testcontainers-go/internal/config" ) +// Deprecated: use [testcontainers.Config] instead // TestcontainersConfig represents the configuration for Testcontainers type TestcontainersConfig struct { Host string `properties:"docker.host,default="` // Deprecated: use Config.Host instead @@ -14,10 +15,11 @@ type TestcontainersConfig struct { Config config.Config } +// Deprecated: use [testcontainers.NewConfig] instead // ReadConfig reads from testcontainers properties file, storing the result in a singleton instance // of the TestcontainersConfig struct func ReadConfig() TestcontainersConfig { - cfg, err := config.Read() + cfg, err := NewConfig() if err != nil { return TestcontainersConfig{} } @@ -31,3 +33,11 @@ func ReadConfig() TestcontainersConfig { Config: cfg, } } + +// Config is a type alias for the internal config.Config type +type Config = config.Config + +// NewConfig reads the properties file and returns a new Config instance +func NewConfig() (Config, error) { + return config.Read() +} diff --git a/docker.go b/docker.go index 1230b55119..ed3c2dc1a4 100644 --- a/docker.go +++ b/docker.go @@ -1429,6 +1429,7 @@ func (p *DockerProvider) RunContainer(ctx context.Context, req ContainerRequest) return c, nil } +// Deprecated: use [testcontainers.NewConfig] instead // Config provides the TestcontainersConfig read from $HOME/.testcontainers.properties or // the environment variables func (p *DockerProvider) Config() TestcontainersConfig { diff --git a/logconsumer_test.go b/logconsumer_test.go index b24eaadefb..18efc0d31f 100644 --- a/logconsumer_test.go +++ b/logconsumer_test.go @@ -283,7 +283,7 @@ func TestContainerLogWithErrClosed(t *testing.T) { require.NoError(t, err) defer dockerClient.Close() - cfg, err := config.Read() + cfg, err := NewConfig() require.NoError(t, err) provider := &DockerProvider{ diff --git a/modules/compose/compose_api.go b/modules/compose/compose_api.go index 45dd72c6e0..1efd0fc467 100644 --- a/modules/compose/compose_api.go +++ b/modules/compose/compose_api.go @@ -328,9 +328,14 @@ func (d *dockerCompose) Up(ctx context.Context, opts ...StackUpOption) (err erro return err } + cfg, err := testcontainers.NewConfig() + if err != nil { + return fmt.Errorf("new config: %w", err) + } + var termSignals []chan bool var reaper *testcontainers.Reaper - if !d.provider.Config().Config.RyukDisabled { + if !cfg.RyukDisabled { // NewReaper is deprecated: we need to find a way to create the reaper for compose // bypassing the deprecation. reaper, err = testcontainers.NewReaper(ctx, testcontainers.SessionID(), d.provider, "") diff --git a/modules/compose/compose_api_test.go b/modules/compose/compose_api_test.go index 808433f513..d0acf3c3e9 100644 --- a/modules/compose/compose_api_test.go +++ b/modules/compose/compose_api_test.go @@ -196,7 +196,9 @@ func TestDockerComposeAPI_TestcontainersLabelsArePresent(t *testing.T) { func TestDockerComposeAPI_WithReaper(t *testing.T) { config.Reset() // reset the config using the internal method to avoid the sync.Once - tcConfig := config.Read() + tcConfig, err := testcontainers.NewConfig() + require.NoError(t, err) + if tcConfig.RyukDisabled { t.Skip("Ryuk is disabled, skipping test") } @@ -225,7 +227,9 @@ func TestDockerComposeAPI_WithReaper(t *testing.T) { func TestDockerComposeAPI_WithoutReaper(t *testing.T) { config.Reset() // reset the config using the internal method to avoid the sync.Once - tcConfig := config.Read() + tcConfig, err := testcontainers.NewConfig() + require.NoError(t, err) + if !tcConfig.RyukDisabled { t.Skip("Ryuk is enabled, skipping test") } diff --git a/provider.go b/provider.go index 66afb224ad..f8b030107f 100644 --- a/provider.go +++ b/provider.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/internal/core" ) @@ -87,6 +86,8 @@ type ContainerProvider interface { ReuseOrCreateContainer(context.Context, ContainerRequest) (Container, error) // reuses a container if it exists or creates a container without starting RunContainer(context.Context, ContainerRequest) (Container, error) // create a container and start it Health(context.Context) error + + // Deprecated: use [testcontainers.NewConfig] instead Config() TestcontainersConfig } @@ -125,7 +126,7 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error return nil, err } - cfg, err := config.Read() + cfg, err := NewConfig() if err != nil { return nil, fmt.Errorf("read config: %w", err) } diff --git a/reaper.go b/reaper.go index 6920f3e38b..8112e70015 100644 --- a/reaper.go +++ b/reaper.go @@ -60,6 +60,8 @@ var ( // The ContainerProvider interface should usually satisfy this as well, so it is pluggable type ReaperProvider interface { RunContainer(ctx context.Context, req ContainerRequest) (Container, error) + + // Deprecated: use [testcontainers.NewConfig] instead Config() TestcontainersConfig } @@ -260,7 +262,7 @@ func (r *reaperSpawner) retryError(err error) error { // // Safe for concurrent calls. func (r *reaperSpawner) reaper(ctx context.Context, sessionID string, provider ReaperProvider) (*Reaper, error) { - cfg, err := config.Read() + cfg, err := NewConfig() if err != nil { return nil, fmt.Errorf("read config: %w", err) } @@ -381,6 +383,7 @@ func (r *reaperSpawner) newReaper(ctx context.Context, sessionID string, provide dockerHostMount := core.MustExtractDockerSocket(ctx) port := r.port() + // TODO: change deprecated usage of Config() once we have more consistent test for the config and the reaper. tcConfig := provider.Config().Config req := ContainerRequest{ Image: config.ReaperDefaultImage, diff --git a/reaper_test.go b/reaper_test.go index 13751da942..1b5e9f17df 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -27,7 +27,7 @@ type mockReaperProvider struct { req ContainerRequest hostConfig *container.HostConfig endpointSettings map[string]*network.EndpointSettings - config TestcontainersConfig + config TestcontainersConfig // Deprecated: use [testcontainers.Config] instead } func newMockReaperProvider(cfg config.Config) *mockReaperProvider { @@ -62,6 +62,7 @@ func (m *mockReaperProvider) RunContainer(ctx context.Context, req ContainerRequ return nil, errExpected } +// Deprecated: use [testcontainers.NewConfig] instead func (m *mockReaperProvider) Config() TestcontainersConfig { return m.config } From 786d79c64ce3560ea3be2c298fcb71ab80d87dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 5 Dec 2024 02:28:43 +0100 Subject: [PATCH 47/50] chore: line separator --- internal/core/docker_host.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index eff06bf65a..d9a3aa1159 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -27,6 +27,7 @@ var ( errNoUnixSchema = errors.New("URL schema is not unix") errSocketNotFound = errors.New("socket not found") errSocketNotFoundInPath = errors.New("docker socket not found in " + DockerSocketPath) + // errTestcontainersHostNotSetInProperties this error is specific to Testcontainers errTestcontainersHostNotSetInProperties = errors.New("tc.host not set in ~/.testcontainers.properties") ) From 4bd9934a4ae4f6549711828c7c945be351350447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 5 Dec 2024 02:30:35 +0100 Subject: [PATCH 48/50] chore: do not export var --- internal/config/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 6914128a2d..187280dd41 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,8 +15,8 @@ const ( // ReaperDefaultImage is the default image used for Ryuk, the resource reaper. ReaperDefaultImage = "testcontainers/ryuk:0.11.0" - // TestcontainersProperties is the name of the properties file used to configure Testcontainers. - TestcontainersProperties = ".testcontainers.properties" + // testcontainersProperties is the name of the properties file used to configure Testcontainers. + testcontainersProperties = ".testcontainers.properties" ) var ( @@ -156,7 +156,7 @@ func read() (Config, error) { return applyEnvironmentConfiguration(config), nil } - tcProp := filepath.Join(home, TestcontainersProperties) + tcProp := filepath.Join(home, testcontainersProperties) // Init from a file, ignore if it doesn't exist, which is the case for most users. // The properties library will return the default values for the struct. props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) From 8fcd65479759738dffff4fb71264626a5e3eb5f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 5 Dec 2024 12:33:37 +0100 Subject: [PATCH 49/50] chore: bubble up error on environment configuration --- internal/config/config.go | 74 +++++++++++++++++++++++++--------- internal/config/config_test.go | 4 +- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 187280dd41..6394b9f7f3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "os" "path/filepath" @@ -22,6 +23,9 @@ const ( var ( tcConfig Config tcConfigOnce *sync.Once = new(sync.Once) + + // errEmptyValue is returned when the value is empty. Needed when parsing boolean values that are not set. + errEmptyValue = errors.New("empty value") ) // testcontainersConfig { @@ -95,10 +99,14 @@ type Config struct { // } -func applyEnvironmentConfiguration(config Config) Config { +func applyEnvironmentConfiguration(config Config) (Config, error) { ryukDisabledEnv := os.Getenv("TESTCONTAINERS_RYUK_DISABLED") - if parseBool(ryukDisabledEnv) { - config.RyukDisabled = ryukDisabledEnv == "true" + if value, err := parseBool(ryukDisabledEnv); err != nil { + if !errors.Is(err, errEmptyValue) { + return config, fmt.Errorf("invalid TESTCONTAINERS_RYUK_DISABLED environment variable: %s", ryukDisabledEnv) + } + } else { + config.RyukDisabled = value } hubImageNamePrefix := os.Getenv("TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX") @@ -107,26 +115,42 @@ func applyEnvironmentConfiguration(config Config) Config { } ryukPrivilegedEnv := os.Getenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED") - if parseBool(ryukPrivilegedEnv) { - config.RyukPrivileged = ryukPrivilegedEnv == "true" + if value, err := parseBool(ryukPrivilegedEnv); err != nil { + if !errors.Is(err, errEmptyValue) { + return config, fmt.Errorf("invalid TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED environment variable: %s", ryukPrivilegedEnv) + } + } else { + config.RyukPrivileged = value } ryukVerboseEnv := readTestcontainersEnv("RYUK_VERBOSE") - if parseBool(ryukVerboseEnv) { - config.RyukVerbose = ryukVerboseEnv == "true" + if value, err := parseBool(ryukVerboseEnv); err != nil { + if !errors.Is(err, errEmptyValue) { + return config, fmt.Errorf("invalid RYUK_VERBOSE environment variable: %s", ryukVerboseEnv) + } + } else { + config.RyukVerbose = value } ryukReconnectionTimeoutEnv := readTestcontainersEnv("RYUK_RECONNECTION_TIMEOUT") - if timeout, err := time.ParseDuration(ryukReconnectionTimeoutEnv); err == nil { - config.RyukReconnectionTimeout = timeout + if ryukReconnectionTimeoutEnv != "" { + if timeout, err := time.ParseDuration(ryukReconnectionTimeoutEnv); err != nil { + return config, fmt.Errorf("invalid RYUK_RECONNECTION_TIMEOUT environment variable: %w", err) + } else { + config.RyukReconnectionTimeout = timeout + } } ryukConnectionTimeoutEnv := readTestcontainersEnv("RYUK_CONNECTION_TIMEOUT") - if timeout, err := time.ParseDuration(ryukConnectionTimeoutEnv); err == nil { - config.RyukConnectionTimeout = timeout + if ryukConnectionTimeoutEnv != "" { + if timeout, err := time.ParseDuration(ryukConnectionTimeoutEnv); err != nil { + return config, fmt.Errorf("invalid RYUK_CONNECTION_TIMEOUT environment variable: %w", err) + } else { + config.RyukConnectionTimeout = timeout + } } - return config + return config, nil } // Read reads from testcontainers properties file, if it exists @@ -153,7 +177,7 @@ func read() (Config, error) { home, err := os.UserHomeDir() if err != nil { - return applyEnvironmentConfiguration(config), nil + return applyEnvironmentConfiguration(config) } tcProp := filepath.Join(home, testcontainersProperties) @@ -161,19 +185,31 @@ func read() (Config, error) { // The properties library will return the default values for the struct. props, err := properties.LoadFiles([]string{tcProp}, properties.UTF8, true) if err != nil { - return applyEnvironmentConfiguration(config), nil + return applyEnvironmentConfiguration(config) } if err := props.Decode(&config); err != nil { - return applyEnvironmentConfiguration(config), fmt.Errorf("invalid testcontainers properties file: %w", err) + cfg, envErr := applyEnvironmentConfiguration(config) + if envErr != nil { + return cfg, envErr + } + return cfg, fmt.Errorf("invalid testcontainers properties file: %w", err) } - return applyEnvironmentConfiguration(config), nil + return applyEnvironmentConfiguration(config) } -func parseBool(input string) bool { - _, err := strconv.ParseBool(input) - return err == nil +func parseBool(input string) (bool, error) { + if input == "" { + return false, errEmptyValue + } + + value, err := strconv.ParseBool(input) + if err != nil { + return false, fmt.Errorf("invalid boolean value: %w", err) + } + + return value, nil } // readTestcontainersEnv reads the environment variable with the given name. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ad3fa07889..98809d6851 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -465,7 +465,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_DISABLED": "foo", }, expected: defaultCfg, - wantErr: false, + wantErr: true, }, { name: "env-vars/properties/ryuk-container-privileged/env-var-does-not-win/wrong-boolean", @@ -474,7 +474,7 @@ func TestReadTCConfig(t *testing.T) { "TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "foo", }, expected: defaultCfg, - wantErr: false, + wantErr: true, }, { name: "properties/hub-image-name-prefix", From 13934dac9a54ee270532196c8cf8bccd8c407925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 5 Dec 2024 12:37:46 +0100 Subject: [PATCH 50/50] chore: rename test cases --- internal/config/config_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 98809d6851..26a01187e2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -32,7 +32,7 @@ func resetTestEnv(t *testing.T) { func TestReadConfig(t *testing.T) { resetTestEnv(t) - t.Run("Config is read just once", func(t *testing.T) { + t.Run("config/read-once", func(t *testing.T) { t.Cleanup(Reset) t.Setenv("HOME", "") @@ -63,7 +63,7 @@ func TestReadTCConfig(t *testing.T) { const defaultHubPrefix string = "registry.mycompany.com/mirror" - t.Run("HOME is not set", func(t *testing.T) { + t.Run("home/unset", func(t *testing.T) { t.Setenv("HOME", "") t.Setenv("USERPROFILE", "") // Windows support @@ -74,7 +74,7 @@ func TestReadTCConfig(t *testing.T) { assert.Equal(t, expected, config) }) - t.Run("HOME is not set - TESTCONTAINERS_ env is set", func(t *testing.T) { + t.Run("home/unset/testcontainers-env/set", func(t *testing.T) { t.Setenv("HOME", "") t.Setenv("USERPROFILE", "") // Windows support t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true") @@ -98,7 +98,7 @@ func TestReadTCConfig(t *testing.T) { assert.Equal(t, expected, config) }) - t.Run("HOME does not contain TC props file", func(t *testing.T) { + t.Run("home/set/no-testcontainers-props-file", func(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) t.Setenv("USERPROFILE", tmpDir) // Windows support @@ -115,7 +115,7 @@ func TestReadTCConfig(t *testing.T) { assert.Equal(t, expected, config) }) - t.Run("HOME does not contain TC props file - DOCKER_HOST env is set", func(t *testing.T) { + t.Run("home/set/no-testcontainers-props-file/docker-host-env/set", func(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) t.Setenv("USERPROFILE", tmpDir) // Windows support @@ -135,7 +135,7 @@ func TestReadTCConfig(t *testing.T) { assert.Equal(t, expected, config) }) - t.Run("HOME does not contain TC props file - TESTCONTAINERS_ env is set", func(t *testing.T) { + t.Run("home/set/no-testcontainers-props-file/testcontainers-ev/set", func(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) t.Setenv("USERPROFILE", tmpDir) // Windows support @@ -161,7 +161,7 @@ func TestReadTCConfig(t *testing.T) { assert.Equal(t, expected, config) }) - t.Run("HOME contains TC properties file", func(t *testing.T) { + t.Run("home/set/with-testcontainers-properties-file", func(t *testing.T) { defaultRyukConnectionTimeout := 60 * time.Second defaultRyukReconnectionTimeout := 10 * time.Second defaultCfg := Config{