Skip to content

Commit

Permalink
fix: conflict in parsing of -h flag for help and hostname in containe…
Browse files Browse the repository at this point in the history 
…r run (#1251)

fix: conflict in parsing of -h flag for help and hostname in container run

Signed-off-by: Shubharanshu Mahapatra <[email protected]>
  • Loading branch information
@Shubhranshu153
Shubhranshu153 authored Jan 23, 2025
1 parent 67ab6f8 commit 8b8df27
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 49 deletions.
4 changes: 3 additions & 1 deletion cmd/finch/nerdctl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ func (nc *nerdctlCommand) shouldReplaceForHelp(cmdName string, args []string) bo
}
}

// this needs to handle cases of -h except for `container run`,`run`,`create`. For these options -h represent hostname
// TODO: Refactor this function.
for _, arg := range args {
if arg == "--help" || arg == "-h" {
if arg == "--help" || (arg == "-h" && !(cmdName == "container run" || cmdName == "run" || cmdName == "create")) {
return true
}
}
Expand Down
72 changes: 49 additions & 23 deletions cmd/finch/nerdctl_remote.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,12 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
case arg == "--help":
nerdctlArgs = append(nerdctlArgs, arg)
case arg == "--add-host":
// exact match to --add-host
args[i+1], err = resolveIP(args[i+1], nc.logger, nc.ecc)
if err != nil {
return err
// exact match to --add-host. resolve ip if param passed
if len(args) > i+1 {
args[i+1], err = resolveIP(args[i+1], nc.logger, nc.ecc)
if err != nil {
return err
}
}
nerdctlArgs = append(nerdctlArgs, arg)
case strings.HasPrefix(arg, "--add-host"):
Expand All @@ -158,21 +160,33 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
case strings.HasPrefix(arg, "--env-file"):
// exact match to --env-file
// or arg begins with --env-file
shouldSkip, addEnvs, err := handleEnvFile(nc.fs, nc.systemDeps, arg, args[i+1])
if err != nil {
return err
if len(args) > i+1 {
shouldSkip, addEnvs, err := handleEnvFile(nc.fs, nc.systemDeps, arg, args[i+1])
if err != nil {
return err
}
skip = shouldSkip
fileEnvs = append(fileEnvs, addEnvs...)
} else {
// if --env-file is at the end of the args, its refers to entrypoint command
// which need not be handled
nerdctlArgs = append(nerdctlArgs, arg)
}
skip = shouldSkip
fileEnvs = append(fileEnvs, addEnvs...)
case argIsEnv(arg):
// exact match to either -e or --env
// or arg begins with -e or --env
// -e="<value>", -e"<value>"
// --env="<key>=<value>", --env"<key>=<value>"
shouldSkip, addEnv := handleEnv(nc.systemDeps, arg, args[i+1])
skip = shouldSkip
if addEnv != "" {
envs = append(envs, addEnv)
if len(args) > i+1 {
shouldSkip, addEnv := handleEnv(nc.systemDeps, arg, args[i+1])
skip = shouldSkip
if addEnv != "" {
envs = append(envs, addEnv)
}
} else {
// if -e or --env is at the end of the args, its refers to entrypoint command
// which need not be handled
nerdctlArgs = append(nerdctlArgs, arg)
}
case shortFlagBoolSet.Has(arg) || longFlagBoolSet.Has(arg):
// exact match to a short no argument flag: -?
Expand All @@ -195,23 +209,35 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
// or begins with a short arg flag:
// short arg flag concatenated to value: -?"<value>"
// short arg flag equated to value: -?="<value>" or -?=<value>
shouldSkip, addKey, addVal := nc.handleFlagArg(arg, args[i+1])
skip = shouldSkip
if addKey != "" {
nerdctlArgs = append(nerdctlArgs, addKey)
nerdctlArgs = append(nerdctlArgs, addVal)
if len(args) > i+1 {
shouldSkip, addKey, addVal := nc.handleFlagArg(arg, args[i+1])
skip = shouldSkip
if addKey != "" {
nerdctlArgs = append(nerdctlArgs, addKey)
nerdctlArgs = append(nerdctlArgs, addVal)
}
} else {
// no value found for short arg flag
// pass the arg as a nerdctl command argument
nerdctlArgs = append(nerdctlArgs, arg)
}
case strings.HasPrefix(arg, "--"):
// exact match to a long arg flag: -<long_flag>
// next arg must be the <value>
// or begins with a long arg flag:
// long arg flag concatenated to value: --<long_flag>"<value>"
// long arg flag equated to value: --<long_flag>="<value>" or --<long_flag>=<value>
shouldSkip, addKey, addVal := nc.handleFlagArg(arg, args[i+1])
skip = shouldSkip
if addKey != "" {
nerdctlArgs = append(nerdctlArgs, addKey)
nerdctlArgs = append(nerdctlArgs, addVal)
if len(args) > i+1 {
shouldSkip, addKey, addVal := nc.handleFlagArg(arg, args[i+1])
skip = shouldSkip
if addKey != "" {
nerdctlArgs = append(nerdctlArgs, addKey)
nerdctlArgs = append(nerdctlArgs, addVal)
}
} else {
// if --<long flag> is at the end of the args, its refers to entrypoint command
// which need not be handled
nerdctlArgs = append(nerdctlArgs, arg)
}
default:
// arg other than a flag ("-?","--<long_flag>") or a skipped <flag_value>
Expand Down
79 changes: 54 additions & 25 deletions cmd/finch/nerdctl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,48 +30,76 @@ func TestNerdctlCommand_shouldReplaceForHelp(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
cmdName string
args []string
mockSvc func(*mocks.NerdctlCmdCreator, *mocks.Logger, *gomock.Controller)
name string
cmdName string
args []string
expected bool
mockSvc func(*mocks.NerdctlCmdCreator, *mocks.Logger, *gomock.Controller)
}{
{
name: "with --help flag",
cmdName: "pull",
args: []string{"test:tag", "--help"},
name: "with --help flag",
cmdName: "pull",
args: []string{"test:tag", "--help"},
expected: true,
},
{
name: "with -h",
cmdName: "pull",
args: []string{"test:tag", "-h"},
name: "with -h",
cmdName: "pull",
args: []string{"test:tag", "-h"},
expected: true,
},
{
name: "system returns help",
cmdName: "system",
name: "system returns help",
cmdName: "system",
expected: true,
},
{
name: "builder returns help",
cmdName: "builder",
name: "builder returns help",
cmdName: "builder",
expected: true,
},
{
name: "container returns help",
cmdName: "container",
name: "container returns help",
cmdName: "container",
expected: true,
},
{
name: "image returns help",
cmdName: "image",
name: "image returns help",
cmdName: "image",
expected: true,
},
{
name: "network returns help",
cmdName: "network",
name: "network returns help",
cmdName: "network",
expected: true,
},
{
name: "volume returns help",
cmdName: "volume",
name: "volume returns help",
cmdName: "volume",
expected: true,
},
{
name: "compose returns help",
cmdName: "compose",
name: "compose returns help",
cmdName: "compose",
expected: true,
},
{
name: "-h argument for hostname-related command",
cmdName: "run",
args: []string{"-h"},
expected: false,
},
{
name: "-h argument for hostname-related command",
cmdName: "container run",
args: []string{"-h"},
expected: false,
},
{
name: "-h argument for hostname-related command",
cmdName: "create",
args: []string{"-h"},
expected: false,
},
}

Expand All @@ -85,7 +113,8 @@ func TestNerdctlCommand_shouldReplaceForHelp(t *testing.T) {
ecc := mocks.NewCommandCreator(ctrl)
ncsd := mocks.NewNerdctlCommandSystemDeps(ctrl)
logger := mocks.NewLogger(ctrl)
assert.True(t, newNerdctlCommand(ncc, ecc, ncsd, logger, nil, &config.Finch{}).shouldReplaceForHelp(tc.cmdName, tc.args))
assert.True(t, (newNerdctlCommand(ncc, ecc, ncsd, logger,
nil, &config.Finch{}).shouldReplaceForHelp(tc.cmdName, tc.args) == tc.expected))
})
}
}

0 comments on commit 8b8df27

Please sign in to comment.