Merge pull request #2250 from jeanlaurent/2204-broken-env-unset

Fix for docker-machine env unset

commands/env.go

@@ -17,7 +17,8 @@ const (
 )
 
 var (
-	errImproperEnvArgs = errors.New("Error: Expected either one machine name, or -u flag to unset the variables in the arguments")
+	errImproperEnvArgs      = errors.New("Error: Expected one machine name")
+	errImproperUnsetEnvArgs = errors.New("Error: Expected no machine name when the -u flag is present")
 )
 
 type ShellConfig struct {
@@ -38,7 +39,14 @@ func cmdEnv(c CommandLine) error {
 	// being run (it is intended to be run in a subshell)
 	log.SetOutWriter(os.Stderr)
 
-	if len(c.Args()) != 1 && !c.Bool("unset") {
+	if c.Bool("unset") {
+		return unset(c)
+	}
+	return set(c)
+}
+
+func set(c CommandLine) error {
+	if len(c.Args()) != 1 {
 		return errImproperEnvArgs
 	}
 
@@ -52,24 +60,16 @@ func cmdEnv(c CommandLine) error {
 		return fmt.Errorf("Error running connection boilerplate: %s", err)
 	}
 
-	userShell := c.String("shell")
+	userShell, err := getShell(c)
-	if userShell == "" {
+	if err != nil {
-		shell, err := detectShell()
+		return err
-		if err != nil {
-			return err
-		}
-		userShell = shell
 	}
 
-	t := template.New("envConfig")
-
-	usageHint := generateUsageHint(userShell, os.Args)
-
 	shellCfg := &ShellConfig{
 		DockerCertPath:  filepath.Join(mcndirs.GetMachineDir(), host.Name),
 		DockerHost:      dockerHost,
 		DockerTLSVerify: "1",
-		UsageHint:       usageHint,
+		UsageHint:       generateUsageHint(userShell, os.Args),
 		MachineName:     host.Name,
 	}
 
@@ -79,22 +79,14 @@ func cmdEnv(c CommandLine) error {
 			return fmt.Errorf("Error getting host IP: %s", err)
 		}
 
-		// first check for an existing lower case no_proxy var
+		noProxyVar, noProxyValue := findNoProxyFromEnv()
-		noProxyVar := "no_proxy"
-		noProxyValue := os.Getenv("no_proxy")
-
-		// otherwise default to allcaps HTTP_PROXY
-		if noProxyValue == "" {
-			noProxyVar = "NO_PROXY"
-			noProxyValue = os.Getenv("NO_PROXY")
-		}
 
 		// add the docker host to the no_proxy list idempotently
 		switch {
 		case noProxyValue == "":
 			noProxyValue = ip
 		case strings.Contains(noProxyValue, ip):
-			//ip already in no_proxy list, nothing to do
+		//ip already in no_proxy list, nothing to do
 		default:
 			noProxyValue = fmt.Sprintf("%s,%s", noProxyValue, ip)
 		}
@@ -103,39 +95,6 @@ func cmdEnv(c CommandLine) error {
 		shellCfg.NoProxyValue = noProxyValue
 	}
 
-	// unset vars
-	if c.Bool("unset") {
-		switch userShell {
-		case "fish":
-			shellCfg.Prefix = "set -e "
-			shellCfg.Delimiter = ""
-			shellCfg.Suffix = ";\n"
-		case "powershell":
-			shellCfg.Prefix = "Remove-Item Env:\\\\"
-			shellCfg.Delimiter = ""
-			shellCfg.Suffix = "\n"
-		case "cmd":
-			// since there is no way to unset vars in cmd just reset to empty
-			shellCfg.DockerCertPath = ""
-			shellCfg.DockerHost = ""
-			shellCfg.DockerTLSVerify = ""
-			shellCfg.Prefix = "set "
-			shellCfg.Delimiter = "="
-			shellCfg.Suffix = "\n"
-		default:
-			shellCfg.Prefix = "unset "
-			shellCfg.Delimiter = " "
-			shellCfg.Suffix = "\n"
-		}
-
-		tmpl, err := t.Parse(envTmpl)
-		if err != nil {
-			return err
-		}
-
-		return tmpl.Execute(os.Stdout, shellCfg)
-	}
-
 	switch userShell {
 	case "fish":
 		shellCfg.Prefix = "set -x "
@@ -155,6 +114,51 @@ func cmdEnv(c CommandLine) error {
 		shellCfg.Delimiter = "=\""
 	}
 
+	return executeTemplateStdout(shellCfg)
+}
+
+func unset(c CommandLine) error {
+	if len(c.Args()) != 0 {
+		return errImproperUnsetEnvArgs
+	}
+
+	userShell, err := getShell(c)
+	if err != nil {
+		return err
+	}
+
+	shellCfg := &ShellConfig{
+		UsageHint: generateUsageHint(userShell, os.Args),
+	}
+
+	if c.Bool("no-proxy") {
+		shellCfg.NoProxyVar, shellCfg.NoProxyValue = findNoProxyFromEnv()
+	}
+
+	switch userShell {
+	case "fish":
+		shellCfg.Prefix = "set -e "
+		shellCfg.Suffix = ";\n"
+		shellCfg.Delimiter = ""
+	case "powershell":
+		shellCfg.Prefix = `Remove-Item Env:\\`
+		shellCfg.Suffix = "\n"
+		shellCfg.Delimiter = ""
+	case "cmd":
+		shellCfg.Prefix = "SET "
+		shellCfg.Suffix = "\n"
+		shellCfg.Delimiter = "="
+	default:
+		shellCfg.Prefix = "unset "
+		shellCfg.Suffix = "\n"
+		shellCfg.Delimiter = ""
+	}
+
+	return executeTemplateStdout(shellCfg)
+}
+
+func executeTemplateStdout(shellCfg *ShellConfig) error {
+	t := template.New("envConfig")
 	tmpl, err := t.Parse(envTmpl)
 	if err != nil {
 		return err
@@ -163,6 +167,27 @@ func cmdEnv(c CommandLine) error {
 	return tmpl.Execute(os.Stdout, shellCfg)
 }
 
+func getShell(c CommandLine) (string, error) {
+	userShell := c.String("shell")
+	if userShell != "" {
+		return userShell, nil
+	}
+	return detectShell()
+}
+
+func findNoProxyFromEnv() (string, string) {
+	// first check for an existing lower case no_proxy var
+	noProxyVar := "no_proxy"
+	noProxyValue := os.Getenv("no_proxy")
+
+	// otherwise default to allcaps HTTP_PROXY
+	if noProxyValue == "" {
+		noProxyVar = "NO_PROXY"
+		noProxyValue = os.Getenv("NO_PROXY")
+	}
+	return noProxyVar, noProxyValue
+}
+
 func generateUsageHint(userShell string, args []string) string {
 	cmd := ""
 	comment := "#"

commands/env_test.go

@@ -18,21 +18,25 @@ func TestHints(t *testing.T) {
 		{"", "machine env --no-proxy default", "# Run this command to configure your shell: \n# eval \"$(machine env --no-proxy default)\"\n"},
 		{"", "machine env --swarm default", "# Run this command to configure your shell: \n# eval \"$(machine env --swarm default)\"\n"},
 		{"", "machine env --no-proxy --swarm default", "# Run this command to configure your shell: \n# eval \"$(machine env --no-proxy --swarm default)\"\n"},
+		{"", "machine env --unset", "# Run this command to configure your shell: \n# eval \"$(machine env --unset)\"\n"},
 
 		{"fish", "./machine env --shell=fish default", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish default)\n"},
 		{"fish", "./machine env --shell=fish --no-proxy default", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish --no-proxy default)\n"},
 		{"fish", "./machine env --shell=fish --swarm default", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish --swarm default)\n"},
 		{"fish", "./machine env --shell=fish --no-proxy --swarm default", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish --no-proxy --swarm default)\n"},
+		{"fish", "./machine env --shell=fish --unset", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish --unset)\n"},
 
 		{"powershell", "./machine env --shell=powershell default", "# Run this command to configure your shell: \n# ./machine env --shell=powershell default | Invoke-Expression\n"},
 		{"powershell", "./machine env --shell=powershell --no-proxy default", "# Run this command to configure your shell: \n# ./machine env --shell=powershell --no-proxy default | Invoke-Expression\n"},
 		{"powershell", "./machine env --shell=powershell --swarm default", "# Run this command to configure your shell: \n# ./machine env --shell=powershell --swarm default | Invoke-Expression\n"},
 		{"powershell", "./machine env --shell=powershell --no-proxy --swarm default", "# Run this command to configure your shell: \n# ./machine env --shell=powershell --no-proxy --swarm default | Invoke-Expression\n"},
+		{"powershell", "./machine env --shell=powershell --unset", "# Run this command to configure your shell: \n# ./machine env --shell=powershell --unset | Invoke-Expression\n"},
 
 		{"cmd", "./machine env --shell=cmd default", "REM Run this command to configure your shell: \nREM \tFOR /f \"tokens=*\" %i IN ('./machine env --shell=cmd default') DO %i\n"},
 		{"cmd", "./machine env --shell=cmd --no-proxy default", "REM Run this command to configure your shell: \nREM \tFOR /f \"tokens=*\" %i IN ('./machine env --shell=cmd --no-proxy default') DO %i\n"},
 		{"cmd", "./machine env --shell=cmd --swarm default", "REM Run this command to configure your shell: \nREM \tFOR /f \"tokens=*\" %i IN ('./machine env --shell=cmd --swarm default') DO %i\n"},
 		{"cmd", "./machine env --shell=cmd --no-proxy --swarm default", "REM Run this command to configure your shell: \nREM \tFOR /f \"tokens=*\" %i IN ('./machine env --shell=cmd --no-proxy --swarm default') DO %i\n"},
+		{"cmd", "./machine env --shell=cmd --unset", "REM Run this command to configure your shell: \nREM \tFOR /f \"tokens=*\" %i IN ('./machine env --shell=cmd --unset') DO %i\n"},
 	}
 
 	for _, test := range tests {

libmachine/drivers/base_test.go

@@ -3,8 +3,9 @@ package drivers
 import (
 	"errors"
 	"fmt"
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestIP(t *testing.T) {

test/integration/core/env_shell.bats

@@ -7,6 +7,14 @@ load ${BASE_TEST_DIR}/helpers.bash
   [ "$status" -eq 0  ]
 }
 
+@test "$DRIVER: test basic bash / zsh notation" {
+  run machine env $NAME
+  [[ ${lines[0]} == "export DOCKER_TLS_VERIFY=\"1\"" ]]
+  [[ ${lines[1]} == "export DOCKER_HOST=\"$(machine url $NAME)\"" ]]
+  [[ ${lines[2]} == "export DOCKER_CERT_PATH=\"$MACHINE_STORAGE_PATH/machines/$NAME\"" ]]
+  [[ ${lines[3]} == "export DOCKER_MACHINE_NAME=\"$NAME\"" ]]
+}
+
 @test "$DRIVER: test powershell notation" {
   run machine env --shell powershell --no-proxy $NAME
   [[ ${lines[0]} == "\$Env:DOCKER_TLS_VERIFY = \"1\"" ]]
@@ -16,7 +24,7 @@ load ${BASE_TEST_DIR}/helpers.bash
   [[ ${lines[4]} == "\$Env:NO_PROXY = \"$(machine ip $NAME)\"" ]]
 }
 
-@test "$DRIVER: test bash / zsh notation" {
+@test "$DRIVER: test bash / zsh notation with no-proxy" {
   run machine env --no-proxy $NAME
   [[ ${lines[0]} == "export DOCKER_TLS_VERIFY=\"1\"" ]]
   [[ ${lines[1]} == "export DOCKER_HOST=\"$(machine url $NAME)\"" ]]
@@ -43,8 +51,56 @@ load ${BASE_TEST_DIR}/helpers.bash
   [[ ${lines[4]} == "set -x NO_PROXY \"$(machine ip $NAME)\";" ]]
 }
 
-@test "$DRIVER: no proxy with NO_PROXY already set" {
+@test "$DRIVER: test no proxy with NO_PROXY already set" {
   export NO_PROXY=localhost
   run machine env --no-proxy $NAME
   [[ ${lines[4]} == "export NO_PROXY=\"localhost,$(machine ip $NAME)\"" ]]
 }
+
+@test "$DRIVER: test unset with an args should fail" {
+  run machine env -u $NAME
+  [ "$status" -eq 1 ]
+  [[ ${lines} == "Error: Expected no machine name when the -u flag is present" ]]
+}
+
+
+@test "$DRIVER: test bash/zsh unset" {
+  run machine env -u
+  [[ ${lines[0]} == "unset DOCKER_TLS_VERIFY" ]]
+  [[ ${lines[1]} == "unset DOCKER_HOST" ]]
+  [[ ${lines[2]} == "unset DOCKER_CERT_PATH" ]]
+  [[ ${lines[3]} == "unset DOCKER_MACHINE_NAME" ]]
+}
+
+@test "$DRIVER: test unset killing no proxy" {
+  run machine env --no-proxy -u
+  [[ ${lines[0]} == "unset DOCKER_TLS_VERIFY" ]]
+  [[ ${lines[1]} == "unset DOCKER_HOST" ]]
+  [[ ${lines[2]} == "unset DOCKER_CERT_PATH" ]]
+  [[ ${lines[3]} == "unset DOCKER_MACHINE_NAME" ]]
+  [[ ${lines[4]} == "unset NO_PROXY" ]]
+}
+
+@test "$DRIVER: unset powershell" {
+  run machine env --shell powershell -u
+  [[ ${lines[0]} == 'Remove-Item Env:\\DOCKER_TLS_VERIFY' ]]
+  [[ ${lines[1]} == 'Remove-Item Env:\\DOCKER_HOST' ]]
+  [[ ${lines[2]} == 'Remove-Item Env:\\DOCKER_CERT_PATH' ]]
+  [[ ${lines[3]} == 'Remove-Item Env:\\DOCKER_MACHINE_NAME' ]]
+}
+
+@test "$DRIVER: unset with fish shell" {
+  run machine env --shell fish -u
+  [[ ${lines[0]} == "set -e DOCKER_TLS_VERIFY;" ]]
+  [[ ${lines[1]} == "set -e DOCKER_HOST;" ]]
+  [[ ${lines[2]} == "set -e DOCKER_CERT_PATH;" ]]
+  [[ ${lines[3]} == "set -e DOCKER_MACHINE_NAME;" ]]
+}
+
+@test "$DRIVER: unset with cmd shell" {
+  run machine env --shell cmd -u
+  [[ ${lines[0]} == "SET DOCKER_TLS_VERIFY=" ]]
+  [[ ${lines[1]} == "SET DOCKER_HOST=" ]]
+  [[ ${lines[2]} == "SET DOCKER_CERT_PATH=" ]]
+  [[ ${lines[3]} == "SET DOCKER_MACHINE_NAME=" ]]
+}