fix: app cmd parsing, usage & tests

Note: the integration tests don't work due to ValidateApp still
attempting to validate the host key for the test app which doesn't
exist. This will be fixed in a future commit.

cli/app/cmd.go

@@ -25,21 +25,6 @@ import (
 	"github.com/urfave/cli"
 )
 
-var localCmd bool
-var localCmdFlag = &cli.BoolFlag{
-	Name:        "local, l",
-	Usage:       "Run command locally",
-	Destination: &localCmd,
-}
-
-var remoteUser string
-var remoteUserFlag = &cli.StringFlag{
-	Name:        "user, u",
-	Value:       "",
-	Usage:       "User to run command within a service context",
-	Destination: &remoteUser,
-}
-
 var appCmdCommand = cli.Command{
 	Name:    "command",
 	Aliases: []string{"cmd"},
@@ -56,21 +41,23 @@ Example:
 
   abra app cmd example.com app create_user -- me@example.com
 `,
-	ArgsUsage: "<domain> [<service>] <command>",
+	ArgsUsage: "<domain> [<service>] <command> [-- <args>]",
 	Flags: []cli.Flag{
 		internal.DebugFlag,
-		localCmdFlag,
-		remoteUserFlag,
+		internal.LocalCmdFlag,
+		internal.RemoteUserFlag,
 	},
 	BashComplete: autocomplete.AppNameComplete,
 	Before:       internal.SubCommandBefore,
 	Action: func(c *cli.Context) error {
 		app := internal.ValidateApp(c)
 
-		if localCmd && remoteUser != "" {
-			internal.ShowSubcommandHelpAndError(c, errors.New("cannot use --local & <user> together"))
+		if internal.LocalCmd && internal.RemoteUser != "" {
+			internal.ShowSubcommandHelpAndError(c, errors.New("cannot use --local & --user together"))
 		}
 
+		hasCmdArgs, parsedCmdArgs := parseCmdArgs(c.Args(), internal.LocalCmd)
+
 		abraSh := path.Join(config.RECIPES_DIR, app.Recipe, "abra.sh")
 		if _, err := os.Stat(abraSh); err != nil {
 			if os.IsNotExist(err) {
@@ -79,21 +66,7 @@ Example:
 			logrus.Fatal(err)
 		}
 
-		var parsedCmdArgs string
-		var cmdArgsIdx int
-		var hasCmdArgs bool
-		for idx, arg := range c.Args() {
-			if arg == "--" {
-				cmdArgsIdx = idx
-				hasCmdArgs = true
-			}
-
-			if hasCmdArgs && idx > cmdArgsIdx {
-				parsedCmdArgs += fmt.Sprintf("%s ", c.Args().Get(idx))
-			}
-		}
-
-		if localCmd {
+		if internal.LocalCmd {
 			cmdName := c.Args().Get(1)
 			if err := ensureCommand(abraSh, app.Recipe, cmdName); err != nil {
 				logrus.Fatal(err)
@@ -156,6 +129,25 @@ Example:
 	},
 }
 
+func parseCmdArgs(args []string, isLocal bool) (bool, string) {
+	var (
+		parsedCmdArgs string
+		hasCmdArgs    bool
+	)
+
+	if isLocal {
+		if len(args) > 2 {
+			return true, fmt.Sprintf("%s ", strings.Join(args[2:], " "))
+		}
+	} else {
+		if len(args) > 3 {
+			return true, fmt.Sprintf("%s ", strings.Join(args[3:], " "))
+		}
+	}
+
+	return hasCmdArgs, parsedCmdArgs
+}
+
 func ensureCommand(abraSh, recipeName, execCmd string) error {
 	bytes, err := ioutil.ReadFile(abraSh)
 	if err != nil {
@@ -214,9 +206,9 @@ func runCmdRemote(app config.App, abraSh, serviceName, cmdName, cmdArgs string)
 		Tty:          true,
 	}
 
-	if remoteUser != "" {
-		logrus.Debugf("running command with user %s", remoteUser)
-		execCreateOpts.User = remoteUser
+	if internal.RemoteUser != "" {
+		logrus.Debugf("running command with user %s", internal.RemoteUser)
+		execCreateOpts.User = internal.RemoteUser
 	}
 
 	// FIXME: avoid instantiating a new CLI

cli/app/cmd_test.go

@@ -0,0 +1,31 @@
+package app
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestParseCmdArgs(t *testing.T) {
+	tests := []struct {
+		input          []string
+		shouldParse    bool
+		expectedOutput string
+	}{
+		// `--` is not parsed when passed in from the command-line e.g. -- foo bar baz
+		// so we need to eumlate that as missing when testing if bash args are passed in
+		// see https://git.coopcloud.tech/coop-cloud/organising/issues/336 for more
+		{[]string{"foo.com", "app", "test"}, false, ""},
+		{[]string{"foo.com", "app", "test", "foo"}, true, "foo "},
+		{[]string{"foo.com", "app", "test", "foo", "bar", "baz"}, true, "foo bar baz "},
+	}
+
+	for _, test := range tests {
+		ok, parsed := parseCmdArgs(test.input, false)
+		if ok != test.shouldParse {
+			t.Fatalf("[%s] should not parse", strings.Join(test.input, " "))
+		}
+		if parsed != test.expectedOutput {
+			t.Fatalf("%s does not match %s", parsed, test.expectedOutput)
+		}
+	}
+}

cli/internal/cli.go

@@ -353,6 +353,21 @@ var AllTagsFlag = &cli.BoolFlag{
 	Destination: &AllTags,
 }
 
+var LocalCmd bool
+var LocalCmdFlag = &cli.BoolFlag{
+	Name:        "local, l",
+	Usage:       "Run command locally",
+	Destination: &LocalCmd,
+}
+
+var RemoteUser string
+var RemoteUserFlag = &cli.StringFlag{
+	Name:        "user, u",
+	Value:       "",
+	Usage:       "User to run command within a service context",
+	Destination: &RemoteUser,
+}
+
 // SSHFailMsg is a hopefully helpful SSH failure message
 var SSHFailMsg = `
 Woops, Abra is unable to connect to connect to %s.

go.mod

@@ -46,7 +46,7 @@ require (
 	github.com/sergi/go-diff v1.2.0 // indirect
 	github.com/spf13/cobra v1.3.0 // indirect
 	github.com/theupdateframework/notary v0.7.0 // indirect
-	github.com/urfave/cli v1.22.5
+	github.com/urfave/cli v1.22.9
 	github.com/xanzy/ssh-agent v0.3.1 // indirect
 	github.com/xeipuuv/gojsonpointer v0.0.0-20190809123943-df4f5c81cb3b // indirect
 	golang.org/x/crypto v0.0.0-20220131195533-30dcbda58838

go.sum

@@ -982,8 +982,8 @@ github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijb
 github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/urfave/cli v1.22.4/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
-github.com/urfave/cli v1.22.5 h1:lNq9sAHXK2qfdI8W+GRItjCEkI+2oR4d+MEHy1CKXoU=
-github.com/urfave/cli v1.22.5/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
+github.com/urfave/cli v1.22.9 h1:cv3/KhXGBGjEXLC4bH0sLuJ9BewaAbpk5oyMOveu4pw=
+github.com/urfave/cli v1.22.9/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/vbatts/tar-split v0.11.2/go.mod h1:vV3ZuO2yWSVsz+pfFzDG/upWH1JhjOiEaWq6kXyQ3VI=
 github.com/vishvananda/netlink v0.0.0-20181108222139-023a6dafdcdf/go.mod h1:+SR5DhBJrl6ZM7CoCKvpw5BKroDKQ+PJqOg65H/2ktk=
 github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYppBueQtXaqoE=

pkg/test/test.go

@@ -0,0 +1,38 @@
+package test
+
+import (
+	"log"
+	"os"
+
+	"github.com/sirupsen/logrus"
+)
+
+// RmServerAppRecipe deletes the test server / app / recipe.
+func RmServerAppRecipe() {
+	testAppLink := os.ExpandEnv("$HOME/.abra/servers/foo.com")
+	if err := os.Remove(testAppLink); err != nil {
+		logrus.Fatal(err)
+	}
+
+	testRecipeLink := os.ExpandEnv("$HOME/.abra/recipes/test")
+	if err := os.Remove(testRecipeLink); err != nil {
+		logrus.Fatal(err)
+	}
+}
+
+// MkServerAppRecipe symlinks the test server / app / recipe.
+func MkServerAppRecipe() {
+	RmServerAppRecipe()
+
+	testAppDir := os.ExpandEnv("$PWD/../../tests/resources/testapp")
+	testAppLink := os.ExpandEnv("$HOME/.abra/servers/foo.com")
+	if err := os.Symlink(testAppDir, testAppLink); err != nil {
+		log.Fatal(err)
+	}
+
+	testRecipeDir := os.ExpandEnv("$PWD/../../tests/resources/testrecipe")
+	testRecipeLink := os.ExpandEnv("$HOME/.abra/recipes/test")
+	if err := os.Symlink(testRecipeDir, testRecipeLink); err != nil {
+		log.Fatal(err)
+	}
+}

tests/integration/.gitignore

@@ -0,0 +1 @@
+logs

tests/integration/cmd.sh

@@ -0,0 +1,14 @@
+#!/bin/bash
+
+source ./testfunctions.sh
+source ./common.sh
+
+create_server_app_recipe
+
+run_test '$ABRA app cmd foo.com test --local'
+
+run_test '$ABRA app cmd foo.com test --local -- foo'
+
+run_test '$ABRA app cmd foo.com test --local -- foo bar baz'
+
+clean_server_app_recipe

tests/integration/common.sh

@@ -2,6 +2,16 @@
 
 set -e
 
+create_server_app_recipe() {
+  ln -srf ../resources/testapp ~/.abra/servers/foo.com
+  ln -srf ../resources/testrecipe ~/.abra/recipes
+}
+
+clean_server_app_recipe() {
+  unlink ~/.abra/servers/foo.com
+  unlink ~/.abra/recipes/testrecipe
+}
+
 function init() {
   ABRA="$(pwd)/../../abra"
   INSTALLER_URL="https://git.coopcloud.tech/coop-cloud/abra/raw/branch/main/scripts/installer/installer"

tests/integration/test_all.sh

@@ -16,7 +16,7 @@ run_test () {
     echo $logfile
 }
 
-testScripts=("app.sh" "autocomplete.sh" "catalogue.sh" "install.sh" "recipe.sh" "records.sh" "server.sh")
+testScripts=("app.sh" "autocomplete.sh" "catalogue.sh" "install.sh" "recipe.sh" "records.sh" "server.sh", "cmd.sh")
 
 for i in "${testScripts[@]}"; do
     cmd="./$i $res_dir${i/sh/log}"

tests/resources/testapp/foo.com.env

@@ -0,0 +1 @@
+TYPE=test

tests/resources/testapp/testapp

@@ -0,0 +1 @@
+.

tests/resources/testrecipe/abra.sh

@@ -0,0 +1,5 @@
+test(){
+  echo "1: $1"
+  echo "2: $2"
+  echo "all: $@"
+}

tests/resources/testrecipe/compose.yml

@@ -0,0 +1,5 @@
+---
+version: "3.8"
+
+services:
+  app: []