Skip to content

Commit e2f42fe

Browse files
Shubhranshu153Shubhranshu153
authored
fix: parse management commands with proper arguments (runfinch#876)
Issue #, if available: runfinch#827 Description of changes: Addresses runfinch#827 by adding all environment variables at the location of first occurrence of the environment flag Testing done: Changes tested locally to ensure that the issue is fixed. $ ./_output/bin/finch container run --debug -it --rm --env="E=v" busybox env DEBU[0000] Creating limactl command: ARGUMENTS: [shell finch sudo -E nerdctl container run -e E=v -it --rm busybox env], LIMA_HOME: /Users/mharwani/work/runfinch/finch/_output/lima/data PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin E=v TERM=xterm HOME=/root *Testing done:* - [ ] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Shubharanshu Mahapatra <[email protected]>
1 parent 9223219 commit e2f42fe

File tree

3 files changed

+176
-12
lines changed

3 files changed

+176
-12
lines changed

Diff for: ‎cmd/finch/nerdctl.go

+22-4
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
111111
skip, hasCmdHander, hasArgHandler bool
112112
cmdHandler commandHandler
113113
aMap map[string]argHandler
114+
envArgPos int
115+
isDebug int
114116
)
115117

116118
alias, hasAlias := aliasMap[cmdName]
@@ -139,6 +141,10 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
139141
}
140142
}
141143

144+
// envArgPos is used to preserve the position of first environment parameter
145+
envArgPos = -1
146+
// if a debug flag is passed before env arg pos we reduce the env arg pos by 1 to account for skipping debug flag
147+
isDebug = 0
142148
for i, arg := range args {
143149
// Check if command requires arg handling
144150
if hasArgHandler {
@@ -164,14 +170,24 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
164170
switch {
165171
case arg == "--debug":
166172
// explicitly setting log level to avoid `--debug` flag being interpreted as nerdctl command
173+
if envArgPos == -1 {
174+
isDebug = 1
175+
}
167176
nc.logger.SetLevel(flog.Debug)
168177
case argIsEnv(arg):
178+
if envArgPos == -1 {
179+
envArgPos = i - isDebug
180+
}
169181
shouldSkip, addEnv := handleEnv(nc.systemDeps, arg, args[i+1])
170182
skip = shouldSkip
171183
if addEnv != "" {
172184
envs = append(envs, addEnv)
173185
}
174186
case strings.HasPrefix(arg, "--env-file"):
187+
if envArgPos == -1 {
188+
envArgPos = i - isDebug
189+
}
190+
175191
shouldSkip, addEnvs, err := handleEnvFile(nc.fs, nc.systemDeps, arg, args[i+1])
176192
if err != nil {
177193
return err
@@ -249,14 +265,16 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
249265

250266
limaArgs = append(limaArgs, append([]string{nerdctlCmdName}, strings.Fields(cmdName)...)...)
251267

252-
var finalArgs []string
268+
var envArgs []string
253269
for key, val := range envVars {
254-
finalArgs = append(finalArgs, "-e", fmt.Sprintf("%s=%s", key, val))
270+
envArgs = append(envArgs, "-e", fmt.Sprintf("%s=%s", key, val))
271+
}
272+
if envArgPos > -1 {
273+
nerdctlArgs = append(nerdctlArgs[:envArgPos], append(envArgs, nerdctlArgs[envArgPos:]...)...)
255274
}
256-
finalArgs = append(finalArgs, nerdctlArgs...)
257275
// Add -E to sudo command in order to preserve existing environment variables, more info:
258276
// https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo/8633575#8633575
259-
limaArgs = append(limaArgs, finalArgs...)
277+
limaArgs = append(limaArgs, nerdctlArgs...)
260278

261279
if nc.shouldReplaceForHelp(cmdName, args) {
262280
return nc.lcc.RunWithReplacingStdout([]command.Replacement{{Source: "nerdctl", Target: "finch"}}, limaArgs...)

Diff for: ‎cmd/finch/nerdctl_darwin_test.go

+72-4
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func TestNerdctlCommand_run(t *testing.T) {
176176
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
177177

178178
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
179-
"-e", "ARG1=val1", "--rm", "alpine:latest", "env").Return(c)
179+
"--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c)
180180
c.EXPECT().Run()
181181
},
182182
},
@@ -207,7 +207,39 @@ func TestNerdctlCommand_run(t *testing.T) {
207207
ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true)
208208
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
209209
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
210-
"-e", "ARG3=val3", "--rm", "alpine:latest", "env").Return(c)
210+
"--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c)
211+
c.EXPECT().Run()
212+
},
213+
},
214+
{
215+
name: "with environment flags parsing and env value exists and with --debug flag",
216+
cmdName: "run",
217+
fc: &config.Finch{},
218+
args: []string{"--debug", "--rm", "--env=ARG2", "-eARG3", "alpine:latest", "env"},
219+
wantErr: nil,
220+
mockSvc: func(
221+
_ *testing.T,
222+
lcc *mocks.LimaCmdCreator,
223+
_ *mocks.CommandCreator,
224+
ncsd *mocks.NerdctlCommandSystemDeps,
225+
logger *mocks.Logger,
226+
ctrl *gomock.Controller,
227+
_ afero.Fs,
228+
) {
229+
getVMStatusC := mocks.NewCommand(ctrl)
230+
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
231+
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
232+
logger.EXPECT().SetLevel(flog.Debug)
233+
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
234+
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
235+
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
236+
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
237+
c := mocks.NewCommand(ctrl)
238+
ncsd.EXPECT().LookupEnv("ARG2")
239+
ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true)
240+
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
241+
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
242+
"--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c)
211243
c.EXPECT().Run()
212244
},
213245
},
@@ -241,7 +273,43 @@ func TestNerdctlCommand_run(t *testing.T) {
241273
ncsd.EXPECT().LookupEnv("NOTSETARG")
242274
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
243275
lcc.EXPECT().
244-
Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env").
276+
Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env").
277+
Return(c)
278+
c.EXPECT().Run()
279+
},
280+
},
281+
{
282+
name: "with --env-file flag replacement and with --debug flag",
283+
cmdName: "run",
284+
fc: &config.Finch{},
285+
args: []string{"--debug", "--rm", "--env-file=/env-file", "alpine:latest", "env"},
286+
wantErr: nil,
287+
mockSvc: func(
288+
t *testing.T,
289+
lcc *mocks.LimaCmdCreator,
290+
_ *mocks.CommandCreator,
291+
ncsd *mocks.NerdctlCommandSystemDeps,
292+
logger *mocks.Logger,
293+
ctrl *gomock.Controller,
294+
fs afero.Fs,
295+
) {
296+
envFileStr := "# a comment\nARG1=val1\n ARG2\n\n # a 2nd comment\nNOTSETARG\n "
297+
require.NoError(t, afero.WriteFile(fs, envFilePath, []byte(envFileStr), 0o600))
298+
299+
getVMStatusC := mocks.NewCommand(ctrl)
300+
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
301+
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
302+
logger.EXPECT().SetLevel(flog.Debug)
303+
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
304+
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
305+
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
306+
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
307+
c := mocks.NewCommand(ctrl)
308+
ncsd.EXPECT().LookupEnv("ARG2")
309+
ncsd.EXPECT().LookupEnv("NOTSETARG")
310+
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
311+
lcc.EXPECT().
312+
Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env").
245313
Return(c)
246314
c.EXPECT().Run()
247315
},
@@ -276,7 +344,7 @@ func TestNerdctlCommand_run(t *testing.T) {
276344
ncsd.EXPECT().LookupEnv("NOTSETARG")
277345
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
278346
lcc.EXPECT().
279-
Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-e", "ARG2=val2", "--rm", "alpine:latest", "env").
347+
Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", "ARG2=val2", "alpine:latest", "env").
280348
Return(c)
281349
c.EXPECT().Run()
282350
},

Diff for: ‎cmd/finch/nerdctl_windows_test.go

+82-4
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func TestNerdctlCommand_run(t *testing.T) {
202202
ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
203203

204204
lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run",
205-
"-e", "ARG1=val1", "--rm", "alpine:latest", "env").Return(c)
205+
"--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c)
206206
c.EXPECT().Run()
207207
},
208208
},
@@ -239,7 +239,45 @@ func TestNerdctlCommand_run(t *testing.T) {
239239
ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
240240
// alias substitution run=>container run
241241
lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run",
242-
"-e", "ARG3=val3", "--rm", "alpine:latest", "env").Return(c)
242+
"--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c)
243+
c.EXPECT().Run()
244+
},
245+
},
246+
{
247+
name: "with environment flags parsing and env value exists and with --debug flag",
248+
cmdName: "run",
249+
fc: &config.Finch{},
250+
args: []string{"--debug", "--rm", "--env=ARG2", "-eARG3", "alpine:latest", "env"},
251+
wantErr: nil,
252+
mockSvc: func(
253+
_ *testing.T,
254+
_ *mocks.CommandCreator,
255+
lcc *mocks.LimaCmdCreator,
256+
_ *mocks.Command,
257+
ncsd *mocks.NerdctlCommandSystemDeps,
258+
logger *mocks.Logger,
259+
ctrl *gomock.Controller,
260+
_ afero.Fs,
261+
) {
262+
getVMStatusC := mocks.NewCommand(ctrl)
263+
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
264+
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
265+
logger.EXPECT().SetLevel(flog.Debug)
266+
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
267+
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
268+
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
269+
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
270+
c := mocks.NewCommand(ctrl)
271+
ncsd.EXPECT().LookupEnv("ARG2")
272+
ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true)
273+
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
274+
ncsd.EXPECT().GetWd().Return("C:\\workdir", nil)
275+
ncsd.EXPECT().FilePathAbs("C:\\workdir").Return("C:\\workdir", nil)
276+
ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
277+
ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
278+
// alias substitution run=>container run
279+
lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run",
280+
"--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c)
243281
c.EXPECT().Run()
244282
},
245283
},
@@ -278,7 +316,47 @@ func TestNerdctlCommand_run(t *testing.T) {
278316
ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
279317
ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
280318
lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
281-
"sudo", "-E", nerdctlCmdName, "container", "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env").Return(c)
319+
"sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c)
320+
c.EXPECT().Run()
321+
},
322+
},
323+
{
324+
name: "with --env-file flag replacement and with --debug flag",
325+
cmdName: "run",
326+
fc: &config.Finch{},
327+
args: []string{"--debug", "--rm", "--env-file=" + envFilePath, "alpine:latest", "env"},
328+
wantErr: nil,
329+
mockSvc: func(
330+
t *testing.T,
331+
_ *mocks.CommandCreator,
332+
lcc *mocks.LimaCmdCreator,
333+
_ *mocks.Command,
334+
ncsd *mocks.NerdctlCommandSystemDeps,
335+
logger *mocks.Logger,
336+
ctrl *gomock.Controller,
337+
fs afero.Fs,
338+
) {
339+
envFileStr := "# a comment\nARG1=val1\n ARG2\n\n # a 2nd comment\nNOTSETARG\n "
340+
require.NoError(t, afero.WriteFile(fs, envFilePath, []byte(envFileStr), 0o600))
341+
342+
getVMStatusC := mocks.NewCommand(ctrl)
343+
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
344+
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
345+
logger.EXPECT().SetLevel(flog.Debug)
346+
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
347+
ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false)
348+
ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false)
349+
ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false)
350+
c := mocks.NewCommand(ctrl)
351+
ncsd.EXPECT().LookupEnv("ARG2")
352+
ncsd.EXPECT().LookupEnv("NOTSETARG")
353+
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
354+
ncsd.EXPECT().GetWd().Return("C:\\workdir", nil)
355+
ncsd.EXPECT().FilePathAbs("C:\\workdir").Return("C:\\workdir", nil)
356+
ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
357+
ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
358+
lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
359+
"sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c)
282360
c.EXPECT().Run()
283361
},
284362
},
@@ -317,7 +395,7 @@ func TestNerdctlCommand_run(t *testing.T) {
317395
ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
318396
ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
319397
lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
320-
"sudo", "-E", nerdctlCmdName, "container", "run", "-e", "ARG2=val2", "--rm", "alpine:latest", "env").Return(c)
398+
"sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", "ARG2=val2", "alpine:latest", "env").Return(c)
321399
c.EXPECT().Run()
322400
},
323401
},

0 commit comments

Comments
 (0)