Skip to content

Commit

Permalink
break: return error from Customize request option (#2267)
Browse files Browse the repository at this point in the history
* chore!: return error from Customize

Change ContainerCustomizer.Customize method to return an error so that
options can handle errors gracefully instead of relying on panic or just
a log entry, neither of which are user friendly.

Enable errcheck linter to ensure that errors that aren't handled are
reported.

Run go mod tidy on k3s and weaviate to allow tests to be run using go
1.22.

Run gofumpt on a few files to satisfy golangci-lint.

Fix direct comparison with http.ErrServerClosed flagged by errcheck.

Fixes #2266

BREAKING CHANGE: `ContainerCustomizer.Customize` now returns an error.

* fix(mongodb): captured loop variable

Fix captured loop variable in mongodb test reported by govet.

* fix(k3s): test formatting

Fix formatting in test file reported by gci during linting.

* chore: add missing Customize error returns

Add missing error returns for implementations of CustomizeRequestOption.

* chore: update modules with the new error API

* fix: use logger

* fix: update modulegen tests

* chore: fix lint

* chore: handle customise errors

* chore: update new host port access option

* docs: move new error API to the right doc

* docs: move customise request to the bottom

* docs: sync functional options

* chore: avoid panics in localstack

* chore: require no error in test

* chore: ignore error in copy API, using nolint:errcheck

* chore: convert log into error

---------

Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
  • Loading branch information
stevenh and mdelapenya committed Apr 26, 2024
1 parent fa5f304 commit 14d6929
Show file tree
Hide file tree
Showing 72 changed files with 618 additions and 264 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Expand Up @@ -7,6 +7,7 @@ linters:
- misspell
- nonamedreturns
- testifylint
- errcheck

linters-settings:
errorlint:
Expand Down
6 changes: 3 additions & 3 deletions docker_test.go
Expand Up @@ -2123,7 +2123,7 @@ func TestDockerProvider_BuildImage_Retries(t *testing.T) {
},
{
name: "no retry when not implemented by provider",
errReturned: errdefs.NotImplemented(errors.New("unkown method")),
errReturned: errdefs.NotImplemented(errors.New("unknown method")),
shouldRetry: false,
},
{
Expand Down Expand Up @@ -2169,7 +2169,7 @@ func TestDockerProvider_waitContainerCreation_retries(t *testing.T) {
},
{
name: "no retry when not implemented by provider",
errReturned: errdefs.NotImplemented(errors.New("unkown method")),
errReturned: errdefs.NotImplemented(errors.New("unknown method")),
shouldRetry: false,
},
{
Expand Down Expand Up @@ -2235,7 +2235,7 @@ func TestDockerProvider_attemptToPullImage_retries(t *testing.T) {
},
{
name: "no retry when not implemented by provider",
errReturned: errdefs.NotImplemented(errors.New("unkown method")),
errReturned: errdefs.NotImplemented(errors.New("unknown method")),
shouldRetry: false,
},
{
Expand Down
34 changes: 31 additions & 3 deletions docs/modules/index.md
Expand Up @@ -105,7 +105,22 @@ We are going to propose a set of steps to follow when adding types and methods t

- Make sure a public `Container` type exists for the module. This type has to use composition to embed the `testcontainers.Container` type, promoting all the methods from it.
- Make sure a `RunContainer` function exists and is public. This function is the entrypoint to the module and will define the initial values for a `testcontainers.GenericContainerRequest` struct, including the image, the default exposed ports, wait strategies, etc. Therefore, the function must initialise the container request with the default values.
- Define container options for the module leveraging the `testcontainers.ContainerCustomizer` interface, that has one single method: `Customize(req *GenericContainerRequest)`.
- Define container options for the module leveraging the `testcontainers.ContainerCustomizer` interface, that has one single method: `Customize(req *GenericContainerRequest) error`.

!!!warning
The interface definition for `ContainerCustomizer` was changed to allow errors to be correctly processed.
More specifically, the `Customize` method was changed from:

```go
Customize(req *GenericContainerRequest)
```

To:

```go
Customize(req *GenericContainerRequest) error
```

- We consider that a best practice for the options is define a function using the `With` prefix, that returns a function returning a modified `testcontainers.GenericContainerRequest` type. For that, the library already provides a `testcontainers.CustomizeRequestOption` type implementing the `ContainerCustomizer` interface, and we encourage you to use this type for creating your own customizer functions.
- At the same time, you could need to create your own container customizers for your module. Make sure they implement the `testcontainers.ContainerCustomizer` interface. Defining your own customizer functions is useful when you need to transfer a certain state that is not present at the `ContainerRequest` for the container, possibly using an intermediate Config struct.
- The options will be passed to the `RunContainer` function as variadic arguments after the Go context, and they will be processed right after defining the initial `testcontainers.GenericContainerRequest` struct using a for loop.
Expand All @@ -130,7 +145,9 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
}
...
for _, opt := range opts {
req = opt.Customize(&genericContainerReq)
if err := opt.Customize(&genericContainerReq); err != nil {
return nil, fmt.Errorf("customise: %w", err)
}

// If you need to transfer some state from the options to the container, you can do it here
if myCustomizer, ok := opt.(MyCustomizer); ok {
Expand Down Expand Up @@ -174,13 +191,24 @@ func (c *Container) ConnectionString(ctx context.Context) (string, error) {...}

In order to simplify the creation of the container for a given module, `Testcontainers for Go` provides a set of `testcontainers.CustomizeRequestOption` functions to customize the container request for the module. These options are:

- `testcontainers.CustomizeRequest`: a function that merges the default options with the ones provided by the user. Recommended for completely customizing the container request.
- `testcontainers.WithImage`: a function that sets the image for the container request.
- `testcontainers.WithImageSubstitutors`: a function that sets your own substitutions to the container images.
- `testcontainers.WithEnv`: a function that sets the environment variables for the container request.
- `testcontainers.WithHostPortAccess`: a function that enables the container to access a port that is already running in the host.
- `testcontainers.WithLogConsumers`: a function that sets the log consumers for the container request.
- `testcontainers.WithLogger`: a function that sets the logger for the container request.
- `testcontainers.WithWaitStrategy`: a function that sets the wait strategy for the container request.
- `testcontainers.WithWaitStrategyAndDeadline`: a function that sets the wait strategy for the container request with a deadline.
- `testcontainers.WithStartupCommand`: a function that sets the execution of a command when the container starts.
- `testcontainers.WithAfterReadyCommand`: a function that sets the execution of a command right after the container is ready (its wait strategy is satisfied).
- `testcontainers.WithNetwork`: a function that sets the network and the network aliases for the container request.
- `testcontainers.WithNewNetwork`: a function that sets the network aliases for a throw-away network for the container request.
- `testcontainers.WithConfigModifier`: a function that sets the config Docker type for the container request. Please see [Advanced Settings](../features/creating_container.md#advanced-settings) for more information.
- `testcontainers.WithEndpointSettingsModifier`: a function that sets the endpoint settings Docker type for the container request. Please see [Advanced Settings](../features/creating_container.md#advanced-settings) for more information.
- `testcontainers.WithHostConfigModifier`: a function that sets the host config Docker type for the container request. Please see [Advanced Settings](../features/creating_container.md#advanced-settings) for more information.
- `testcontainers.WithWaitStrategy`: a function that sets the wait strategy for the container request, adding all the passed wait strategies to the container request, using a `testcontainers.MultiStrategy` with 60 seconds of deadline. Please see [Wait strategies](../features/wait/multi.md) for more information.
- `testcontainers.WithWaitStrategyAndDeadline`: a function that sets the wait strategy for the container request, adding all the passed wait strategies to the container request, using a `testcontainers.MultiStrategy` with the passed deadline. Please see [Wait strategies](../features/wait/multi.md) for more information.
- `testcontainers.CustomizeRequest`: a function that merges the default options with the ones provided by the user. Recommended for completely customizing the container request.

### Update Go dependencies in the modules

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/localstack.md
Expand Up @@ -23,7 +23,7 @@ Running LocalStack as a stand-in for multiple AWS services during a test:
<!--/codeinclude-->

Environment variables listed in [Localstack's README](https://github.com/localstack/localstack#configurations) may be used to customize Localstack's configuration.
Use the `OverrideContainerRequest` option when creating the `LocalStackContainer` to apply configuration settings.
Use the `testcontainers.WithEnv` option when creating the `LocalStackContainer` to apply those variables.

## Module reference

Expand Down
3 changes: 2 additions & 1 deletion logger.go
Expand Up @@ -68,8 +68,9 @@ func (o LoggerOption) ApplyDockerTo(opts *DockerProviderOptions) {
}

// Customize implements ContainerCustomizer.
func (o LoggerOption) Customize(req *GenericContainerRequest) {
func (o LoggerOption) Customize(req *GenericContainerRequest) error {
req.Logger = o.logger
return nil
}

type testLogger struct {
Expand Down
2 changes: 1 addition & 1 deletion logger_test.go
Expand Up @@ -11,7 +11,7 @@ func TestWithLogger(t *testing.T) {
logOpt := WithLogger(logger)
t.Run("container", func(t *testing.T) {
var req GenericContainerRequest
logOpt.Customize(&req)
require.NoError(t, logOpt.Customize(&req))
require.Equal(t, logger, req.Logger)
})

Expand Down
4 changes: 3 additions & 1 deletion modulegen/_template/module.go.tmpl
Expand Up @@ -23,7 +23,9 @@ func {{ $entrypoint }}(ctx context.Context, opts ...testcontainers.ContainerCust
}

for _, opt := range opts {
opt.Customize(&genericContainerReq)
if err := opt.Customize(&genericContainerReq); err != nil {
return nil, fmt.Errorf("customize: %w", err)
}
}

container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
Expand Down
4 changes: 3 additions & 1 deletion modulegen/main_test.go
Expand Up @@ -428,7 +428,9 @@ func assertModuleContent(t *testing.T, module context.TestcontainersModule, exam
assert.Equal(t, data[13], "// "+entrypoint+" creates an instance of the "+exampleName+" container type")
assert.Equal(t, data[14], "func "+entrypoint+"(ctx context.Context, opts ...testcontainers.ContainerCustomizer) (*"+containerName+", error) {")
assert.Equal(t, data[16], "\t\tImage: \""+module.Image+"\",")
assert.Equal(t, data[33], "\treturn &"+containerName+"{Container: container}, nil")
assert.Equal(t, data[25], "\t\tif err := opt.Customize(&genericContainerReq); err != nil {")
assert.Equal(t, data[26], "\t\t\treturn nil, fmt.Errorf(\"customize: %w\", err)")
assert.Equal(t, data[35], "\treturn &"+containerName+"{Container: container}, nil")
}

// assert content GitHub workflow for the module
Expand Down
16 changes: 12 additions & 4 deletions modules/artemis/artemis.go
Expand Up @@ -49,16 +49,20 @@ func (c *Container) ConsoleURL(ctx context.Context) (string, error) {

// WithCredentials sets the administrator credentials. The default is artemis:artemis.
func WithCredentials(user, password string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
req.Env["ARTEMIS_USER"] = user
req.Env["ARTEMIS_PASSWORD"] = password

return nil
}
}

// WithAnonymousLogin enables anonymous logins.
func WithAnonymousLogin() testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
req.Env["ANONYMOUS_LOGIN"] = "true"

return nil
}
}

Expand All @@ -67,8 +71,10 @@ func WithAnonymousLogin() testcontainers.CustomizeRequestOption {
// Setting this value will override the default.
// See the documentation on `artemis create` for available options.
func WithExtraArgs(args string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
req.Env["EXTRA_ARGS"] = args

return nil
}
}

Expand All @@ -91,7 +97,9 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
}

for _, opt := range opts {
opt.Customize(&req)
if err := opt.Customize(&req); err != nil {
return nil, err
}
}

container, err := testcontainers.GenericContainer(ctx, req)
Expand Down
12 changes: 8 additions & 4 deletions modules/cassandra/cassandra.go
Expand Up @@ -41,19 +41,21 @@ func (c *CassandraContainer) ConnectionHost(ctx context.Context) (string, error)
// It will also set the "configFile" parameter to the path of the config file
// as a command line argument to the container.
func WithConfigFile(configFile string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
cf := testcontainers.ContainerFile{
HostFilePath: configFile,
ContainerFilePath: "/etc/cassandra/cassandra.yaml",
FileMode: 0o755,
}
req.Files = append(req.Files, cf)

return nil
}
}

// WithInitScripts sets the init cassandra queries to be run when the container starts
func WithInitScripts(scripts ...string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
var initScripts []testcontainers.ContainerFile
var execs []testcontainers.Executable
for _, script := range scripts {
Expand All @@ -68,7 +70,7 @@ func WithInitScripts(scripts ...string) testcontainers.CustomizeRequestOption {
}

req.Files = append(req.Files, initScripts...)
testcontainers.WithAfterReadyCommand(execs...)(req)
return testcontainers.WithAfterReadyCommand(execs...)(req)
}
}

Expand Down Expand Up @@ -100,7 +102,9 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
}

for _, opt := range opts {
opt.Customize(&genericContainerReq)
if err := opt.Customize(&genericContainerReq); err != nil {
return nil, err
}
}

container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
Expand Down
4 changes: 3 additions & 1 deletion modules/chroma/chroma.go
Expand Up @@ -33,7 +33,9 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
}

for _, opt := range opts {
opt.Customize(&genericContainerReq)
if err := opt.Customize(&genericContainerReq); err != nil {
return nil, err
}
}

container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
Expand Down
38 changes: 27 additions & 11 deletions modules/clickhouse/clickhouse.go
Expand Up @@ -101,34 +101,36 @@ func renderZookeeperConfig(settings ZookeeperOptions) ([]byte, error) {

// WithZookeeper pass a config to connect clickhouse with zookeeper and make clickhouse as cluster
func WithZookeeper(host, port string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
f, err := os.CreateTemp("", "clickhouse-tc-config-")
if err != nil {
panic(err)
return fmt.Errorf("temporary file: %w", err)
}

defer f.Close()

// write data to the temporary file
data, err := renderZookeeperConfig(ZookeeperOptions{Host: host, Port: port})
if err != nil {
panic(err)
return fmt.Errorf("zookeeper config: %w", err)
}
if _, err := f.Write(data); err != nil {
panic(err)
return fmt.Errorf("write zookeeper config: %w", err)
}
cf := testcontainers.ContainerFile{
HostFilePath: f.Name(),
ContainerFilePath: "/etc/clickhouse-server/config.d/zookeeper_config.xml",
FileMode: 0o755,
}
req.Files = append(req.Files, cf)

return nil
}
}

// WithInitScripts sets the init scripts to be run when the container starts
func WithInitScripts(scripts ...string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
initScripts := []testcontainers.ContainerFile{}
for _, script := range scripts {
cf := testcontainers.ContainerFile{
Expand All @@ -139,52 +141,62 @@ func WithInitScripts(scripts ...string) testcontainers.CustomizeRequestOption {
initScripts = append(initScripts, cf)
}
req.Files = append(req.Files, initScripts...)

return nil
}
}

// WithConfigFile sets the XML config file to be used for the clickhouse container
// It will also set the "configFile" parameter to the path of the config file
// as a command line argument to the container.
func WithConfigFile(configFile string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
cf := testcontainers.ContainerFile{
HostFilePath: configFile,
ContainerFilePath: "/etc/clickhouse-server/config.d/config.xml",
FileMode: 0o755,
}
req.Files = append(req.Files, cf)

return nil
}
}

// WithConfigFile sets the YAML config file to be used for the clickhouse container
// It will also set the "configFile" parameter to the path of the config file
// as a command line argument to the container.
func WithYamlConfigFile(configFile string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
cf := testcontainers.ContainerFile{
HostFilePath: configFile,
ContainerFilePath: "/etc/clickhouse-server/config.d/config.yaml",
FileMode: 0o755,
}
req.Files = append(req.Files, cf)

return nil
}
}

// WithDatabase sets the initial database to be created when the container starts
// It can be used to define a different name for the default database that is created when the image is first started.
// If it is not specified, then the default value("clickhouse") will be used.
func WithDatabase(dbName string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
req.Env["CLICKHOUSE_DB"] = dbName

return nil
}
}

// WithPassword sets the initial password of the user to be created when the container starts
// It is required for you to use the ClickHouse image. It must not be empty or undefined.
// This environment variable sets the password for ClickHouse.
func WithPassword(password string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
req.Env["CLICKHOUSE_PASSWORD"] = password

return nil
}
}

Expand All @@ -193,12 +205,14 @@ func WithPassword(password string) testcontainers.CustomizeRequestOption {
// It will create the specified user with superuser power.
// If it is not specified, then the default user of clickhouse will be used.
func WithUsername(user string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
return func(req *testcontainers.GenericContainerRequest) error {
if user == "" {
user = defaultUser
}

req.Env["CLICKHOUSE_USER"] = user

return nil
}
}

Expand All @@ -225,7 +239,9 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
}

for _, opt := range opts {
opt.Customize(&genericContainerReq)
if err := opt.Customize(&genericContainerReq); err != nil {
return nil, err
}
}

container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
Expand Down
4 changes: 3 additions & 1 deletion modules/cockroachdb/cockroachdb.go
Expand Up @@ -96,7 +96,9 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
if apply, ok := opt.(Option); ok {
apply(&o)
}
opt.Customize(&req)
if err := opt.Customize(&req); err != nil {
return nil, err
}
}

// modify request
Expand Down

0 comments on commit 14d6929

Please sign in to comment.