From aa7b1b24a53c2b96cffc1b44e2f73fdc69a3c625 Mon Sep 17 00:00:00 2001 From: Nick Santos Date: Tue, 7 Jun 2022 17:56:11 -0400 Subject: [PATCH] command: treat DOCKER_HOST the same if it's empty or unset print appropriate warning messages on 'context list'/'context use' Signed-off-by: Nick Santos Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 2 +- cli/command/context/use.go | 2 +- cli/command/context/use_test.go | 78 +++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index ab89bef8b3..497e0f9e51 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -456,7 +456,7 @@ func resolveContextName(opts *cliflags.CommonOptions, config *configfile.ConfigF if len(opts.Hosts) > 0 { return DefaultContextName, nil } - if _, present := os.LookupEnv(client.EnvOverrideHost); present { + if os.Getenv(client.EnvOverrideHost) != "" { return DefaultContextName, nil } if ctxName, ok := os.LookupEnv("DOCKER_CONTEXT"); ok { diff --git a/cli/command/context/use.go b/cli/command/context/use.go index ab2a6f8d78..29d157a377 100644 --- a/cli/command/context/use.go +++ b/cli/command/context/use.go @@ -42,7 +42,7 @@ func RunUse(dockerCli command.Cli, name string) error { } fmt.Fprintln(dockerCli.Out(), name) fmt.Fprintf(dockerCli.Err(), "Current context is now %q\n", name) - if os.Getenv(client.EnvOverrideHost) != "" { + if name != command.DefaultContextName && os.Getenv(client.EnvOverrideHost) != "" { fmt.Fprintf(dockerCli.Err(), "Warning: %[1]s environment variable overrides the active context. "+ "To use %[2]q, either set the global --context flag, or unset %[1]s environment variable.\n", client.EnvOverrideHost, name) } diff --git a/cli/command/context/use_test.go b/cli/command/context/use_test.go index a0563daa98..3053cc03b2 100644 --- a/cli/command/context/use_test.go +++ b/cli/command/context/use_test.go @@ -1,13 +1,17 @@ package context import ( + "bytes" "path/filepath" "testing" + "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/context/store" + "github.com/docker/cli/cli/flags" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) func TestUse(t *testing.T) { @@ -41,3 +45,77 @@ func TestUseNoExist(t *testing.T) { err := newUseCommand(cli).RunE(nil, []string{"test"}) assert.Check(t, store.IsErrContextDoesNotExist(err)) } + +func TestUseHostOverride(t *testing.T) { + t.Setenv("DOCKER_HOST", "tcp://ed:2375/") + + configDir := t.TempDir() + configFilePath := filepath.Join(configDir, "config.json") + testCfg := configfile.New(configFilePath) + cli := makeFakeCli(t, withCliConfig(testCfg)) + err := RunCreate(cli, &CreateOptions{ + Name: "test", + Docker: map[string]string{}, + }) + assert.NilError(t, err) + + cli.ResetOutputBuffers() + err = newUseCommand(cli).RunE(nil, []string{"test"}) + assert.NilError(t, err) + assert.Assert(t, is.Contains( + cli.ErrBuffer().String(), + `Warning: DOCKER_HOST environment variable overrides the active context.`, + )) + assert.Assert(t, is.Contains(cli.ErrBuffer().String(), `Current context is now "test"`)) + assert.Equal(t, cli.OutBuffer().String(), "test\n") + + // setting DOCKER_HOST with the default context should not print a warning + cli.ResetOutputBuffers() + err = newUseCommand(cli).RunE(nil, []string{"default"}) + assert.NilError(t, err) + assert.Assert(t, is.Contains(cli.ErrBuffer().String(), `Current context is now "default"`)) + assert.Equal(t, cli.OutBuffer().String(), "default\n") +} + +// An empty DOCKER_HOST used to break the 'context use' flow. +// So we have a test with fewer fakes that tests this flow holistically. +// https://github.com/docker/cli/issues/3667 +func TestUseHostOverrideEmpty(t *testing.T) { + t.Setenv("DOCKER_HOST", "") + + configDir := t.TempDir() + config.SetDir(configDir) + + socketPath := "unix://" + filepath.Join(configDir, "docker.sock") + + var out *bytes.Buffer + var cli *command.DockerCli + + loadCli := func() { + out = bytes.NewBuffer(nil) + + var err error + cli, err = command.NewDockerCli(command.WithCombinedStreams(out)) + assert.NilError(t, err) + assert.NilError(t, cli.Initialize(flags.NewClientOptions())) + } + loadCli() + err := RunCreate(cli, &CreateOptions{ + Name: "test", + Docker: map[string]string{"host": socketPath}, + }) + assert.NilError(t, err) + + err = newUseCommand(cli).RunE(nil, []string{"test"}) + assert.NilError(t, err) + assert.Assert(t, !is.Contains(out.String(), "Warning")().Success()) + assert.Assert(t, is.Contains(out.String(), `Current context is now "test"`)) + + loadCli() + err = newShowCommand(cli).RunE(nil, nil) + assert.NilError(t, err) + assert.Assert(t, is.Contains(out.String(), "test")) + + apiclient := cli.Client() + assert.Equal(t, apiclient.DaemonHost(), socketPath) +}