diff --git a/cmd/root/run.go b/cmd/root/run.go index 417503c98..c01025965 100644 --- a/cmd/root/run.go +++ b/cmd/root/run.go @@ -144,9 +144,8 @@ func (f *runExecFlags) runRunCommand(cmd *cobra.Command, args []string) (command }() } - // If --sandbox is set, delegate everything to docker sandbox. if f.sandbox { - return runInSandbox(cmd, &f.runConfig, f.sandboxTemplate) + return runInSandbox(ctx, cmd, args, &f.runConfig, f.sandboxTemplate) } out := cli.NewPrinter(cmd.OutOrStdout()) diff --git a/cmd/root/sandbox.go b/cmd/root/sandbox.go index 0d523f825..b888e980e 100644 --- a/cmd/root/sandbox.go +++ b/cmd/root/sandbox.go @@ -2,14 +2,17 @@ package root import ( "cmp" + "context" "errors" "fmt" "log/slog" "os" "os/exec" + "path/filepath" "github.com/docker/cli/cli" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/docker/docker-agent/pkg/config" "github.com/docker/docker-agent/pkg/environment" @@ -20,30 +23,35 @@ import ( // runInSandbox delegates the current command to a Docker sandbox. // It ensures a sandbox exists (creating or recreating as needed), then // executes docker agent inside it via `docker sandbox exec`. -func runInSandbox(cmd *cobra.Command, runConfig *config.RuntimeConfig, template string) error { +func runInSandbox(ctx context.Context, cmd *cobra.Command, args []string, runConfig *config.RuntimeConfig, template string) error { if environment.InSandbox() { return fmt.Errorf("already running inside a Docker sandbox (VM %s)", os.Getenv("SANDBOX_VM_ID")) } - ctx := cmd.Context() if err := sandbox.CheckAvailable(ctx); err != nil { return err } - dockerAgentArgs := sandbox.BuildCagentArgs(os.Args) - agentRef := sandbox.AgentRefFromArgs(dockerAgentArgs) - configDir := paths.GetConfigDir() + var agentRef string + if len(args) > 0 { + agentRef = args[0] + } - // Always forward config directory paths so the sandbox-side - // docker agent resolves it to the same host directories - // (which is mounted read-write by ensureSandbox). - dockerAgentArgs = sandbox.AppendFlagIfMissing(dockerAgentArgs, "--config-dir", configDir) + configDir := paths.GetConfigDir() + dockerAgentArgs := dockerAgentArgs(cmd, args, configDir) + dockerAgentArgs = append(dockerAgentArgs, args...) + dockerAgentArgs = append(dockerAgentArgs, "--config-dir", configDir) stopTokenWriter := sandbox.StartTokenWriterIfNeeded(ctx, configDir, runConfig.ModelsGateway) defer stopTokenWriter() - // Ensure a sandbox with the right workspace mounts exists. - wd := cmp.Or(runConfig.WorkingDir, ".") + // Resolve wd to an absolute path so that it matches the absolute + // workspace paths returned by `docker sandbox ls --json`. + wd, err := filepath.Abs(cmp.Or(runConfig.WorkingDir, ".")) + if err != nil { + return fmt.Errorf("resolving workspace path: %w", err) + } + name, err := sandbox.Ensure(ctx, wd, sandbox.ExtraWorkspace(wd, agentRef), template, configDir) if err != nil { return err @@ -60,7 +68,7 @@ func runInSandbox(cmd *cobra.Command, runConfig *config.RuntimeConfig, template envFlags = append(envFlags, "-e", envModelsGateway+"="+gateway) } - dockerCmd := sandbox.BuildExecCmd(ctx, name, dockerAgentArgs, envFlags, envVars) + dockerCmd := sandbox.BuildExecCmd(ctx, name, wd, dockerAgentArgs, envFlags, envVars) slog.Debug("Executing in sandbox", "name", name, "args", dockerCmd.Args) if err := dockerCmd.Run(); err != nil { @@ -72,3 +80,31 @@ func runInSandbox(cmd *cobra.Command, runConfig *config.RuntimeConfig, template return nil } + +func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []string { + var dockerAgentArgs []string + hasYolo := false + cmd.Flags().Visit(func(f *pflag.Flag) { + if f.Name == "sandbox" || f.Name == "config-dir" { + return + } + + if f.Name == "yolo" { + hasYolo = true + } + + if f.Value.Type() == "bool" { + dockerAgentArgs = append(dockerAgentArgs, "--"+f.Name) + } else { + dockerAgentArgs = append(dockerAgentArgs, "--"+f.Name, f.Value.String()) + } + }) + if !hasYolo { + dockerAgentArgs = append(dockerAgentArgs, "--yolo") + } + + dockerAgentArgs = append(dockerAgentArgs, args...) + dockerAgentArgs = append(dockerAgentArgs, "--config-dir", configDir) + + return dockerAgentArgs +} diff --git a/go.mod b/go.mod index 3090e6145..bc7d75e6e 100644 --- a/go.mod +++ b/go.mod @@ -193,7 +193,7 @@ require ( github.com/sergi/go-diff v1.4.0 // indirect github.com/sirupsen/logrus v1.9.4 // indirect github.com/skeema/knownhosts v1.3.1 // indirect - github.com/spf13/pflag v1.0.10 // indirect + github.com/spf13/pflag v1.0.10 github.com/stretchr/objx v0.5.2 // indirect github.com/tidwall/gjson v1.18.0 // indirect github.com/tidwall/match v1.1.1 // indirect diff --git a/pkg/sandbox/args.go b/pkg/sandbox/args.go index e025daf7a..baf1794c9 100644 --- a/pkg/sandbox/args.go +++ b/pkg/sandbox/args.go @@ -1,119 +1,13 @@ package sandbox import ( - "log/slog" "os" "path/filepath" "strings" - "github.com/docker/cli/cli-plugins/plugin" - "github.com/docker/docker-agent/pkg/userconfig" ) -// Flags stripped when forwarding args to the sandbox. -var ( - // stripFlags are standalone boolean flags that don't take a value. - stripFlags = map[string]bool{ - "--sandbox": true, - "--debug": true, - "-d": true, - } - // stripFlagsWithValue are flags followed by a separate value argument. - // They are also recognised in --flag=value form. - stripFlagsWithValue = map[string]bool{ - "--log-file": true, - "--template": true, - "--models-gateway": true, - } -) - -// BuildCagentArgs takes os.Args and returns the arguments to pass after -// "--" to the sandbox. It strips the binary name, "--sandbox", and the first -// occurrence of the "run" subcommand. If the agent reference is a user-defined -// alias, it is resolved to its path so the sandbox (which lacks the host's -// user config) receives a concrete reference. -// It also injects --yolo since the sandbox provides isolation. -// Host-only flags --debug/-d, --log-file, --template, and --models-gateway -// are stripped because they reference host paths/logging, sandbox creation -// options, or settings forwarded via env var that don't apply inside the -// sandbox. -// -// ["cagent", "run", "./agent.yaml", "--sandbox", "--debug"] → ["./agent.yaml", "--yolo"] -// ["cagent", "--debug", "run", "--sandbox", "myalias"] → ["/path/to/agent.yaml", "--yolo"] -func BuildCagentArgs(argv []string) []string { - out := make([]string, 0, len(argv)) - runStripped := false - agentStripped := false - agentResolved := false - hasYolo := false - skipNext := false - for _, a := range argv[1:] { // skip binary name - if skipNext { - skipNext = false - continue - } - if stripFlags[a] { - continue - } - if stripFlagsWithValue[a] { - skipNext = true - continue - } - if isEqualsFormOf(a, stripFlagsWithValue) { - continue - } - if a == "--yolo" { - hasYolo = true - } - if !runStripped && a == "run" { - runStripped = true - continue - } - if !plugin.RunningStandalone() && !agentStripped && a == "agent" { - agentStripped = true - continue - } - // The first positional arg after "run" is the agent reference. - // Resolve it if it's a user-defined alias. - if runStripped && !agentResolved && !strings.HasPrefix(a, "-") { - agentResolved = true - if resolved := ResolveAlias(a); resolved != "" { - slog.Debug("Resolved alias for sandbox", "alias", a, "path", resolved) - a = resolved - } - } - out = append(out, a) - } - // The sandbox provides isolation, so auto-approve all tool calls. - if !hasYolo { - out = append(out, "--yolo") - } - return out -} - -// isEqualsFormOf reports whether arg matches "--flag=..." for any flag in the set. -func isEqualsFormOf(arg string, flags map[string]bool) bool { - for f := range flags { - if strings.HasPrefix(arg, f+"=") { - return true - } - } - return false -} - -// AgentRefFromArgs returns the first positional (non-flag) argument from the -// docker-agent arg list, which is the agent file reference. Returns "" if there are -// no positional arguments. -func AgentRefFromArgs(args []string) string { - for _, a := range args { - if !strings.HasPrefix(a, "-") { - return a - } - } - return "" -} - // ResolveAlias returns the alias path if name is a user-defined alias, // or an empty string otherwise. func ResolveAlias(name string) string { @@ -167,14 +61,3 @@ func looksLikeLocalFile(path string) bool { info, err := os.Stat(path) return err == nil && !info.IsDir() } - -// AppendFlagIfMissing appends "--flag value" to args unless args already -// contain the flag (in either "--flag value" or "--flag=value" form). -func AppendFlagIfMissing(args []string, flag, value string) []string { - for _, a := range args { - if a == flag || strings.HasPrefix(a, flag+"=") { - return args - } - } - return append(args, flag, value) -} diff --git a/pkg/sandbox/sandbox.go b/pkg/sandbox/sandbox.go index 7c3d6ccd8..0d61e4560 100644 --- a/pkg/sandbox/sandbox.go +++ b/pkg/sandbox/sandbox.go @@ -106,7 +106,7 @@ func Ensure(ctx context.Context, wd, extra, template, configDir string) (string, createArgs = append(createArgs, "-t", template) } createArgs = append(createArgs, "cagent", wd) - if extra != "" { + if extra != "" && extra != wd { createArgs = append(createArgs, extra+":ro") } // Mount config directory read-only so the sandbox can @@ -133,8 +133,8 @@ func Ensure(ctx context.Context, wd, extra, template, configDir string) (string, } // BuildExecCmd assembles the `docker sandbox exec` command. -func BuildExecCmd(ctx context.Context, name string, cagentArgs, envFlags, envVars []string) *exec.Cmd { - execArgs := []string{"sandbox", "exec", "-it"} +func BuildExecCmd(ctx context.Context, name, wd string, cagentArgs, envFlags, envVars []string) *exec.Cmd { + execArgs := []string{"sandbox", "exec", "-it", "-w", wd} execArgs = append(execArgs, envFlags...) execArgs = append(execArgs, name, "cagent", "run") execArgs = append(execArgs, cagentArgs...) diff --git a/pkg/sandbox/sandbox_test.go b/pkg/sandbox/sandbox_test.go index f32571c9d..7721ce8f7 100644 --- a/pkg/sandbox/sandbox_test.go +++ b/pkg/sandbox/sandbox_test.go @@ -6,12 +6,10 @@ import ( "path/filepath" "testing" - "github.com/docker/cli/cli-plugins/metadata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/docker/docker-agent/pkg/sandbox" - "github.com/docker/docker-agent/pkg/userconfig" ) func TestCheckAvailable(t *testing.T) { @@ -118,155 +116,6 @@ func TestExisting_HasWorkspace(t *testing.T) { assert.False(t, s.HasWorkspace("/other")) } -func TestBuildCagentArgs(t *testing.T) { - tests := []struct { - name string - env map[string]string - argv []string - want []string - }{ - { - name: "using cli plugin, strips run and --sandbox and --debug, adds --yolo", - env: map[string]string{metadata.ReexecEnvvar: "docker"}, // env var set by cli plugin execution - argv: []string{"docker", "agent", "run", "./agent.yaml", "--sandbox", "--debug"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "using cli plugin, strips -d shorthand", - env: map[string]string{metadata.ReexecEnvvar: "docker"}, // env var set by cli plugin execution - argv: []string{"docker", "agent", "-d", "run", "./agent.yaml"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips run and --sandbox and --debug, adds --yolo", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox", "--debug"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --sandbox at end", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --debug before run", - argv: []string{"cagent", "--debug", "run", "--sandbox", "default"}, - want: []string{"default", "--yolo"}, - }, - { - name: "strips -d shorthand", - argv: []string{"cagent", "-d", "run", "./agent.yaml"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --log-file with value", - argv: []string{"cagent", "run", "./agent.yaml", "--debug", "--log-file", "/tmp/debug.log"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --log-file=value", - argv: []string{"cagent", "run", "./agent.yaml", "--log-file=/tmp/debug.log"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "no --sandbox present, still strips debug flags", - argv: []string{"cagent", "run", "./agent.yaml", "--debug"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "only binary and run", - argv: []string{"cagent", "run"}, - want: []string{"--yolo"}, - }, - { - name: "does not duplicate --yolo", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox", "--yolo"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "only strips first run occurrence", - argv: []string{"cagent", "run", "--sandbox", "run.yaml"}, - want: []string{"run.yaml", "--yolo"}, - }, - { - name: "sandbox and run only", - argv: []string{"cagent", "run", "--sandbox"}, - want: []string{"--yolo"}, - }, - { - name: "strips --template with value", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox", "--template", "my-image:latest"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --template=value", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox", "--template=my-image:latest"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --models-gateway with value", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox", "--models-gateway", "http://gw:8080"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - { - name: "strips --models-gateway=value", - argv: []string{"cagent", "run", "./agent.yaml", "--sandbox", "--models-gateway=http://gw:8080"}, - want: []string{"./agent.yaml", "--yolo"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Use an empty HOME so no real user aliases interfere. - t.Setenv("HOME", t.TempDir()) - if tt.env != nil { - for k, v := range tt.env { - t.Setenv(k, v) - } - } - - got := sandbox.BuildCagentArgs(tt.argv) - assert.Equal(t, tt.want, got) - }) - } -} - -func TestBuildCagentArgs_ResolvesAlias(t *testing.T) { - // Set up a temporary HOME with an alias "mydev" -> "/path/to/dev.yaml". - home := t.TempDir() - t.Setenv("HOME", home) - - cfg, err := userconfig.Load() - require.NoError(t, err) - require.NoError(t, cfg.SetAlias("mydev", &userconfig.Alias{Path: "/path/to/dev.yaml"})) - require.NoError(t, cfg.Save()) - - got := sandbox.BuildCagentArgs([]string{"cagent", "run", "--sandbox", "mydev", "--debug"}) - assert.Equal(t, []string{"/path/to/dev.yaml", "--yolo"}, got) -} - -func TestAgentRefFromArgs(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - args []string - want string - }{ - {"file first", []string{"./agent.yaml", "--debug"}, "./agent.yaml"}, - {"flags before file", []string{"--debug", "./agent.yaml"}, "./agent.yaml"}, - {"no positional", []string{"--debug", "--yolo"}, ""}, - {"empty", nil, ""}, - {"built-in name", []string{"default"}, "default"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - assert.Equal(t, tt.want, sandbox.AgentRefFromArgs(tt.args)) - }) - } -} - func TestExtraWorkspace(t *testing.T) { tests := []struct { name string @@ -322,52 +171,3 @@ func TestExtraWorkspace(t *testing.T) { }) } } - -func TestAppendFlagIfMissing(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - args []string - flag string - value string - want []string - }{ - { - name: "appends when missing", - args: []string{"./agent.yaml", "--yolo"}, - flag: "--models-gateway", - value: "http://gw:8080", - want: []string{"./agent.yaml", "--yolo", "--models-gateway", "http://gw:8080"}, - }, - { - name: "skips when present as separate arg", - args: []string{"./agent.yaml", "--models-gateway", "http://gw:8080"}, - flag: "--models-gateway", - value: "http://other:9090", - want: []string{"./agent.yaml", "--models-gateway", "http://gw:8080"}, - }, - { - name: "skips when present as equals form", - args: []string{"./agent.yaml", "--data-dir=/custom/path"}, - flag: "--data-dir", - value: "/default/path", - want: []string{"./agent.yaml", "--data-dir=/custom/path"}, - }, - { - name: "empty args", - args: nil, - flag: "--data-dir", - value: "/some/path", - want: []string{"--data-dir", "/some/path"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got := sandbox.AppendFlagIfMissing(tt.args, tt.flag, tt.value) - assert.Equal(t, tt.want, got) - }) - } -}