Merge pull request #14209 from samuelkarp/ctr-stop-on-nth-arg

ctr: fix flag parsing issues in run, containers create, tasks exec, and oci-hook

cmd/containerd/command/oci-hook.go

@@ -57,6 +57,7 @@ var ociHook = &cli.Command{
 		}
 		return syscall.Exec(args[0], args, env)
 	},
+	StopOnNthArg: new(1),
 }
 
 // hookSpec is a shallow version of [oci.Spec] containing only the

cmd/containerd/command/oci-hook_test.go

@@ -0,0 +1,63 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package command
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v3"
+)
+
+func TestOCIHookStopFlagParsing(t *testing.T) {
+	wantArgs := []string{
+		"/usr/local/bin/my-hook",
+		"--id", "{{id}}",
+		"--config", "/etc/hook/config.toml",
+		"-l", "debug",
+	}
+
+	for _, tc := range []struct {
+		name string
+		args []string
+	}{
+		{
+			name: "without separator",
+			args: append([]string{"oci-hook"}, wantArgs...),
+		},
+		{
+			name: "with double-dash separator",
+			args: append([]string{"oci-hook", "--"}, wantArgs...),
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cmd := *ociHook
+			var capturedArgs []string
+
+			cmd.Action = func(ctx context.Context, c *cli.Command) error {
+				capturedArgs = c.Args().Slice()
+				return nil
+			}
+
+			err := cmd.Run(context.Background(), tc.args)
+			require.NoError(t, err)
+			assert.Equal(t, wantArgs, capturedArgs)
+		})
+	}
+}

cmd/ctr/commands/containers/containers.go

@@ -90,6 +90,7 @@ var createCommand = &cli.Command{
 		return nil
 	},
 	DisableSliceFlagSeparator: true,
+	StopOnNthArg:              new(2),
 }
 
 var listCommand = &cli.Command{

cmd/ctr/commands/containers/containers_test.go

@@ -117,3 +117,35 @@ func TestContainersCommandHierarchyFlagParsing(t *testing.T) {
 	}
 	assert.Equal(t, wantEnvs, capturedEnvs)
 }
+
+func TestContainersCreateCommandProcessArgsWithFlags(t *testing.T) {
+	cmd := *createCommand
+	var (
+		capturedRef  string
+		capturedID   string
+		capturedArgs []string
+	)
+
+	cmd.Action = func(ctx context.Context, c *cli.Command) error {
+		capturedRef = c.Args().First()
+		capturedID = c.Args().Get(1)
+		capturedArgs = c.Args().Slice()[2:]
+		return nil
+	}
+
+	args := []string{
+		"create",
+		"docker.io/library/busybox:latest",
+		"test-container",
+		"sh",
+		"-uexc",
+		"echo hello",
+	}
+
+	err := cmd.Run(context.Background(), args)
+	require.NoError(t, err)
+
+	assert.Equal(t, "docker.io/library/busybox:latest", capturedRef)
+	assert.Equal(t, "test-container", capturedID)
+	assert.Equal(t, []string{"sh", "-uexc", "echo hello"}, capturedArgs)
+}

cmd/ctr/commands/run/run.go

@@ -144,6 +144,7 @@ var Command = &cli.Command{
 			append(append(commands.SnapshotterFlags, []cli.Flag{commands.SnapshotterLabels}...),
 				commands.ContainerFlags...)...)...)...),
 	DisableSliceFlagSeparator: true,
+	StopOnNthArg:              new(2),
 	Action: func(ctx context.Context, cmd *cli.Command) error {
 		var (
 			err error

cmd/ctr/commands/run/run_test.go

@@ -165,3 +165,42 @@ func TestRunCommandFlagParsing(t *testing.T) {
 	}
 	assert.Equal(t, wantAnnotations, capturedAnnotations)
 }
+
+func TestRunCommandProcessArgsWithFlags(t *testing.T) {
+	cmd := *Command
+	var (
+		capturedRef    string
+		capturedID     string
+		capturedArgs   []string
+		capturedRM     bool
+		capturedDetach bool
+	)
+
+	cmd.Action = func(ctx context.Context, c *cli.Command) error {
+		capturedRef = c.Args().First()
+		capturedID = c.Args().Get(1)
+		capturedArgs = c.Args().Slice()[2:]
+		capturedRM = c.Bool("rm")
+		capturedDetach = c.Bool("detach")
+		return nil
+	}
+
+	args := []string{
+		"run",
+		"--rm",
+		"docker.io/library/busybox:latest",
+		"test-container",
+		"sh",
+		"-uexc",
+		"echo -n hello && ls -d /tmp",
+	}
+
+	err := cmd.Run(context.Background(), args)
+	require.NoError(t, err)
+
+	assert.True(t, capturedRM)
+	assert.False(t, capturedDetach)
+	assert.Equal(t, "docker.io/library/busybox:latest", capturedRef)
+	assert.Equal(t, "test-container", capturedID)
+	assert.Equal(t, []string{"sh", "-uexc", "echo -n hello && ls -d /tmp"}, capturedArgs)
+}

cmd/ctr/commands/tasks/exec.go

@@ -70,6 +70,7 @@ var execCommand = &cli.Command{
 		},
 	},
 	DisableSliceFlagSeparator: true,
+	StopOnNthArg:              new(1),
 	Action: func(ctx context.Context, cmd *cli.Command) error {
 		var (
 			id     = cmd.Args().First()

cmd/ctr/commands/tasks/exec_test.go

@@ -0,0 +1,65 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package tasks
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v3"
+)
+
+func TestExecCommandProcessArgsWithFlags(t *testing.T) {
+	cmd := *execCommand
+	var (
+		capturedID     string
+		capturedArgs   []string
+		capturedExecID string
+		capturedTTY    bool
+		capturedDetach bool
+	)
+
+	cmd.Action = func(ctx context.Context, c *cli.Command) error {
+		capturedID = c.Args().First()
+		capturedArgs = c.Args().Tail()
+		capturedExecID = c.String("exec-id")
+		capturedTTY = c.Bool("tty")
+		capturedDetach = c.Bool("detach")
+		return nil
+	}
+
+	args := []string{
+		"exec",
+		"--exec-id", "exec-1",
+		"-t",
+		"test-container",
+		"sh",
+		"-uexc",
+		"ls -d /tmp",
+	}
+
+	err := cmd.Run(context.Background(), args)
+	require.NoError(t, err)
+
+	assert.Equal(t, "exec-1", capturedExecID)
+	assert.True(t, capturedTTY)
+	assert.False(t, capturedDetach)
+	assert.Equal(t, "test-container", capturedID)
+	assert.Equal(t, []string{"sh", "-uexc", "ls -d /tmp"}, capturedArgs)
+}