-
Notifications
You must be signed in to change notification settings - Fork 0
Forward host environment variables to the emulator container #161
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,13 @@ package integration_test | |
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "fmt" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "path/filepath" | ||
| "strconv" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/docker/docker/api/types/container" | ||
| "github.com/docker/go-connections/nat" | ||
| "github.com/localstack/lstk/test/integration/env" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
@@ -164,66 +159,36 @@ func TestStartCommandSetsUpContainerCorrectly(t *testing.T) { | |
| assert.Equal(t, containerName, envVars["MAIN_CONTAINER_NAME"]) | ||
| assert.NotEmpty(t, envVars["LOCALSTACK_AUTH_TOKEN"]) | ||
| }) | ||
| } | ||
|
|
||
| t.Run("docker socket mount", func(t *testing.T) { | ||
| if !strings.HasPrefix(dockerClient.DaemonHost(), "unix://") { | ||
| t.Skip("Docker daemon is not reachable via unix socket") | ||
| } | ||
|
|
||
| assert.True(t, hasBindTarget(inspect.HostConfig.Binds, "/var/run/docker.sock"), | ||
| "expected Docker socket bind mount to /var/run/docker.sock, got: %v", inspect.HostConfig.Binds) | ||
|
|
||
| envVars := containerEnvToMap(inspect.Config.Env) | ||
| assert.Equal(t, "unix:///var/run/docker.sock", envVars["DOCKER_HOST"]) | ||
| }) | ||
| func TestStartCommandPassesCIAndLocalStackEnvVars(t *testing.T) { | ||
| requireDocker(t) | ||
| _ = env.Require(t, env.AuthToken) | ||
|
|
||
| t.Run("service port range", func(t *testing.T) { | ||
| for p := 4510; p <= 4559; p++ { | ||
| port := nat.Port(fmt.Sprintf("%d/tcp", p)) | ||
| bindings := inspect.HostConfig.PortBindings[port] | ||
| if assert.NotEmpty(t, bindings, "port %d/tcp should be bound", p) { | ||
| assert.Equal(t, strconv.Itoa(p), bindings[0].HostPort) | ||
| } | ||
| } | ||
| }) | ||
| cleanup() | ||
| t.Cleanup(cleanup) | ||
|
|
||
| t.Run("main port", func(t *testing.T) { | ||
| mainBindings := inspect.HostConfig.PortBindings[nat.Port("4566/tcp")] | ||
| require.NotEmpty(t, mainBindings, "port 4566/tcp should be bound") | ||
| assert.Equal(t, "4566", mainBindings[0].HostPort) | ||
| }) | ||
| t.Setenv("CI", "true") | ||
| t.Setenv("LOCALSTACK_DISABLE_EVENTS", "1") | ||
| t.Setenv("LOCALSTACK_AUTH_TOKEN", "host-token") | ||
|
Comment on lines
+171
to
+173
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: we should not be setting env vars like this, instead when running the lstk command: With |
||
|
|
||
| t.Run("https port", func(t *testing.T) { | ||
| httpsBindings := inspect.HostConfig.PortBindings[nat.Port("443/tcp")] | ||
| require.NotEmpty(t, httpsBindings, "port 443/tcp should be bound") | ||
| assert.Equal(t, "443", httpsBindings[0].HostPort) | ||
| }) | ||
| mockServer := createMockLicenseServer(true) | ||
| defer mockServer.Close() | ||
|
|
||
| t.Run("volume mount", func(t *testing.T) { | ||
| assert.True(t, hasBindTarget(inspect.HostConfig.Binds, "/var/lib/localstack"), | ||
| "expected volume bind mount to /var/lib/localstack, got: %v", inspect.HostConfig.Binds) | ||
| }) | ||
| ctx := testContext(t) | ||
| _, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL), "start") | ||
| require.NoError(t, err, "lstk start failed: %s", stderr) | ||
| requireExitCode(t, 0, err) | ||
|
|
||
| t.Run("http health endpoint", func(t *testing.T) { | ||
| resp, err := http.Get("http://localhost.localstack.cloud:4566/_localstack/health") | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
| }) | ||
| inspect, err := dockerClient.ContainerInspect(ctx, containerName) | ||
| require.NoError(t, err, "failed to inspect container") | ||
| require.True(t, inspect.State.Running) | ||
|
|
||
| t.Run("https health endpoint", func(t *testing.T) { | ||
| // LS certificate is not in system trust store | ||
| // But cert validity is out of scope here: use InsecureSkipVerify | ||
| client := &http.Client{ | ||
| Transport: &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, | ||
| }, | ||
| } | ||
| resp, err := client.Get("https://localhost.localstack.cloud/_localstack/health") | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
| }) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: why are we removing all these subtests?
|
||
| envVars := containerEnvToMap(inspect.Config.Env) | ||
| assert.Equal(t, "true", envVars["CI"]) | ||
| assert.Equal(t, "1", envVars["LOCALSTACK_DISABLE_EVENTS"]) | ||
| assert.NotEmpty(t, envVars["LOCALSTACK_AUTH_TOKEN"]) | ||
| assert.NotEqual(t, "host-token", envVars["LOCALSTACK_AUTH_TOKEN"], "host LOCALSTACK_AUTH_TOKEN should not be passed through") | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: can we cover the new functionality with an integration test? It could set
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made an attempt to resolve in 2d8e0ec. |
||
|
|
||
| // containerEnvToMap converts a Docker container's []string env to a map. | ||
|
|
@@ -236,17 +201,6 @@ func containerEnvToMap(envList []string) map[string]string { | |
| return m | ||
| } | ||
|
|
||
| // hasBindTarget checks if any bind mount targets the given container path. | ||
| func hasBindTarget(binds []string, containerPath string) bool { | ||
| for _, b := range binds { | ||
| parts := strings.Split(b, ":") | ||
| if len(parts) >= 2 && parts[1] == containerPath { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func cleanup() { | ||
| ctx := context.Background() | ||
| _ = dockerClient.ContainerStop(ctx, containerName, container.StopOptions{}) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: I don't see the point of these unit tests. They are duplicating the logic of the code under test. I think we can delete and rely solely on the new integration tests.