Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for LocalStack v2 #994

Merged
merged 7 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions modules/localstack/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ github.com/Microsoft/hcsshim v0.9.7 h1:mKNHW/Xvv1aFH87Jb6ERDzXTJTLPlmzfZ28VBFD/b
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/aws/aws-sdk-go v1.44.227 h1:HWNpINBu20yyfEXGHHSIsB955KUjWmZJETqnLIXizN4=
github.com/aws/aws-sdk-go v1.44.227/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
github.com/aws/aws-sdk-go-v2 v1.17.5/go.mod h1:uzbQtefpm44goOPmdKyAlXSNcwlRgF3ePWVW6EtJvvw=
github.com/aws/aws-sdk-go-v2 v1.17.7 h1:CLSjnhJSTSogvqUGhIC6LqFKATMRexcxLZ0i/Nzk9Eg=
github.com/aws/aws-sdk-go-v2 v1.17.7/go.mod h1:uzbQtefpm44goOPmdKyAlXSNcwlRgF3ePWVW6EtJvvw=
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.4.10 h1:dK82zF6kkPeCo8J1e+tGx4JdvDIQzj7ygIoLg8WMuGs=
Expand All @@ -20,10 +19,8 @@ github.com/aws/aws-sdk-go-v2/credentials v1.13.18 h1:EQMdtHwz0ILTW1hoP+EwuWhwCG1
github.com/aws/aws-sdk-go-v2/credentials v1.13.18/go.mod h1:vnwlwjIe+3XJPBYKu1et30ZPABG3VaXJYr8ryohpIyM=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.13.1 h1:gt57MN3liKiyGopcqgNzJb2+d9MJaKT/q1OksHNXVE4=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.13.1/go.mod h1:lfUx8puBRdM5lVVMQlwt2v+ofiG/X6Ms+dy0UkG/kXw=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.29/go.mod h1:Dip3sIGv485+xerzVv24emnjX5Sg88utCL8fwGmCeWg=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.31 h1:sJLYcS+eZn5EeNINGHSCRAwUJMFVqklwkH36Vbyai7M=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.31/go.mod h1:QT0BqUvX1Bh2ABdTGnjqEjvjzrCfIniM9Sc8zn9Yndo=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.23/go.mod h1:mr6c4cHC+S/MMkrjtSlG4QA36kOznDep+0fga5L/fGQ=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.25 h1:1mnRASEKnkqsntcxHaysxwgVoUUp5dkiB+l3llKnqyg=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.25/go.mod h1:zBHOPwhBc3FlQjQJE/D3IfPWiWaQmT06Vq9aNukDo0k=
github.com/aws/aws-sdk-go-v2/internal/ini v1.3.32 h1:p5luUImdIqywn6JpQsW3tq5GNOxKmOnEpybzPx+d1lk=
Expand All @@ -34,7 +31,6 @@ github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.9.11 h1:y2+VQzC
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.9.11/go.mod h1:iV4q2hsqtNECrfmlXyord9u4zyuFEJX9eLgLpSPzWA8=
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.1.26 h1:CeuSeq/8FnYpPtnuIeLQEEvDv9zUjneuYi8EghMBdwQ=
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.1.26/go.mod h1:2UqAAwMUXKeRkAHIlDJqvMVgOWkUi/AUXPk/YIe+Dg4=
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.23/go.mod h1:9uPh+Hrz2Vn6oMnQYiUi/zbh3ovbnQk19YKINkQny44=
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.25 h1:5LHn8JQ0qvjD9L9JhMtylnkcw7j05GDZqM9Oin6hpr0=
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.25/go.mod h1:/95IA+0lMnzW6XzqYJRpjjsAbKEORVeO0anQqjd2CNU=
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.14.0 h1:e2ooMhpYGhDnBfSvIyusvAwX7KexuZaHbQY2Dyei7VU=
Expand Down
44 changes: 36 additions & 8 deletions modules/localstack/localstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const defaultPort = 4566
const defaultRegion = "us-east-1"
const defaultVersion = "1.4.0"
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
const hostnameExternalEnvVar = "HOSTNAME_EXTERNAL"
const localstackHostEnvVar = "LOCALSTACK_HOST"

const defaultAccessKeyID = "accesskey"
const defaultSecretAccessKey = "secretkey"
Expand All @@ -40,6 +41,25 @@ func isLegacyMode(image string) bool {
return true
}

func isVersion2(image string) bool {
parts := strings.Split(image, ":")
version := parts[len(parts)-1]

if version == "latest" {
return true
}

if !strings.HasPrefix(version, "v") {
version = fmt.Sprintf("v%s", version)
}

if semver.IsValid(version) {
return semver.Compare(version, "v2.0") > 0 // version < v0.11
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved
}

return true
}

// StartContainer creates an instance of the LocalStack container type, being possible to pass a custom request and options:
// - overrideReq: a function that can be used to override the default container request, usually used to set the image version, environment variables for localstack, etc.
func StartContainer(ctx context.Context, overrideReq OverrideContainerRequestOption) (*LocalStackContainer, error) {
Expand Down Expand Up @@ -67,11 +87,19 @@ func StartContainer(ctx context.Context, overrideReq OverrideContainerRequestOpt
return nil, fmt.Errorf("version=%s. Testcontainers for Go does not support running LocalStack in legacy mode. Please use a version >= 0.11.0", localStackReq.Image)
}

hostnameExternalReason, err := configureDockerHost(&localStackReq)
if err != nil {
return nil, err
if isVersion2(localStackReq.Image) {
hostnameExternalReason, err := configureDockerHost(&localStackReq, localstackHostEnvVar)
if err != nil {
return nil, err
}
fmt.Printf("Setting %s to %s (%s)\n", localstackHostEnvVar, req.Env[localstackHostEnvVar], hostnameExternalReason)
} else {
hostnameExternalReason, err := configureDockerHost(&localStackReq, hostnameExternalEnvVar)
if err != nil {
return nil, err
}
fmt.Printf("Setting %s to %s (%s)\n", hostnameExternalEnvVar, req.Env[hostnameExternalEnvVar], hostnameExternalReason)
}
fmt.Printf("Setting %s to %s (%s)\n", hostnameExternalEnvVar, req.Env[hostnameExternalEnvVar], hostnameExternalReason)

container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ContainerRequest: localStackReq.ContainerRequest,
Expand All @@ -87,11 +115,11 @@ func StartContainer(ctx context.Context, overrideReq OverrideContainerRequestOpt
return c, nil
}

func configureDockerHost(req *LocalStackContainerRequest) (reason string, err error) {
func configureDockerHost(req *LocalStackContainerRequest, envVar string) (reason string, err error) {
err = nil
reason = ""

if _, ok := req.Env[hostnameExternalEnvVar]; ok {
if _, ok := req.Env[envVar]; ok {
reason = "explicitly as environment variable"
return
}
Expand All @@ -101,7 +129,7 @@ func configureDockerHost(req *LocalStackContainerRequest) (reason string, err er
if len(req.Networks) > 0 && len(req.NetworkAliases) > 0 && len(req.NetworkAliases[req.Networks[0]]) > 0 {
alias := req.NetworkAliases[req.Networks[0]][len(req.NetworkAliases[req.Networks[0]])-1]

req.Env[hostnameExternalEnvVar] = alias
req.Env[envVar] = alias
reason = "to match last network alias on container with non-default network"
return
}
Expand All @@ -119,7 +147,7 @@ func configureDockerHost(req *LocalStackContainerRequest) (reason string, err er
return
}

req.Env[hostnameExternalEnvVar] = daemonHost
req.Env[envVar] = daemonHost
reason = "to match host-routable address for container"

return
Expand Down
137 changes: 102 additions & 35 deletions modules/localstack/localstack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package localstack
import (
"context"
"fmt"
"io"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -20,52 +22,60 @@ func generateContainerRequest() *LocalStackContainerRequest {
}

func TestConfigureDockerHost(t *testing.T) {
tests := []struct {
envVar string
}{
{hostnameExternalEnvVar},
{localstackHostEnvVar},
}

t.Run("HOSTNAME_EXTERNAL variable is passed as part of the request", func(t *testing.T) {
req := generateContainerRequest()
for _, tt := range tests {
t.Run("HOSTNAME_EXTERNAL variable is passed as part of the request", func(t *testing.T) {
req := generateContainerRequest()

req.Env[hostnameExternalEnvVar] = "foo"
req.Env[tt.envVar] = "foo"

reason, err := configureDockerHost(req)
assert.Nil(t, err)
assert.Equal(t, "explicitly as environment variable", reason)
})
reason, err := configureDockerHost(req, tt.envVar)
assert.Nil(t, err)
assert.Equal(t, "explicitly as environment variable", reason)
})

t.Run("HOSTNAME_EXTERNAL matches the last network alias on a container with non-default network", func(t *testing.T) {
req := generateContainerRequest()
t.Run("HOSTNAME_EXTERNAL matches the last network alias on a container with non-default network", func(t *testing.T) {
req := generateContainerRequest()

req.Networks = []string{"foo", "bar", "baaz"}
req.NetworkAliases = map[string][]string{
"foo": {"foo0", "foo1", "foo2", "foo3"},
"bar": {"bar0", "bar1", "bar2", "bar3"},
"baaz": {"baaz0", "baaz1", "baaz2", "baaz3"},
}
req.Networks = []string{"foo", "bar", "baaz"}
req.NetworkAliases = map[string][]string{
"foo": {"foo0", "foo1", "foo2", "foo3"},
"bar": {"bar0", "bar1", "bar2", "bar3"},
"baaz": {"baaz0", "baaz1", "baaz2", "baaz3"},
}

reason, err := configureDockerHost(req)
assert.Nil(t, err)
assert.Equal(t, "to match last network alias on container with non-default network", reason)
assert.Equal(t, "foo3", req.Env[hostnameExternalEnvVar])
})
reason, err := configureDockerHost(req, tt.envVar)
assert.Nil(t, err)
assert.Equal(t, "to match last network alias on container with non-default network", reason)
assert.Equal(t, "foo3", req.Env[tt.envVar])
})

t.Run("HOSTNAME_EXTERNAL matches the daemon host because there are no aliases", func(t *testing.T) {
dockerProvider, err := testcontainers.NewDockerProvider()
assert.Nil(t, err)
defer dockerProvider.Close()
t.Run("HOSTNAME_EXTERNAL matches the daemon host because there are no aliases", func(t *testing.T) {
dockerProvider, err := testcontainers.NewDockerProvider()
assert.Nil(t, err)
defer dockerProvider.Close()

// because the daemon host could be a remote one, we need to get it from the provider
expectedDaemonHost, err := dockerProvider.DaemonHost(context.Background())
assert.Nil(t, err)
// because the daemon host could be a remote one, we need to get it from the provider
expectedDaemonHost, err := dockerProvider.DaemonHost(context.Background())
assert.Nil(t, err)

req := generateContainerRequest()
req := generateContainerRequest()

req.Networks = []string{"foo", "bar", "baaz"}
req.NetworkAliases = map[string][]string{}
req.Networks = []string{"foo", "bar", "baaz"}
req.NetworkAliases = map[string][]string{}

reason, err := configureDockerHost(req)
assert.Nil(t, err)
assert.Equal(t, "to match host-routable address for container", reason)
assert.Equal(t, expectedDaemonHost, req.Env[hostnameExternalEnvVar])
})
reason, err := configureDockerHost(req, tt.envVar)
assert.Nil(t, err)
assert.Equal(t, "to match host-routable address for container", reason)
assert.Equal(t, expectedDaemonHost, req.Env[tt.envVar])
})
}
}

func TestIsLegacyMode(t *testing.T) {
Expand Down Expand Up @@ -165,3 +175,60 @@ func TestStartWithNetwork(t *testing.T) {
require.Equal(t, 1, len(networks))
require.Equal(t, "localstack-network", networks[0])
}

func TestStartV2WithNetwork(t *testing.T) {
ctx := context.Background()

nw, err := testcontainers.GenericNetwork(ctx, testcontainers.GenericNetworkRequest{
NetworkRequest: testcontainers.NetworkRequest{
Name: "localstack-network",
},
})
require.Nil(t, err)
assert.NotNil(t, nw)

localstack, err := StartContainer(
ctx,
OverrideContainerRequest(testcontainers.ContainerRequest{
Image: "localstack/localstack:latest",
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved
Networks: []string{"localstack-network"},
NetworkAliases: map[string][]string{"localstack-network": {"localstack"}},
}),
)
require.Nil(t, err)
assert.NotNil(t, localstack)

cli, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ContainerRequest: testcontainers.ContainerRequest{
Image: "amazon/aws-cli:2.7.27",
Networks: []string{"localstack-network"},
Entrypoint: []string{"tail"},
Cmd: []string{"-f", "/dev/null"},
Env: map[string]string{
"AWS_ACCESS_KEY_ID": "accesskey",
"AWS_SECRET_ACCESS_KEY": "secretkey",
"AWS_REGION": "eu-west-1",
},
},
Started: true,
})
require.Nil(t, err)
assert.NotNil(t, cli)

c, output, err := cli.Exec(ctx, []string{"/usr/local/bin/aws", "sqs", "create-queue", "--queue-name", "baz", "--region", "eu-west-1",
"--endpoint-url", "http://localstack:4566", "--no-verify-ssl"})
if err != nil {
t.Fatal(err)
}
if c != 0 {
t.Fatalf("AWS CLI command was executed, expected return code 1, got %v", c)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are currently failing here. I'm debugging them locally atm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checked that it fails locally. Will try to understand why and propose a fix

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm able to reproduce it but under certain circumstances: debugging and waiting too much while debugging.

Have you though about adding a waitStrategy with the command to the cli container? Then we can waitFor the result after a number of retries/timeout

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eddumelendez I'm sending a PR to provide wait.ForExec(...).WithResponseCodeMatcher(...), in the same way that we provide it for wait.ForHTTP

That will simplify this PR :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting! the same config works in tc-java. I was wondering if the network name was making those tests flaky but look like is not. So, if I understood correctly the suggestion is to add the wait strategies as part of the container request, right? Although, the check is against the cli output. I think we can store the result in a file and check it later but doesn't seem to simplify the pr

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still create the CLI container, but instead of directly using the Exec method of the container, move it to a wait.ForExec(...).WithResponseMatcher(). If the container is not created without error, then the tests should fail

Does it sound good to you?

Copy link
Member Author

@eddumelendez eddumelendez Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have the same issue 🤔 (locally)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Could it be caused by the startup timeout not being enough depending on the machine?

I've seen some issues regarding that in the couchbase module in a non-deterministic manner. Restarting its CI build fixes it. Could it be the case for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it could be.

}

content, err := io.ReadAll(output)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(content), "http://localstack:4566") {
t.Fatal("Created queue should use LOCALSTACK_HOSTNAME env var")
}
}