Skip to content

Commit fe37f1f

Browse files
jeanlaurentjeanlaurent
committed
Fix docker-archive-public#2204 broken env unset
Signed-off-by: Jean-Laurent de Morlhon <jeanlaurent@morlhon.net>
1 parent 9dd438e commit fe37f1f

File tree

4 files changed

+146
-60
lines changed

4 files changed

+146
-60
lines changed

‎commands/env.go‎

Lines changed: 82 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ const (
1717
)
1818

1919
var (
20-
errImproperEnvArgs = errors.New("Error: Expected either one machine name, or -u flag to unset the variables in the arguments")
20+
errImproperEnvArgs = errors.New("Error: Expected one machine name")
21+
errImproperUnsetEnvArgs = errors.New("Error: Expected no machine name when the -u flag is present")
2122
)
2223

2324
type ShellConfig struct {
@@ -38,7 +39,14 @@ func cmdEnv(c CommandLine) error {
3839
// being run (it is intended to be run in a subshell)
3940
log.SetOutWriter(os.Stderr)
4041

41-
if len(c.Args()) != 1 && !c.Bool("unset") {
42+
if c.Bool("unset") {
43+
return unset(c)
44+
}
45+
return set(c)
46+
}
47+
48+
func set(c CommandLine) error {
49+
if len(c.Args()) != 1 {
4250
return errImproperEnvArgs
4351
}
4452

@@ -52,24 +60,16 @@ func cmdEnv(c CommandLine) error {
5260
return fmt.Errorf("Error running connection boilerplate: %s", err)
5361
}
5462

55-
userShell := c.String("shell")
56-
if userShell == "" {
57-
shell, err := detectShell()
58-
if err != nil {
59-
return err
60-
}
61-
userShell = shell
63+
userShell, err := getShell(c)
64+
if err != nil {
65+
return err
6266
}
6367

64-
t := template.New("envConfig")
65-
66-
usageHint := generateUsageHint(userShell, os.Args)
67-
6868
shellCfg := &ShellConfig{
6969
DockerCertPath: filepath.Join(mcndirs.GetMachineDir(), host.Name),
7070
DockerHost: dockerHost,
7171
DockerTLSVerify: "1",
72-
UsageHint: usageHint,
72+
UsageHint: generateUsageHint(userShell, os.Args),
7373
MachineName: host.Name,
7474
}
7575

@@ -79,22 +79,14 @@ func cmdEnv(c CommandLine) error {
7979
return fmt.Errorf("Error getting host IP: %s", err)
8080
}
8181

82-
// first check for an existing lower case no_proxy var
83-
noProxyVar := "no_proxy"
84-
noProxyValue := os.Getenv("no_proxy")
85-
86-
// otherwise default to allcaps HTTP_PROXY
87-
if noProxyValue == "" {
88-
noProxyVar = "NO_PROXY"
89-
noProxyValue = os.Getenv("NO_PROXY")
90-
}
82+
noProxyVar, noProxyValue := findNoProxyFromEnv()
9183

9284
// add the docker host to the no_proxy list idempotently
9385
switch {
9486
case noProxyValue == "":
9587
noProxyValue = ip
9688
case strings.Contains(noProxyValue, ip):
97-
//ip already in no_proxy list, nothing to do
89+
//ip already in no_proxy list, nothing to do
9890
default:
9991
noProxyValue = fmt.Sprintf("%s,%s", noProxyValue, ip)
10092
}
@@ -103,39 +95,6 @@ func cmdEnv(c CommandLine) error {
10395
shellCfg.NoProxyValue = noProxyValue
10496
}
10597

106-
// unset vars
107-
if c.Bool("unset") {
108-
switch userShell {
109-
case "fish":
110-
shellCfg.Prefix = "set -e "
111-
shellCfg.Delimiter = ""
112-
shellCfg.Suffix = ";\n"
113-
case "powershell":
114-
shellCfg.Prefix = "Remove-Item Env:\\\\"
115-
shellCfg.Delimiter = ""
116-
shellCfg.Suffix = "\n"
117-
case "cmd":
118-
// since there is no way to unset vars in cmd just reset to empty
119-
shellCfg.DockerCertPath = ""
120-
shellCfg.DockerHost = ""
121-
shellCfg.DockerTLSVerify = ""
122-
shellCfg.Prefix = "set "
123-
shellCfg.Delimiter = "="
124-
shellCfg.Suffix = "\n"
125-
default:
126-
shellCfg.Prefix = "unset "
127-
shellCfg.Delimiter = " "
128-
shellCfg.Suffix = "\n"
129-
}
130-
131-
tmpl, err := t.Parse(envTmpl)
132-
if err != nil {
133-
return err
134-
}
135-
136-
return tmpl.Execute(os.Stdout, shellCfg)
137-
}
138-
13998
switch userShell {
14099
case "fish":
141100
shellCfg.Prefix = "set -x "
@@ -155,6 +114,51 @@ func cmdEnv(c CommandLine) error {
155114
shellCfg.Delimiter = "=\""
156115
}
157116

117+
return executeTemplateStdout(shellCfg)
118+
}
119+
120+
func unset(c CommandLine) error {
121+
if len(c.Args()) != 0 {
122+
return errImproperUnsetEnvArgs
123+
}
124+
125+
userShell, err := getShell(c)
126+
if err != nil {
127+
return err
128+
}
129+
130+
shellCfg := &ShellConfig{
131+
UsageHint: generateUsageHint(userShell, os.Args),
132+
}
133+
134+
if c.Bool("no-proxy") {
135+
shellCfg.NoProxyVar, shellCfg.NoProxyValue = findNoProxyFromEnv()
136+
}
137+
138+
switch userShell {
139+
case "fish":
140+
shellCfg.Prefix = "set -e "
141+
shellCfg.Suffix = ";\n"
142+
shellCfg.Delimiter = ""
143+
case "powershell":
144+
shellCfg.Prefix = `Remove-Item Env:\\`
145+
shellCfg.Suffix = "\n"
146+
shellCfg.Delimiter = ""
147+
case "cmd":
148+
shellCfg.Prefix = "SET "
149+
shellCfg.Suffix = "\n"
150+
shellCfg.Delimiter = "="
151+
default:
152+
shellCfg.Prefix = "unset "
153+
shellCfg.Suffix = "\n"
154+
shellCfg.Delimiter = ""
155+
}
156+
157+
return executeTemplateStdout(shellCfg)
158+
}
159+
160+
func executeTemplateStdout(shellCfg *ShellConfig) error {
161+
t := template.New("envConfig")
158162
tmpl, err := t.Parse(envTmpl)
159163
if err != nil {
160164
return err
@@ -163,6 +167,27 @@ func cmdEnv(c CommandLine) error {
163167
return tmpl.Execute(os.Stdout, shellCfg)
164168
}
165169

170+
func getShell(c CommandLine) (string, error) {
171+
userShell := c.String("shell")
172+
if userShell != "" {
173+
return userShell, nil
174+
}
175+
return detectShell()
176+
}
177+
178+
func findNoProxyFromEnv() (string, string) {
179+
// first check for an existing lower case no_proxy var
180+
noProxyVar := "no_proxy"
181+
noProxyValue := os.Getenv("no_proxy")
182+
183+
// otherwise default to allcaps HTTP_PROXY
184+
if noProxyValue == "" {
185+
noProxyVar = "NO_PROXY"
186+
noProxyValue = os.Getenv("NO_PROXY")
187+
}
188+
return noProxyVar, noProxyValue
189+
}
190+
166191
func generateUsageHint(userShell string, args []string) string {
167192
cmd := ""
168193
comment := "#"

‎commands/env_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,25 @@ func TestHints(t *testing.T) {
1818
{"", "machine env --no-proxy default", "# Run this command to configure your shell: \n# eval \"$(machine env --no-proxy default)\"\n"},
1919
{"", "machine env --swarm default", "# Run this command to configure your shell: \n# eval \"$(machine env --swarm default)\"\n"},
2020
{"", "machine env --no-proxy --swarm default", "# Run this command to configure your shell: \n# eval \"$(machine env --no-proxy --swarm default)\"\n"},
21+
{"", "machine env --unset", "# Run this command to configure your shell: \n# eval \"$(machine env --unset)\"\n"},
2122

2223
{"fish", "./machine env --shell=fish default", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish default)\n"},
2324
{"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"},
2425
{"fish", "./machine env --shell=fish --swarm default", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish --swarm default)\n"},
2526
{"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"},
27+
{"fish", "./machine env --shell=fish --unset", "# Run this command to configure your shell: \n# eval (./machine env --shell=fish --unset)\n"},
2628

2729
{"powershell", "./machine env --shell=powershell default", "# Run this command to configure your shell: \n# ./machine env --shell=powershell default | Invoke-Expression\n"},
2830
{"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"},
2931
{"powershell", "./machine env --shell=powershell --swarm default", "# Run this command to configure your shell: \n# ./machine env --shell=powershell --swarm default | Invoke-Expression\n"},
3032
{"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"},
33+
{"powershell", "./machine env --shell=powershell --unset", "# Run this command to configure your shell: \n# ./machine env --shell=powershell --unset | Invoke-Expression\n"},
3134

3235
{"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"},
3336
{"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"},
3437
{"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"},
3538
{"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"},
39+
{"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"},
3640
}
3741

3842
for _, test := range tests {

‎libmachine/drivers/base_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ package drivers
33
import (
44
"errors"
55
"fmt"
6-
"github.com/stretchr/testify/assert"
76
"testing"
7+
8+
"github.com/stretchr/testify/assert"
89
)
910

1011
func TestIP(t *testing.T) {

‎test/integration/core/env_shell.bats‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ load ${BASE_TEST_DIR}/helpers.bash
77
[ "$status" -eq 0 ]
88
}
99

10+
@test "$DRIVER: test basic bash / zsh notation" {
11+
run machine env $NAME
12+
[[ ${lines[0]} == "export DOCKER_TLS_VERIFY=\"1\"" ]]
13+
[[ ${lines[1]} == "export DOCKER_HOST=\"$(machine url $NAME)\"" ]]
14+
[[ ${lines[2]} == "export DOCKER_CERT_PATH=\"$MACHINE_STORAGE_PATH/machines/$NAME\"" ]]
15+
[[ ${lines[3]} == "export DOCKER_MACHINE_NAME=\"$NAME\"" ]]
16+
}
17+
1018
@test "$DRIVER: test powershell notation" {
1119
run machine env --shell powershell --no-proxy $NAME
1220
[[ ${lines[0]} == "\$Env:DOCKER_TLS_VERIFY = \"1\"" ]]
@@ -16,7 +24,7 @@ load ${BASE_TEST_DIR}/helpers.bash
1624
[[ ${lines[4]} == "\$Env:NO_PROXY = \"$(machine ip $NAME)\"" ]]
1725
}
1826

19-
@test "$DRIVER: test bash / zsh notation" {
27+
@test "$DRIVER: test bash / zsh notation with no-proxy" {
2028
run machine env --no-proxy $NAME
2129
[[ ${lines[0]} == "export DOCKER_TLS_VERIFY=\"1\"" ]]
2230
[[ ${lines[1]} == "export DOCKER_HOST=\"$(machine url $NAME)\"" ]]
@@ -43,8 +51,56 @@ load ${BASE_TEST_DIR}/helpers.bash
4351
[[ ${lines[4]} == "set -x NO_PROXY \"$(machine ip $NAME)\";" ]]
4452
}
4553

46-
@test "$DRIVER: no proxy with NO_PROXY already set" {
54+
@test "$DRIVER: test no proxy with NO_PROXY already set" {
4755
export NO_PROXY=localhost
4856
run machine env --no-proxy $NAME
4957
[[ ${lines[4]} == "export NO_PROXY=\"localhost,$(machine ip $NAME)\"" ]]
5058
}
59+
60+
@test "$DRIVER: test unset with an args should fail" {
61+
run machine env -u $NAME
62+
[ "$status" -eq 1 ]
63+
[[ ${lines} == "Error: Expected no machine name when the -u flag is present" ]]
64+
}
65+
66+
67+
@test "$DRIVER: test bash/zsh unset" {
68+
run machine env -u
69+
[[ ${lines[0]} == "unset DOCKER_TLS_VERIFY" ]]
70+
[[ ${lines[1]} == "unset DOCKER_HOST" ]]
71+
[[ ${lines[2]} == "unset DOCKER_CERT_PATH" ]]
72+
[[ ${lines[3]} == "unset DOCKER_MACHINE_NAME" ]]
73+
}
74+
75+
@test "$DRIVER: test unset killing no proxy" {
76+
run machine env --no-proxy -u
77+
[[ ${lines[0]} == "unset DOCKER_TLS_VERIFY" ]]
78+
[[ ${lines[1]} == "unset DOCKER_HOST" ]]
79+
[[ ${lines[2]} == "unset DOCKER_CERT_PATH" ]]
80+
[[ ${lines[3]} == "unset DOCKER_MACHINE_NAME" ]]
81+
[[ ${lines[4]} == "unset NO_PROXY" ]]
82+
}
83+
84+
@test "$DRIVER: unset powershell" {
85+
run machine env --shell powershell -u
86+
[[ ${lines[0]} == 'Remove-Item Env:\\DOCKER_TLS_VERIFY' ]]
87+
[[ ${lines[1]} == 'Remove-Item Env:\\DOCKER_HOST' ]]
88+
[[ ${lines[2]} == 'Remove-Item Env:\\DOCKER_CERT_PATH' ]]
89+
[[ ${lines[3]} == 'Remove-Item Env:\\DOCKER_MACHINE_NAME' ]]
90+
}
91+
92+
@test "$DRIVER: unset with fish shell" {
93+
run machine env --shell fish -u
94+
[[ ${lines[0]} == "set -e DOCKER_TLS_VERIFY;" ]]
95+
[[ ${lines[1]} == "set -e DOCKER_HOST;" ]]
96+
[[ ${lines[2]} == "set -e DOCKER_CERT_PATH;" ]]
97+
[[ ${lines[3]} == "set -e DOCKER_MACHINE_NAME;" ]]
98+
}
99+
100+
@test "$DRIVER: unset with cmd shell" {
101+
run machine env --shell cmd -u
102+
[[ ${lines[0]} == "SET DOCKER_TLS_VERIFY=" ]]
103+
[[ ${lines[1]} == "SET DOCKER_HOST=" ]]
104+
[[ ${lines[2]} == "SET DOCKER_CERT_PATH=" ]]
105+
[[ ${lines[3]} == "SET DOCKER_MACHINE_NAME=" ]]
106+
}

0 commit comments

Comments
 (0)