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

Signed-off-by: Shubharanshu Mahapatra <shubhum@amazon.com>

Author: Shubhranshu153

cmd/finch/nerdctl.go

@@ -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
 		}
 	}

cmd/finch/nerdctl_remote.go

@@ -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"):
@@ -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: -?
@@ -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>

cmd/finch/nerdctl_test.go

@@ -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,
 		},
 	}
 
@@ -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))
 		})
 	}
 }