Fix exec command strings (#5470)

* fix(k8s): fix multiple command arguments for exec

* chore: fix unit test

* chore: add integ test for exec command

* chore: fix test

* chore: keep optional command arg to avoid breaking change

* chore: add deprecation warning for positional command argument

core/src/commands/exec.ts

@@ -11,11 +11,11 @@ import type { CommandResult, CommandParams } from "./base.js"
 import { Command } from "./base.js"
 import dedent from "dedent"
 import type { ParameterValues } from "../cli/params.js"
-import { StringParameter, BooleanParameter, StringsParameter } from "../cli/params.js"
+import { StringParameter, BooleanParameter } from "../cli/params.js"
 import type { ExecInDeployResult } from "../plugin/handlers/Deploy/exec.js"
 import { execInDeployResultSchema } from "../plugin/handlers/Deploy/exec.js"
 import { executeAction } from "../graph/actions.js"
-import { NotFoundError } from "../exceptions.js"
+import { CommandError, NotFoundError } from "../exceptions.js"
 import type { DeployStatus } from "../plugin/handlers/Deploy/get-status.js"
 import { createActionLog } from "../logger/log-entry.js"
 import { K8_POD_DEFAULT_CONTAINER_ANNOTATION_KEY } from "../plugins/kubernetes/run.js"
@@ -29,10 +29,9 @@ const execArgs = {
       return Object.keys(configDump.actionConfigs.Deploy)
     },
   }),
-  command: new StringsParameter({
-    help: "The command to run.",
-    required: true,
-    spread: true,
+  command: new StringParameter({
+    help: "The use of the positional command argument is deprecated. Use  `--` followed by your command instead.",
+    required: false,
   }),
 }
 
@@ -62,12 +61,16 @@ export class ExecCommand extends Command<Args, Opts> {
   override description = dedent`
     Finds an active container for a deployed Deploy and executes the given command within the container.
     Supports interactive shells.
+    You can specify the command to run as a parameter, or pass it after a \`--\` separator. For commands
+    with arguments or quoted substrings, use the \`--\` separator.
 
-    _NOTE: This command may not be supported for all action types._
+    _NOTE: This command may not be supported for all action types. The use of the positional command argument
+    is deprecated. Use  \`--\` followed by your command instead._
 
     Examples:
 
-         garden exec my-service /bin/sh   # runs a shell in the my-service Deploy's container
+         garden exec my-service /bin/sh   # runs an interactive shell in the my-service Deploy's container
+         garden exec my-service -- /bin/sh -c echo "hello world" # prints "hello world" in the my-service Deploy's container and exits
   `
 
   override arguments = execArgs
@@ -88,6 +91,11 @@ export class ExecCommand extends Command<Args, Opts> {
   async action({ garden, log, args, opts }: CommandParams<Args, Opts>): Promise<CommandResult<ExecInDeployResult>> {
     const deployName = args.deploy
     const command = this.getCommand(args)
+
+    if (!command.length) {
+      throw new CommandError({ message: `No command specified. Nothing to execute.` })
+    }
+
     const target = opts["target"] as string | undefined
 
     const graph = await garden.getConfigGraph({ log, emit: false })
@@ -160,6 +168,6 @@ export class ExecCommand extends Command<Args, Opts> {
   }
 
   private getCommand(args: ParameterValues<Args>) {
-    return args.command || []
+    return args.command ? args.command.split(" ") : args["--"] || []
   }
 }

core/test/integ/src/plugins/kubernetes/commands/exec.ts

@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2018-2023 Garden Technologies, Inc. <[email protected]>
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+import { expect } from "chai"
+import { ExecCommand } from "../../../../../../src/commands/exec.js"
+import type { ConfigGraph } from "../../../../../../src/graph/config-graph.js"
+import type { ContainerDeployAction } from "../../../../../../src/plugins/container/config.js"
+import { DeployTask } from "../../../../../../src/tasks/deploy.js"
+import type { TestGarden } from "../../../../../helpers.js"
+import { expectError, withDefaultGlobalOpts } from "../../../../../helpers.js"
+import { getContainerTestGarden } from "../container/container.js"
+import { CommandError } from "../../../../../../src/exceptions.js"
+
+describe("runExecCommand", () => {
+  let garden: TestGarden
+  let cleanup: (() => void) | undefined
+  let graph: ConfigGraph
+
+  before(async () => {
+    ;({ garden, cleanup } = await getContainerTestGarden("local"))
+    const action = await resolveDeployAction("simple-service")
+    const deployTask = new DeployTask({
+      garden,
+      graph,
+      log: garden.log,
+      action,
+      force: true,
+      forceBuild: false,
+    })
+    await garden.processTasks({ tasks: [deployTask], log: garden.log, throwOnError: true })
+  })
+
+  async function resolveDeployAction(name: string) {
+    graph = await garden.getConfigGraph({ log: garden.log, emit: false, actionModes: { default: ["deploy." + name] } })
+    return garden.resolveAction<ContainerDeployAction>({ action: graph.getDeploy(name), log: garden.log, graph })
+  }
+
+  after(async () => {
+    if (cleanup) {
+      cleanup()
+    }
+  })
+
+  it("should exec a command in a running service using the -- separator", async () => {
+    const execCommand = new ExecCommand()
+    const args = { deploy: "simple-service", command: "" }
+    args["--"] = ["echo", "ok, lots of text"]
+
+    const { result, errors } = await execCommand.action({
+      garden,
+      log: garden.log,
+      args,
+      opts: withDefaultGlobalOpts({
+        interactive: false,
+        target: "",
+      }),
+    })
+
+    if (errors) {
+      throw errors[0]
+    }
+    expect(result?.output).to.equal("ok, lots of text")
+  })
+
+  it("should exec a command in a running service without the -- separator", async () => {
+    const execCommand = new ExecCommand()
+    const args = { deploy: "simple-service", command: "echo hello" }
+
+    const { result, errors } = await execCommand.action({
+      garden,
+      log: garden.log,
+      args,
+      opts: withDefaultGlobalOpts({
+        interactive: false,
+        target: "",
+      }),
+    })
+
+    if (errors) {
+      throw errors[0]
+    }
+    expect(result?.output).to.equal("hello")
+  })
+
+  it("should throw if no command was specified", async () => {
+    const execCommand = new ExecCommand()
+    const args = { deploy: "simple-service", command: "" }
+    await expectError(
+      () =>
+        execCommand.action({
+          garden,
+          log: garden.log,
+          args,
+          opts: withDefaultGlobalOpts({
+            interactive: false,
+            target: "",
+          }),
+        }),
+      (err) =>
+        expect(err).to.be.instanceOf(CommandError).with.property("message", "No command specified. Nothing to execute.")
+    )
+  })
+})

core/test/unit/src/cli/helpers.ts

@@ -298,7 +298,7 @@ describe("processCliArgs", () => {
 
   it("ignores cliOnly options when cli=false", () => {
     const cmd = new ExecCommand()
-    const { opts } = parseAndProcess(["my-service", "echo 'test'", "--interactive=true"], cmd, false)
+    const { opts } = parseAndProcess(["my-service", "--", "echo 'test'", "--interactive=true"], cmd, false)
     expect(opts.interactive).to.be.false
   })
 
@@ -317,13 +317,13 @@ describe("processCliArgs", () => {
 
   it("prefers defaultValue value over cliDefault when cli=false", () => {
     const cmd = new ExecCommand()
-    const { opts } = parseAndProcess(["my-service", "echo 'test'"], cmd, false)
+    const { opts } = parseAndProcess(["my-service", "--", "echo 'test'"], cmd, false)
     expect(opts.interactive).to.be.false
   })
 
   it("prefers cliDefault value over defaultValue when cli=true", () => {
     const cmd = new ExecCommand()
-    const { opts } = parseAndProcess(["my-service", "echo 'test'"], cmd, true)
+    const { opts } = parseAndProcess(["my-service", "--", "echo 'test'"], cmd, true)
     expect(opts.interactive).to.be.true
   })
 

core/test/unit/src/commands/exec.ts

@@ -17,7 +17,8 @@ describe("ExecCommand", () => {
     const garden = await makeTestGardenA()
     const log = garden.log
 
-    const args = { deploy: "service-a", command: ["echo", "ok"] }
+    const args = { deploy: "service-a", command: "" }
+    args["--"] = ["echo", "ok"]
 
     command.printHeader({ log, args })
 

docs/reference/commands.md

@@ -1346,23 +1346,27 @@ run:
 
 Finds an active container for a deployed Deploy and executes the given command within the container.
 Supports interactive shells.
+You can specify the command to run as a parameter, or pass it after a `--` separator. For commands
+with arguments or quoted substrings, use the `--` separator.
 
-_NOTE: This command may not be supported for all action types._
+_NOTE: This command may not be supported for all action types. The use of the positional command argument
+is deprecated. Use  `--` followed by your command instead._
 
 Examples:
 
-     garden exec my-service /bin/sh   # runs a shell in the my-service Deploy's container
+     garden exec my-service /bin/sh   # runs an interactive shell in the my-service Deploy's container
+     garden exec my-service -- /bin/sh -c echo "hello world" # prints "hello world" in the my-service Deploy's container and exits
 
 #### Usage
 
-    garden exec <deploy> <command> [options]
+    garden exec <deploy> [command] [options]
 
 #### Arguments
 
 | Argument | Required | Description |
 | -------- | -------- | ----------- |
   | `deploy` | Yes | The running Deploy action to exec the command in.
-  | `command` | Yes | The command to run.
+  | `command` | No | The use of the positional command argument is deprecated. Use  &#x60;--&#x60; followed by your command instead.
 
 #### Options
 

docs/using-garden/using-the-cli.md

@@ -72,15 +72,15 @@ garden deploy my-deploy --sync=*
 ### Executing a command in a running `Deploy` container
 
 ```sh
-garden exec my-deploy <command>
+garden exec my-deploy -- <command>
 ```
 
 ### Executing an interactive shell in a running `Deploy` container
 
 _Note: This assumes that `sh` is available in the container._
 
 ```sh
-garden exec my-deploy sh
+garden exec my-deploy -- sh
 ```
 
 ### Getting the status of your `Deploy`s