From ea8341865dd689b75ab3d493d10a09b45d6dace7 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Wed, 15 Jun 2022 16:24:07 -0400 Subject: [PATCH 1/5] e2e: isolate test command env from system env When running Docker / Compose commands, do NOT inherit the system environment to ensure that the tests are reproducible regardless of host settings. Additionally, per-command environment overrides are provided to the command instead of using `os.SetEnv`, as this is not safe when running tests in parallel (`testing.T::SetEnv` will actually error if used in this way!) Signed-off-by: Milas Bowman --- pkg/e2e/compose_build_test.go | 6 ---- pkg/e2e/compose_environment_test.go | 28 ++++++--------- pkg/e2e/framework.go | 53 +++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 39 deletions(-) diff --git a/pkg/e2e/compose_build_test.go b/pkg/e2e/compose_build_test.go index 9483a10e..68df43f8 100644 --- a/pkg/e2e/compose_build_test.go +++ b/pkg/e2e/compose_build_test.go @@ -18,7 +18,6 @@ package e2e import ( "net/http" - "os" "strings" "testing" "time" @@ -81,11 +80,6 @@ func TestLocalComposeBuild(t *testing.T) { }) t.Run("build failed with ssh default value", func(t *testing.T) { - //unset SSH_AUTH_SOCK to be sure we don't have a default value for the SSH Agent - defaultSSHAUTHSOCK := os.Getenv("SSH_AUTH_SOCK") - os.Unsetenv("SSH_AUTH_SOCK") //nolint:errcheck - defer os.Setenv("SSH_AUTH_SOCK", defaultSSHAUTHSOCK) //nolint:errcheck - res := c.RunDockerComposeCmdNoCheck(t, "--project-directory", "fixtures/build-test", "build", "--ssh", "") res.Assert(t, icmd.Expected{ ExitCode: 1, diff --git a/pkg/e2e/compose_environment_test.go b/pkg/e2e/compose_environment_test.go index e70a2536..60ec3d79 100644 --- a/pkg/e2e/compose_environment_test.go +++ b/pkg/e2e/compose_environment_test.go @@ -17,7 +17,6 @@ package e2e import ( - "os" "strings" "testing" @@ -43,13 +42,10 @@ func TestEnvPriority(t *testing.T) { // 4. Dockerfile // 5. Variable is not defined t.Run("compose file priority", func(t *testing.T) { - os.Setenv("WHEREAMI", "shell") //nolint:errcheck - defer os.Unsetenv("WHEREAMI") //nolint:errcheck - - res := c.RunDockerComposeCmd(t, "-f", "./fixtures/environment/env-priority/compose-with-env.yaml", - "--project-directory", projectDir, "--env-file", "./fixtures/environment/env-priority/.env.override", - "run", "--rm", "-e", "WHEREAMI", "env-compose-priority") - + cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/environment/env-priority/compose-with-env.yaml", + "--project-directory", projectDir, "--env-file", "./fixtures/environment/env-priority/.env.override", "run", + "--rm", "-e", "WHEREAMI", "env-compose-priority") + res := icmd.RunCmd(cmd, icmd.WithEnv("WHEREAMI=shell")) assert.Equal(t, strings.TrimSpace(res.Stdout()), "Compose File") }) @@ -60,12 +56,10 @@ func TestEnvPriority(t *testing.T) { // 4. Dockerfile // 5. Variable is not defined t.Run("shell priority", func(t *testing.T) { - os.Setenv("WHEREAMI", "shell") //nolint:errcheck - defer os.Unsetenv("WHEREAMI") //nolint:errcheck - - res := c.RunDockerComposeCmd(t, "-f", "./fixtures/environment/env-priority/compose.yaml", - "--project-directory", projectDir, "--env-file", "./fixtures/environment/env-priority/.env.override", - "run", "--rm", "-e", "WHEREAMI", "env-compose-priority") + cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/environment/env-priority/compose.yaml", "--project-directory", + projectDir, "--env-file", "./fixtures/environment/env-priority/.env.override", "run", "--rm", "-e", + "WHEREAMI", "env-compose-priority") + res := icmd.RunCmd(cmd, icmd.WithEnv("WHEREAMI=shell")) assert.Equal(t, strings.TrimSpace(res.Stdout()), "shell") }) @@ -137,11 +131,9 @@ func TestEnvInterpolation(t *testing.T) { // 4. Dockerfile // 5. Variable is not defined t.Run("shell priority from run command", func(t *testing.T) { - os.Setenv("WHEREAMI", "shell") //nolint:errcheck - defer os.Unsetenv("WHEREAMI") //nolint:errcheck - res := c.RunDockerComposeCmd(t, "-f", "./fixtures/environment/env-interpolation/compose.yaml", + cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/environment/env-interpolation/compose.yaml", "--project-directory", projectDir, "config") - + res := icmd.RunCmd(cmd, icmd.WithEnv("WHEREAMI=shell")) res.Assert(t, icmd.Expected{Out: `IMAGE: default_env:shell`}) }) } diff --git a/pkg/e2e/framework.go b/pkg/e2e/framework.go index fabf90cd..37f5483a 100644 --- a/pkg/e2e/framework.go +++ b/pkg/e2e/framework.go @@ -30,6 +30,7 @@ import ( "time" "github.com/pkg/errors" + "github.com/stretchr/testify/require" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/icmd" @@ -154,28 +155,29 @@ func CopyFile(sourceFile string, destinationFile string) error { return err } +// BaseEnvironment provides the minimal environment variables used across all +// Docker / Compose commands. +func (c *CLI) BaseEnvironment() []string { + return []string{ + "DOCKER_CONFIG=" + c.ConfigDir, + "KUBECONFIG=invalid", + } +} + // NewCmd creates a cmd object configured with the test environment set func (c *CLI) NewCmd(command string, args ...string) icmd.Cmd { - env := append(os.Environ(), - "DOCKER_CONFIG="+c.ConfigDir, - "KUBECONFIG=invalid", - ) return icmd.Cmd{ Command: append([]string{command}, args...), - Env: env, + Env: c.BaseEnvironment(), } } // NewCmdWithEnv creates a cmd object configured with the test environment set with additional env vars func (c *CLI) NewCmdWithEnv(envvars []string, command string, args ...string) icmd.Cmd { - env := append(os.Environ(), - append(envvars, - "DOCKER_CONFIG="+c.ConfigDir, - "KUBECONFIG=invalid")..., - ) + cmdEnv := append(c.BaseEnvironment(), envvars...) return icmd.Cmd{ Command: append([]string{command}, args...), - Env: env, + Env: cmdEnv, } } @@ -234,13 +236,34 @@ func (c *CLI) RunDockerComposeCmd(t testing.TB, args ...string) *icmd.Result { // RunDockerComposeCmdNoCheck runs a docker compose command, don't presume of any expectation and returns a result func (c *CLI) RunDockerComposeCmdNoCheck(t testing.TB, args ...string) *icmd.Result { + return icmd.RunCmd(c.NewDockerComposeCmd(t, args...)) +} + +// NewDockerComposeCmd creates a command object for Compose, either in plugin +// or standalone mode (based on build tags). +func (c *CLI) NewDockerComposeCmd(t testing.TB, args ...string) icmd.Cmd { + t.Helper() if composeStandaloneMode { - composeBinary, err := findExecutable(DockerComposeExecutableName, []string{"../../bin", "../../../bin"}) - assert.NilError(t, err) - return icmd.RunCmd(c.NewCmd(composeBinary, args...)) + return c.NewCmd(ComposeStandalonePath(t), args...) } args = append([]string{"compose"}, args...) - return icmd.RunCmd(c.NewCmd(DockerExecutableName, args...)) + return c.NewCmd(DockerExecutableName, args...) +} + +// ComposeStandalonePath returns the path to the locally-built Compose +// standalone binary from the repo. +// +// This function will fail the test immediately if invoked when not running +// in standalone test mode. +func ComposeStandalonePath(t testing.TB) string { + t.Helper() + if !composeStandaloneMode { + require.Fail(t, "Not running in standalone mode") + } + composeBinary, err := findExecutable(DockerComposeExecutableName, []string{"../../bin", "../../../bin"}) + require.NoError(t, err, "Could not find standalone Compose binary (%q)", + DockerComposeExecutableName) + return composeBinary } // StdoutContains returns a predicate on command result expecting a string in stdout From 1c41df8f56d61b218a401fc77f01445be5151580 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Thu, 16 Jun 2022 08:44:18 -0400 Subject: [PATCH 2/5] e2e: robustness changes for ddev test The most important change here is to ensure that the correct Compose standalone binary is used by `ddev`. Since it invokes Compose itself, we need to ensure that `PATH` is set appropriately such that it finds the binary we want to test rather than something from the system. As part of this, the rest of the environment has been isolated, which should make the test more reliable, and avoids polluting `~/.ddev` with test artifacts by using a tmpdir as `HOME` for the test instead of the user's real home folder. Signed-off-by: Milas Bowman --- pkg/e2e/ddev_test.go | 59 +++++++++++++++++++++++++++----------------- pkg/e2e/framework.go | 55 ++++++++++++++++++++++++++++++++--------- 2 files changed, 79 insertions(+), 35 deletions(-) diff --git a/pkg/e2e/ddev_test.go b/pkg/e2e/ddev_test.go index 37293afb..63217f95 100644 --- a/pkg/e2e/ddev_test.go +++ b/pkg/e2e/ddev_test.go @@ -19,11 +19,13 @@ package e2e import ( "fmt" "os" + "os/exec" "path/filepath" "runtime" "strings" "testing" + "github.com/stretchr/testify/require" "gotest.tools/v3/assert" ) @@ -31,26 +33,38 @@ const ddevVersion = "v1.19.1" func TestComposeRunDdev(t *testing.T) { if !composeStandaloneMode { - t.Skip("Not running on standalone mode.") + t.Skip("Not running in plugin mode - ddev only supports invoking standalone `docker-compose`") } if runtime.GOOS == "windows" { t.Skip("Running on Windows. Skipping...") } - _ = os.Setenv("DDEV_DEBUG", "true") - c := NewParallelCLI(t) - dir, err := os.MkdirTemp("", t.Name()+"-") - assert.NilError(t, err) + // ddev shells out to `docker` and `docker-compose` (standalone), so a + // temporary directory is created with symlinks to system Docker and the + // locally-built standalone Compose binary to use as PATH + pathDir := t.TempDir() + dockerBin, err := exec.LookPath(DockerExecutableName) + require.NoError(t, err, "Could not find %q in path", DockerExecutableName) + require.NoError(t, os.Symlink(dockerBin, filepath.Join(pathDir, DockerExecutableName)), + "Could not create %q symlink", DockerExecutableName) - // ddev needs to be able to find mkcert to figure out where certs are. - _ = os.Setenv("PATH", fmt.Sprintf("%s:%s", os.Getenv("PATH"), dir)) + composeBin := ComposeStandalonePath(t) + require.NoError(t, os.Symlink(composeBin, filepath.Join(pathDir, DockerComposeExecutableName)), + "Could not create %q symlink", DockerComposeExecutableName) - siteName := filepath.Base(dir) + c := NewCLI(t, WithEnv( + "DDEV_DEBUG=true", + fmt.Sprintf("HOME=%s", t.TempDir()), + fmt.Sprintf("USER=%s", os.Getenv("USER")), + fmt.Sprintf("PATH=%s", pathDir), + )) + + ddevDir := t.TempDir() + siteName := filepath.Base(ddevDir) t.Cleanup(func() { - _ = c.RunCmdInDir(t, dir, "./ddev", "delete", "-Oy") - _ = c.RunCmdInDir(t, dir, "./ddev", "poweroff") - _ = os.RemoveAll(dir) + _ = c.RunCmdInDir(t, ddevDir, "./ddev", "delete", "-Oy") + _ = c.RunCmdInDir(t, ddevDir, "./ddev", "poweroff") }) osName := "linux" @@ -59,27 +73,26 @@ func TestComposeRunDdev(t *testing.T) { } compressedFilename := fmt.Sprintf("ddev_%s-%s.%s.tar.gz", osName, runtime.GOARCH, ddevVersion) - c.RunCmdInDir(t, dir, "curl", "-LO", - fmt.Sprintf("https://github.com/drud/ddev/releases/download/%s/%s", - ddevVersion, - compressedFilename)) + c.RunCmdInDir(t, ddevDir, "curl", "-LO", fmt.Sprintf("https://github.com/drud/ddev/releases/download/%s/%s", + ddevVersion, + compressedFilename)) - c.RunCmdInDir(t, dir, "tar", "-xzf", compressedFilename) + c.RunCmdInDir(t, ddevDir, "tar", "-xzf", compressedFilename) // Create a simple index.php we can test against. - c.RunCmdInDir(t, dir, "sh", "-c", "echo 'index.php") + c.RunCmdInDir(t, ddevDir, "sh", "-c", "echo 'index.php") - c.RunCmdInDir(t, dir, "./ddev", "config", "--auto") - c.RunCmdInDir(t, dir, "./ddev", "config", "global", "--use-docker-compose-from-path") - vRes := c.RunCmdInDir(t, dir, "./ddev", "version") + c.RunCmdInDir(t, ddevDir, "./ddev", "config", "--auto") + c.RunCmdInDir(t, ddevDir, "./ddev", "config", "global", "--use-docker-compose-from-path") + vRes := c.RunCmdInDir(t, ddevDir, "./ddev", "version") out := vRes.Stdout() fmt.Printf("ddev version: %s\n", out) - c.RunCmdInDir(t, dir, "./ddev", "poweroff") + c.RunCmdInDir(t, ddevDir, "./ddev", "poweroff") - c.RunCmdInDir(t, dir, "./ddev", "start", "-y") + c.RunCmdInDir(t, ddevDir, "./ddev", "start", "-y") - curlRes := c.RunCmdInDir(t, dir, "curl", "-sSL", fmt.Sprintf("http://%s.ddev.site", siteName)) + curlRes := c.RunCmdInDir(t, ddevDir, "curl", "-sSL", fmt.Sprintf("http://%s.ddev.site", siteName)) out = curlRes.Stdout() fmt.Println(out) assert.Assert(t, strings.Contains(out, "ddev is working"), "Could not start project") diff --git a/pkg/e2e/framework.go b/pkg/e2e/framework.go index 37f5483a..49874e95 100644 --- a/pkg/e2e/framework.go +++ b/pkg/e2e/framework.go @@ -32,7 +32,6 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/require" "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/icmd" "gotest.tools/v3/poll" @@ -61,18 +60,50 @@ func init() { // CLI is used to wrap the CLI for end to end testing type CLI struct { ConfigDir string + + env []string } -// NewParallelCLI returns a configured CLI with t.Parallel() set -func NewParallelCLI(t *testing.T) *CLI { +// CLIOption to customize behavior for all commands for a CLI instance. +type CLIOption func(c *CLI) + +// NewParallelCLI marks the parent test as parallel and returns a CLI instance +// suitable for usage across child tests. +func NewParallelCLI(t *testing.T, opts ...CLIOption) *CLI { + t.Helper() t.Parallel() - return NewCLI(t) + return NewCLI(t, opts...) } -// NewCLI returns a CLI to use for E2E tests -func NewCLI(t testing.TB) *CLI { - d, err := ioutil.TempDir("", "") - assert.Check(t, is.Nil(err)) +// NewCLI creates a CLI instance for running E2E tests. +func NewCLI(t testing.TB, opts ...CLIOption) *CLI { + t.Helper() + + configDir := t.TempDir() + initializePlugins(t, configDir) + + c := &CLI{ + ConfigDir: configDir, + } + + for _, opt := range opts { + opt(c) + } + + return c +} + +// WithEnv sets environment variables that will be passed to commands. +func WithEnv(env ...string) CLIOption { + return func(c *CLI) { + c.env = append(c.env, env...) + } +} + +// initializePlugins copies the necessary plugin files to the temporary config +// directory for the test. +func initializePlugins(t testing.TB, d string) { + t.Helper() t.Cleanup(func() { if t.Failed() { @@ -102,8 +133,6 @@ func NewCLI(t testing.TB) *CLI { panic(err) } } - - return &CLI{ConfigDir: d} } func dirContents(dir string) []string { @@ -168,13 +197,15 @@ func (c *CLI) BaseEnvironment() []string { func (c *CLI) NewCmd(command string, args ...string) icmd.Cmd { return icmd.Cmd{ Command: append([]string{command}, args...), - Env: c.BaseEnvironment(), + Env: append(c.BaseEnvironment(), c.env...), } } // NewCmdWithEnv creates a cmd object configured with the test environment set with additional env vars func (c *CLI) NewCmdWithEnv(envvars []string, command string, args ...string) icmd.Cmd { - cmdEnv := append(c.BaseEnvironment(), envvars...) + // base env -> CLI overrides -> cmd overrides + cmdEnv := append(c.BaseEnvironment(), c.env...) + cmdEnv = append(cmdEnv, envvars...) return icmd.Cmd{ Command: append([]string{command}, args...), Env: cmdEnv, From 3ae6c52e8a64ab55b29896bc10f4156962b5ee7e Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Thu, 16 Jun 2022 09:37:06 -0400 Subject: [PATCH 3/5] e2e: add extra tools needed for ddev test Signed-off-by: Milas Bowman --- pkg/e2e/ddev_test.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/e2e/ddev_test.go b/pkg/e2e/ddev_test.go index 63217f95..e4f44710 100644 --- a/pkg/e2e/ddev_test.go +++ b/pkg/e2e/ddev_test.go @@ -42,15 +42,17 @@ func TestComposeRunDdev(t *testing.T) { // ddev shells out to `docker` and `docker-compose` (standalone), so a // temporary directory is created with symlinks to system Docker and the // locally-built standalone Compose binary to use as PATH + requiredTools := []string{ + findToolInPath(t, DockerExecutableName), + ComposeStandalonePath(t), + findToolInPath(t, "tar"), + findToolInPath(t, "gzip"), + } pathDir := t.TempDir() - dockerBin, err := exec.LookPath(DockerExecutableName) - require.NoError(t, err, "Could not find %q in path", DockerExecutableName) - require.NoError(t, os.Symlink(dockerBin, filepath.Join(pathDir, DockerExecutableName)), - "Could not create %q symlink", DockerExecutableName) - - composeBin := ComposeStandalonePath(t) - require.NoError(t, os.Symlink(composeBin, filepath.Join(pathDir, DockerComposeExecutableName)), - "Could not create %q symlink", DockerComposeExecutableName) + for _, tool := range requiredTools { + require.NoError(t, os.Symlink(tool, filepath.Join(pathDir, filepath.Base(tool))), + "Could not create symlink for %q", tool) + } c := NewCLI(t, WithEnv( "DDEV_DEBUG=true", @@ -97,3 +99,10 @@ func TestComposeRunDdev(t *testing.T) { fmt.Println(out) assert.Assert(t, strings.Contains(out, "ddev is working"), "Could not start project") } + +func findToolInPath(t testing.TB, name string) string { + t.Helper() + binPath, err := exec.LookPath(name) + require.NoError(t, err, "Could not find %q in path", name) + return binPath +} From ccd87311e8e0c3cb210d4c7eaf875ae3cf8def26 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Thu, 16 Jun 2022 09:44:26 -0400 Subject: [PATCH 4/5] e2e: always set HOME + USER for cmd env Signed-off-by: Milas Bowman --- pkg/e2e/ddev_test.go | 2 -- pkg/e2e/framework.go | 10 ++++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/e2e/ddev_test.go b/pkg/e2e/ddev_test.go index e4f44710..00b890d1 100644 --- a/pkg/e2e/ddev_test.go +++ b/pkg/e2e/ddev_test.go @@ -56,8 +56,6 @@ func TestComposeRunDdev(t *testing.T) { c := NewCLI(t, WithEnv( "DDEV_DEBUG=true", - fmt.Sprintf("HOME=%s", t.TempDir()), - fmt.Sprintf("USER=%s", os.Getenv("USER")), fmt.Sprintf("PATH=%s", pathDir), )) diff --git a/pkg/e2e/framework.go b/pkg/e2e/framework.go index 49874e95..2a5880ad 100644 --- a/pkg/e2e/framework.go +++ b/pkg/e2e/framework.go @@ -59,8 +59,15 @@ func init() { // CLI is used to wrap the CLI for end to end testing type CLI struct { + // ConfigDir for Docker configuration (set as DOCKER_CONFIG) ConfigDir string + // HomeDir for tools that look for user files (set as HOME) + HomeDir string + + // env overrides to apply to every invoked command + // + // To populate, use WithEnv when creating a CLI instance. env []string } @@ -84,6 +91,7 @@ func NewCLI(t testing.TB, opts ...CLIOption) *CLI { c := &CLI{ ConfigDir: configDir, + HomeDir: t.TempDir(), } for _, opt := range opts { @@ -188,6 +196,8 @@ func CopyFile(sourceFile string, destinationFile string) error { // Docker / Compose commands. func (c *CLI) BaseEnvironment() []string { return []string{ + "HOME=" + c.HomeDir, + "USER=" + os.Getenv("USER"), "DOCKER_CONFIG=" + c.ConfigDir, "KUBECONFIG=invalid", } From a261682ca8010ffba0569c2875e551471eca7521 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Thu, 16 Jun 2022 11:27:34 -0400 Subject: [PATCH 5/5] e2e: fix per-command env overrides Signed-off-by: Milas Bowman --- pkg/e2e/compose_environment_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/e2e/compose_environment_test.go b/pkg/e2e/compose_environment_test.go index 60ec3d79..3314617c 100644 --- a/pkg/e2e/compose_environment_test.go +++ b/pkg/e2e/compose_environment_test.go @@ -45,7 +45,8 @@ func TestEnvPriority(t *testing.T) { cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/environment/env-priority/compose-with-env.yaml", "--project-directory", projectDir, "--env-file", "./fixtures/environment/env-priority/.env.override", "run", "--rm", "-e", "WHEREAMI", "env-compose-priority") - res := icmd.RunCmd(cmd, icmd.WithEnv("WHEREAMI=shell")) + cmd.Env = append(cmd.Env, "WHEREAMI=shell") + res := icmd.RunCmd(cmd) assert.Equal(t, strings.TrimSpace(res.Stdout()), "Compose File") }) @@ -59,7 +60,8 @@ func TestEnvPriority(t *testing.T) { cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/environment/env-priority/compose.yaml", "--project-directory", projectDir, "--env-file", "./fixtures/environment/env-priority/.env.override", "run", "--rm", "-e", "WHEREAMI", "env-compose-priority") - res := icmd.RunCmd(cmd, icmd.WithEnv("WHEREAMI=shell")) + cmd.Env = append(cmd.Env, "WHEREAMI=shell") + res := icmd.RunCmd(cmd) assert.Equal(t, strings.TrimSpace(res.Stdout()), "shell") }) @@ -133,7 +135,8 @@ func TestEnvInterpolation(t *testing.T) { t.Run("shell priority from run command", func(t *testing.T) { cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/environment/env-interpolation/compose.yaml", "--project-directory", projectDir, "config") - res := icmd.RunCmd(cmd, icmd.WithEnv("WHEREAMI=shell")) + cmd.Env = append(cmd.Env, "WHEREAMI=shell") + res := icmd.RunCmd(cmd) res.Assert(t, icmd.Expected{Out: `IMAGE: default_env:shell`}) }) }