From 1bfb3aaf5944bd047662a5d5e1566010401b2e0c Mon Sep 17 00:00:00 2001 From: Sam Chew Date: Wed, 21 Aug 2024 16:56:44 -0700 Subject: [PATCH] fix: Improve CLI parsing for Image Args Resolves #827 Evaluate CLI args according to positional group. Signed-off-by: Sam Chew --- cmd/finch/nerdctl.go | 194 +++++++++++++++++++++-------- cmd/finch/nerdctl_darwin_test.go | 199 +++++++++++++++++++++++++----- cmd/finch/nerdctl_windows_test.go | 30 ++--- go.mod | 1 + go.sum | 3 + 5 files changed, 330 insertions(+), 97 deletions(-) diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index 18ab0fd48..c5a4a4a85 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -16,6 +16,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/spf13/afero" "github.com/spf13/cobra" + orderedmap "github.com/wk8/go-ordered-map" "github.com/runfinch/finch/pkg/command" "github.com/runfinch/finch/pkg/config" @@ -108,12 +109,11 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { return err } var ( - nerdctlArgs, envs, fileEnvs []string - skip, hasCmdHander, hasArgHandler bool - cmdHandler commandHandler - aMap map[string]argHandler - envArgPos int - isDebug int + nerdctlArgs, envs, fileEnvs, imgArgs []string + skip, hasCmdHander, hasArgHandler bool + cmdHandler commandHandler + aMap map[string]argHandler + optArgPos, imgPos int ) alias, hasAlias := aliasMap[cmdName] @@ -142,10 +142,24 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { } } - // envArgPos is used to preserve the position of first environment parameter - envArgPos = -1 - // if a debug flag is passed before env arg pos we reduce the env arg pos by 1 to account for skipping debug flag - isDebug = 0 + // optArgPos is used to track when the "Option Set" has been found + optArgPos = -1 + // imgPos is used to track when the "Image Name" has been found + imgPos = -1 + + shortFlagBoolSet := sets.NewString("-d", "-i", "-t", "-di", "-dt", "-it", "-dit") + longFlagBoolSet := sets.NewString("--detach", "--init", "--interactive", "--oom-kill-disable", "--privileged", "--read-only", "--rm", "--rootfs", "--tty") + + shortFlagArgsSet := sets.NewString("-h", "-m", "-u", "-w", "-p", "-l", "-v") + + for i, arg := range args { + if strings.HasPrefix(arg, "-") && optArgPos < 0 { + // First finch Option found + optArgPos = i + break + } + } + for i, arg := range args { // Check if command requires arg handling if hasArgHandler { @@ -161,41 +175,32 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { arg = args[i] } } - // parsing environment values from the command line may pre-fetch and - // consume the next argument; this loop variable will skip these pre-consumed - // entries from the command line + // parsing arguments from the command line + // may pre-fetch and consume the next argument; + // the loop variable will skip any pre-consumed args if skip { skip = false continue } + + // after image position is found, pass through each arg as an image argument (short-circuit) + if imgPos > 0 { + imgArgs = append(imgArgs, arg) + continue + } + switch { case arg == "--debug": - // explicitly setting log level to avoid `--debug` flag being interpreted as nerdctl command - if envArgPos == -1 { - isDebug = 1 - } nc.logger.SetLevel(flog.Debug) - case argIsEnv(arg): - if envArgPos == -1 { - envArgPos = i - isDebug - } - shouldSkip, addEnv := handleEnv(nc.systemDeps, arg, args[i+1]) - skip = shouldSkip - if addEnv != "" { - envs = append(envs, addEnv) - } - case strings.HasPrefix(arg, "--env-file"): - if envArgPos == -1 { - envArgPos = i - isDebug - } - - shouldSkip, addEnvs, err := handleEnvFile(nc.fs, nc.systemDeps, arg, args[i+1]) - if err != nil { - return err - } - skip = shouldSkip - fileEnvs = append(fileEnvs, addEnvs...) + case arg == "--help": + nerdctlArgs = append(nerdctlArgs, arg) + break + case shortFlagBoolSet.Has(arg) || longFlagBoolSet.Has(arg): + // -d, -i, -t, -di, -dt, -dit + // --detach,--init,--interactive,--oom-kill-disable,--privileged,--read-only,--rm,--rootfs,--tty + nerdctlArgs = append(nerdctlArgs, arg) case strings.HasPrefix(arg, "--add-host"): + // finch Option: Add Host switch arg { case "--add-host": args[i+1], err = resolveIP(args[i+1], nc.logger, nc.ecc) @@ -213,8 +218,55 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { if err != nil { return err } + case strings.HasPrefix(arg, "--env-file"): + // finch Option: Env File + shouldSkip, addEnvs, err := handleEnvFile(nc.fs, nc.systemDeps, arg, args[i+1]) + if err != nil { + return err + } + skip = shouldSkip + fileEnvs = append(fileEnvs, addEnvs...) + case strings.HasPrefix(arg, "-e") || strings.HasPrefix(arg, "--env"): + // -e, -e="", -e"" + // --env="=", --env "=" + shouldSkip, addEnv := handleEnv(nc.systemDeps, arg, args[i+1]) + skip = shouldSkip + if addEnv != "" { + envs = append(envs, addEnv) + } + case shortFlagArgsSet.Has(arg) || shortFlagArgsSet.Has(arg[:2]): + // -h,-m,-u,-w,-p,-l,-v + shouldSkip, addKey, addVal := handleFlagArg(nc.systemDeps, arg, args[i+1]) + skip = shouldSkip + if addKey != "" { + nerdctlArgs = append(nerdctlArgs, addKey) + nerdctlArgs = append(nerdctlArgs, addVal) + } + case strings.HasPrefix(arg, "--"): + // --="", -- "" + shouldSkip, addKey, addVal := handleFlagArg(nc.systemDeps, arg, args[i+1]) + skip = shouldSkip + if addKey != "" { + nerdctlArgs = append(nerdctlArgs, addKey) + nerdctlArgs = append(nerdctlArgs, addVal) + } default: - nerdctlArgs = append(nerdctlArgs, arg) + // arg other than a flag ("-?","--") or a skipped + switch { + case (i < optArgPos): + // arg is a value prior to the first found flag + // pass the arg as a nerdctl command argument + nerdctlArgs = append(nerdctlArgs, arg) + case (i >= optArgPos) && (imgPos < 0): + // The first arg after procecssing the flags should be the Image Name + imgPos = i + imgArgs = append(imgArgs, arg) + default: + // Unexpected case + // pass the arg as a nerdctl arg by default + nc.logger.Debugln("Unexpected Arg Value: ", arg) + nerdctlArgs = append(nerdctlArgs, arg) + } } } @@ -222,15 +274,15 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { // env-file includes to the map first and then all command line environment // flags, making sure that command line overrides environment file options, // and that later command line flags override earlier ones - envVars := make(map[string]string) + envVars := orderedmap.New() for _, e := range fileEnvs { evar, eval, _ := strings.Cut(e, "=") - envVars[evar] = eval + envVars.Set(evar, eval) } for _, e := range envs { evar, eval, _ := strings.Cut(e, "=") - envVars[evar] = eval + envVars.Set(evar, eval) } passedEnvs := []string{ @@ -275,21 +327,21 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { limaArgs = append(limaArgs, append([]string{nerdctlCmdName}, strings.Fields(cmdName)...)...) var envArgs []string - for key, val := range envVars { - envArgs = append(envArgs, "-e", fmt.Sprintf("%s=%s", key, val)) - } - if envArgPos > -1 { - nerdctlArgs = append(nerdctlArgs[:envArgPos], append(envArgs, nerdctlArgs[envArgPos:]...)...) + for pair := envVars.Oldest(); pair != nil; pair = pair.Next() { + envArgs = append(envArgs, "-e", fmt.Sprintf(`"%s=%s"`, pair.Key, pair.Value)) } + // Add -E to sudo command in order to preserve existing environment variables, more info: // https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo/8633575#8633575 - limaArgs = append(limaArgs, nerdctlArgs...) + runArgs := append(limaArgs, nerdctlArgs...) + runArgs = append(runArgs, envArgs...) + runArgs = append(runArgs, imgArgs...) if nc.shouldReplaceForHelp(cmdName, args) { - return nc.lcc.RunWithReplacingStdout([]command.Replacement{{Source: "nerdctl", Target: "finch"}}, limaArgs...) + return nc.lcc.RunWithReplacingStdout([]command.Replacement{{Source: "nerdctl", Target: "finch"}}, runArgs...) } - return nc.lcc.Create(limaArgs...).Run() + return nc.lcc.Create(runArgs...).Run() } func (nc *nerdctlCommand) assertVMIsRunning(creator command.LimaCmdCreator, logger flog.Logger) error { @@ -342,6 +394,47 @@ func argIsEnv(arg string) bool { return false } +func handleFlagArg(systemDeps NerdctlCommandSystemDeps, arg string, nextArg string) (bool, string, string) { + // handling Flag arguements other than environment variables + var ( + flagKey, flagVal string + skip bool + ) + switch { + case strings.HasPrefix(arg, "--") && strings.Contains(arg, "="): + // long flag concatenated to value by '=': --="" + eqPos := strings.Index(arg, "=") + skip = false + flagKey = arg[:eqPos] + flagVal = arg[eqPos+1:] + case strings.HasPrefix(arg, "--") && !strings.HasPrefix(nextArg, "-"): + // long flag followed by a value: -- "" + skip = true + flagKey = arg + flagVal = nextArg + case strings.HasPrefix(arg, "-") && strings.Contains(arg, "="): + // short flag concatenated to value by '=': -?="" + eqPos := strings.Index(arg, "=") + skip = false + flagKey = arg[:eqPos] + flagVal = arg[eqPos+1:] + case strings.HasPrefix(arg, "-") && len(arg) > 2: + // short flag adjacent to value: -?"" + skip = false + flagKey = arg[:2] + flagVal = arg[2:] + case strings.HasPrefix(arg, "-") && len(arg) == 2: + // short flag followed by a value: -? "" + skip = true + flagKey = arg + flagVal = nextArg + default: + return false, "", "" + } + + return skip, flagKey, fmt.Sprintf("\"%s\"", flagVal) +} + func handleEnv(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, string) { var ( envVar string @@ -357,7 +450,8 @@ func handleEnv(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, str envVar = arg[2:] } else { // only other case is "--env="; skip that prefix - envVar = arg[6:] + eqPos := strings.Index(arg, "=") + envVar = arg[eqPos+1:] } } diff --git a/cmd/finch/nerdctl_darwin_test.go b/cmd/finch/nerdctl_darwin_test.go index 36671d2ad..ad5481c32 100644 --- a/cmd/finch/nerdctl_darwin_test.go +++ b/cmd/finch/nerdctl_darwin_test.go @@ -122,10 +122,10 @@ func TestNerdctlCommand_run(t *testing.T) { }, }, { - name: "with --debug flag", - cmdName: "pull", + name: "with explicit env flag parsing", + cmdName: "run", fc: &config.Finch{}, - args: []string{"test:tag", "--debug"}, + args: []string{"-it", "-e", "ARG1=val1", "--env=ARG2=val2", "-eARG3=val3", "--name", "myContainer", "--rm", "alpine:latest", "env"}, wantErr: nil, mockSvc: func( _ *testing.T, @@ -143,19 +143,20 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) - logger.EXPECT().SetLevel(flog.Debug) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) - lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag").Return(c) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", + "-it", "--name", `"myContainer"`, "--rm", "-e", `"ARG1=val1"`, "-e", `"ARG2=val2"`, "-e", `"ARG3=val3"`, + "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, { - name: "with environment flags parsing and env value doesn't exist", + name: "with implicit env flag parsing; values exist in host env", cmdName: "run", fc: &config.Finch{}, - args: []string{"--rm", "-e", "ARG1=val1", "--env=ARG2", "-eARG3", "alpine:latest", "env"}, + args: []string{"-it", "-e", "ARG1", "--env=ARG2", "-eARG3", "--rm", "--name", "myContainer", "alpine:latest", "env"}, wantErr: nil, mockSvc: func( _ *testing.T, @@ -173,22 +174,56 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("ARG1").Return("val1", true) + ncsd.EXPECT().LookupEnv("ARG2").Return("val2", true) + ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true) + ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) + ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", + "-it", "--rm", "--name", `"myContainer"`, "-e", `"ARG1=val1"`, "-e", `"ARG2=val2"`, "-e", `"ARG3=val3"`, + "alpine:latest", "env").Return(c) + c.EXPECT().Run() + }, + }, + { + name: "with implicit env flag parsing; values do not exist", + cmdName: "run", + fc: &config.Finch{}, + args: []string{"--name", "myContainer", "-it", "-e", "ARG0=val0", "-e", "ARG1", "--env=ARG2", "-eARG3", "--rm", "alpine:latest", "env"}, + wantErr: nil, + mockSvc: func( + _ *testing.T, + lcc *mocks.LimaCmdCreator, + _ *mocks.CommandCreator, + ncsd *mocks.NerdctlCommandSystemDeps, + logger *mocks.Logger, + ctrl *gomock.Controller, + _ afero.Fs, + ) { + getVMStatusC := mocks.NewCommand(ctrl) + lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) + getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) + logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("ARG1") ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("ARG3") ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) - + c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", - "--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c) + "--name", `"myContainer"`, "-it", "--rm", "-e", `"ARG0=val0"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, { - name: "with environment flags parsing and env value exists", + name: "with explicit env flag parsing and debug mode", cmdName: "run", fc: &config.Finch{}, - args: []string{"--rm", "--env=ARG2", "-eARG3", "alpine:latest", "env"}, + args: []string{"--debug", "--name", "myContainer", "--rm", "-e", "ARG1=val1", "--env=ARG2=val2", "-it", "-eARG3=val3", "alpine:latest", "env"}, wantErr: nil, mockSvc: func( _ *testing.T, @@ -202,25 +237,60 @@ func TestNerdctlCommand_run(t *testing.T) { getVMStatusC := mocks.NewCommand(ctrl) lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) + logger.EXPECT().SetLevel(flog.Debug) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) + ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) - ncsd.EXPECT().LookupEnv("ARG2") + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", + "--name", `"myContainer"`, "--rm", "-it", "-e", `"ARG1=val1"`, "-e", `"ARG2=val2"`, "-e", `"ARG3=val3"`, + "alpine:latest", "env").Return(c) + c.EXPECT().Run() + }, + }, + { + name: "with implicit env flag parsing and debug mode; values exist in host env", + cmdName: "run", + fc: &config.Finch{}, + args: []string{"--debug", "--rm", "--name", "myContainer", "-e", "ARG1", "--env=ARG2", "-it", "-eARG3", "alpine:latest", "env"}, + wantErr: nil, + mockSvc: func( + _ *testing.T, + lcc *mocks.LimaCmdCreator, + _ *mocks.CommandCreator, + ncsd *mocks.NerdctlCommandSystemDeps, + logger *mocks.Logger, + ctrl *gomock.Controller, + _ afero.Fs, + ) { + getVMStatusC := mocks.NewCommand(ctrl) + lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) + getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) + logger.EXPECT().SetLevel(flog.Debug) + logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("ARG1").Return("val1", true) + ncsd.EXPECT().LookupEnv("ARG2").Return("val2", true) ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", - "--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c) + "--rm", "--name", `"myContainer"`, "-it", "-e", `"ARG1=val1"`, "-e", `"ARG2=val2"`, "-e", `"ARG3=val3"`, + "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, { - name: "with environment flags parsing and env value exists and with --debug flag", + name: "with implicit env flag parsing and debug mode; values do not exist", cmdName: "run", fc: &config.Finch{}, - args: []string{"--debug", "--rm", "--env=ARG2", "-eARG3", "alpine:latest", "env"}, + args: []string{"--debug", "--rm", "-e", "ARG0=val0", "-e", "ARG1", "--env=ARG2", "-it", "--name", "myContainer", "-eARG3", "alpine:latest", "env"}, wantErr: nil, mockSvc: func( _ *testing.T, @@ -239,13 +309,46 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) - c := mocks.NewCommand(ctrl) + ncsd.EXPECT().LookupEnv("ARG1") ncsd.EXPECT().LookupEnv("ARG2") - ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true) + ncsd.EXPECT().LookupEnv("ARG3") ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", - "--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c) + "--rm", "-it", "--name", `"myContainer"`, "-e", `"ARG0=val0"`, "alpine:latest", "env").Return(c) + c.EXPECT().Run() + }, + }, + { + name: "with explicit env flag parsing and image args", + cmdName: "run", + fc: &config.Finch{}, + args: []string{"--debug", "-i", "--name", "myContainer", "--rm", "-e", "ARG1=val1", "--env=ARG2=val2", "-t", "-eARG3=val3", "busybox:latest", "echo", "-e", `"hello\tbye"`}, + wantErr: nil, + mockSvc: func( + _ *testing.T, + lcc *mocks.LimaCmdCreator, + _ *mocks.CommandCreator, + ncsd *mocks.NerdctlCommandSystemDeps, + logger *mocks.Logger, + ctrl *gomock.Controller, + _ afero.Fs, + ) { + getVMStatusC := mocks.NewCommand(ctrl) + lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) + getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) + logger.EXPECT().SetLevel(flog.Debug) + logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) + ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + c := mocks.NewCommand(ctrl) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", + "-i", "--name", `"myContainer"`, "--rm", "-t", "-e", `"ARG1=val1"`, "-e", `"ARG2=val2"`, "-e", `"ARG3=val3"`, + "busybox:latest", "echo", "-e", `"hello\tbye"`).Return(c) c.EXPECT().Run() }, }, @@ -253,7 +356,7 @@ func TestNerdctlCommand_run(t *testing.T) { name: "with --env-file flag replacement", cmdName: "run", fc: &config.Finch{}, - args: []string{"--rm", "--env-file=/env-file", "alpine:latest", "env"}, + args: []string{"-i", "--name", "myContainer", "--rm", "--env-file=/env-file", "alpine:latest", "env"}, wantErr: nil, mockSvc: func( t *testing.T, @@ -280,13 +383,13 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) lcc.EXPECT(). - Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env"). + Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "-i", "--name", `"myContainer"`, "--rm", "-e", `"ARG1=val1"`, "alpine:latest", "env"). Return(c) c.EXPECT().Run() }, }, { - name: "with --env-file flag replacement and with --debug flag", + name: "with --env-file flag replacement and with --debug flag; implicit value not present", cmdName: "run", fc: &config.Finch{}, args: []string{"--debug", "--rm", "--env-file=/env-file", "alpine:latest", "env"}, @@ -317,7 +420,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) lcc.EXPECT(). - Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env"). + Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", `"ARG1=val1"`, "alpine:latest", "env"). Return(c) c.EXPECT().Run() }, @@ -353,7 +456,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) lcc.EXPECT(). - Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", "ARG2=val2", "alpine:latest", "env"). + Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", "--rm", "-e", `"ARG2=val2"`, "alpine:latest", "env"). Return(c) c.EXPECT().Run() }, @@ -560,8 +663,8 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", - "--rm", "-v", "/tmp:/tmp1/tmp2:rro", "--volume", "/tmp:/tmp1:rprivate,rro", "-v=/tmp:/tmp1/tmp2/tmp3/tmp4:rro", - "--volume=/tmp:/tmp1/tmp3/tmp4:rshared", "-v", "volume", "alpine:latest").Return(c) + "--rm", "-v", `"/tmp:/tmp1/tmp2:rro"`, "--volume", `"/tmp:/tmp1:rprivate,rro"`, "-v", `"/tmp:/tmp1/tmp2/tmp3/tmp4:rro"`, + "--volume", `"/tmp:/tmp1/tmp3/tmp4:rshared"`, "-v", `"volume"`, "alpine:latest").Return(c) c.EXPECT().Run() }, }, @@ -594,16 +697,16 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", - "--rm", "-v", "/tmp:/tmp1/tmp2:rro", "--volume", "/tmp:/tmp1:rprivate,rro", - "-v=/tmp:/tmp1/tmp2/tmp3/tmp4:rro", "--volume=/tmp:/tmp1/tmp3/tmp4:rshared", "-v", "volume", "alpine:latest").Return(c) + "--rm", "-v", `"/tmp:/tmp1/tmp2:rro"`, "--volume", `"/tmp:/tmp1:rprivate,rro"`, + "-v", `"/tmp:/tmp1/tmp2/tmp3/tmp4:rro"`, "--volume", `"/tmp:/tmp1/tmp3/tmp4:rshared"`, "-v", `"volume"`, "alpine:latest").Return(c) c.EXPECT().Run() }, }, { - name: "with --help flag", + name: "with --debug flag", cmdName: "pull", fc: &config.Finch{}, - args: []string{"test:tag", "--help"}, + args: []string{"test:tag", "--debug"}, wantErr: nil, mockSvc: func( _ *testing.T, @@ -621,8 +724,38 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + logger.EXPECT().SetLevel(flog.Debug) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + c := mocks.NewCommand(ctrl) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag").Return(c) + c.EXPECT().Run() + }, + }, + { + name: "with --help flag", + cmdName: "pull", + fc: &config.Finch{}, + args: []string{"test:tag", "--help"}, + wantErr: nil, + mockSvc: func( + _ *testing.T, + lcc *mocks.LimaCmdCreator, + _ *mocks.CommandCreator, + ncsd *mocks.NerdctlCommandSystemDeps, + logger *mocks.Logger, + ctrl *gomock.Controller, + _ afero.Fs, + ) { + getVMStatusC := mocks.NewCommand(ctrl) + lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) + getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) + logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) + ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) lcc.EXPECT().RunWithReplacingStdout( testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil) }, @@ -646,11 +779,11 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) - ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) - ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) lcc.EXPECT().RunWithReplacingStdout( testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help"). Return(fmt.Errorf("failed to replace")) @@ -682,7 +815,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName, - "push", "--sign=cosign", "test:tag").Return(c) + "push", "--sign", `"cosign"`, "test:tag").Return(c) c.EXPECT().Run() }, }, @@ -712,7 +845,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName, - "pull", "--verify=cosign", "test:tag").Return(c) + "pull", "--verify", `"cosign"`, "test:tag").Return(c) c.EXPECT().Run() }, }, diff --git a/cmd/finch/nerdctl_windows_test.go b/cmd/finch/nerdctl_windows_test.go index acc3c8d67..3c46a4341 100644 --- a/cmd/finch/nerdctl_windows_test.go +++ b/cmd/finch/nerdctl_windows_test.go @@ -173,10 +173,10 @@ func TestNerdctlCommand_run(t *testing.T) { }, }, { - name: "with environment flags parsing and env value doesn't exist", + name: "with implicit env flag parsing; values do not exist", cmdName: "run", fc: &config.Finch{}, - args: []string{"--rm", "-e", "ARG1=val1", "--env=ARG2", "-eARG3", "alpine:latest", "env"}, + args: []string{"-it", "-e", "ARG0=val0", "-e", "ARG1", "--env=ARG2", "-eARG3", "--rm", "alpine:latest", "env"}, wantErr: nil, mockSvc: func( _ *testing.T, @@ -196,6 +196,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) c := mocks.NewCommand(ctrl) + ncsd.EXPECT().LookupEnv("ARG1") ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("ARG3") ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) @@ -206,7 +207,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", - "--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c) + "-it", "--rm", "-e", `"ARG0=val0"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, @@ -244,7 +245,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) // alias substitution run=>container run lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", - "--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c) + "--rm", "-e", `"ARG3=val3"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, @@ -283,7 +284,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) // alias substitution run=>container run lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", - "--rm", "-e", "ARG3=val3", "alpine:latest", "env").Return(c) + "--rm", "-e", `"ARG3=val3"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, @@ -323,7 +324,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath) ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, - "sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c) + "sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", `"ARG1=val1"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, @@ -364,7 +365,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath) ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, - "sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", "ARG1=val1", "alpine:latest", "env").Return(c) + "sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", `"ARG1=val1"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, @@ -404,7 +405,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath) ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, - "sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", "ARG2=val2", "alpine:latest", "env").Return(c) + "sudo", "-E", nerdctlCmdName, "container", "run", "--rm", "-e", `"ARG2=val2"`, "alpine:latest", "env").Return(c) c.EXPECT().Run() }, }, @@ -646,7 +647,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath).Times(3) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", - "--rm", "-v", "/mnt/c/workdir:/tmp1/tmp2:rro", "-v=/mnt/c/workdir:/tmp1/tmp2/tmp3/tmp4:rro", + "--rm", "-v", `"/mnt/c/workdir:/tmp1/tmp2:rro"`, "-v", `"/mnt/c/workdir:/tmp1/tmp2/tmp3/tmp4:rro"`, "-v", "volume", "alpine:latest").Return(c) c.EXPECT().Run() }, @@ -750,7 +751,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName, - "push", "--sign=cosign", "test:tag").Return(c) + "push", "--sign", `"cosign"`, "test:tag").Return(c) c.EXPECT().Run() }, }, @@ -785,7 +786,7 @@ func TestNerdctlCommand_run(t *testing.T) { ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName, - "pull", "--verify=cosign", "test:tag").Return(c) + "pull", "--verify", `"cosign"`, "test:tag").Return(c) c.EXPECT().Run() }, }, @@ -977,7 +978,7 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) { c := mocks.NewCommand(ctrl) // alias substitution, run => container run lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, - "sudo", "-E", nerdctlCmdName, "container", "run", + "sudo", "-E", nerdctlCmdName, "container", "run", "--mount", ContainsStr("source=/mnt/c/workdir"), "alpine:latest").Return(c) c.EXPECT().Run() }, @@ -1050,7 +1051,7 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) { // alias substitution, run => container run lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "--mount", - "type=notbind,source=C:/workdir,target=/app", "alpine:latest").Return(c) + `"type=notbind,source=C:/workdir,target=/app"`, "alpine:latest").Return(c) c.EXPECT().Run() }, }, @@ -1249,6 +1250,7 @@ func TestNerdctlCommand_run_BuildCommand(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + logger.EXPECT().Debugln("Unexpected Arg Value: /mnt/c/workdir/buildcontext") ncsd.EXPECT().LookupEnv("AWS_ACCESS_KEY_ID").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) @@ -1299,7 +1301,7 @@ func TestNerdctlCommand_run_BuildCommand(t *testing.T) { c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "image", "build", "--secret", - fmt.Sprintf("src=%s", wslSecretPath), wslBuildContextPath).Return(c) + fmt.Sprintf(`"src=%s"`, wslSecretPath), wslBuildContextPath).Return(c) c.EXPECT().Run() }, }, diff --git a/go.mod b/go.mod index 38add8c4c..7ff4df354 100644 --- a/go.mod +++ b/go.mod @@ -81,6 +81,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/sergi/go-diff v1.3.1 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/wk8/go-ordered-map v1.0.0 go.opencensus.io v0.24.0 // indirect golang.org/x/mod v0.20.0 // indirect golang.org/x/net v0.28.0 // indirect diff --git a/go.sum b/go.sum index d87462fb6..d126e21e9 100644 --- a/go.sum +++ b/go.sum @@ -171,6 +171,7 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= @@ -187,6 +188,8 @@ github.com/tklauser/numcpus v0.7.0 h1:yjuerZP127QG9m5Zh/mSO4wqurYil27tHrqwRoRjpr github.com/tklauser/numcpus v0.7.0/go.mod h1:bb6dMVcj8A42tSE7i32fsIUCbQNllK5iDguyOZRUzAY= github.com/urfave/cli/v2 v2.25.7 h1:VAzn5oq403l5pHjc4OhD54+XGO9cdKVL/7lDjF+iKUs= github.com/urfave/cli/v2 v2.25.7/go.mod h1:8qnjx1vcq5s2/wpsqoZFndg2CE5tNFyrTvS6SinrnYQ= +github.com/wk8/go-ordered-map v1.0.0 h1:BV7z+2PaK8LTSd/mWgY12HyMAo5CEgkHqbkVq2thqr8= +github.com/wk8/go-ordered-map v1.0.0/go.mod h1:9ZIbRunKbuvfPKyBP1SIKLcXNlv74YCOZ3t3VTS6gRk= github.com/xorcare/pointer v1.2.2 h1:zjD77b5DTehClND4MK+9dDE0DcpFIZisAJ/+yVJvKYA= github.com/xorcare/pointer v1.2.2/go.mod h1:azsKh7oVwYB7C1o8P284fG8MvtErX/F5/dqXiaj71ak= github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU=