add dbl-quotes to echo command

nit: typo in assert message

add a couple of tests

add playbook-level `ssh_shell`

fix empty shell in command if not set in env

default local shell to /bin/sh if no SHELL var set

add debug info for runner creation

pin go version to 1.20.5 due to testcontainers/testcontainers-go#1359

add support of --shell parameter setting the default ssh shell for the remote execution

README.md

@@ -134,14 +134,15 @@ Spot supports the following command-line options:
 - `-c`, `--concurrent=`: Sets the number of concurrent hosts to execute tasks. Defaults to `1`, which means hosts will be handled  sequentially.
 - `--timeout`: Sets the SSH timeout. Defaults to `30s`. User can also set the environment variable `$SPOT_TIMEOUT` to define the SSH timeout.
 - `--ssh-agent`: Enables the use of the SSH agent for authentication. Defaults to `false`. User can also set the environment variable `SPOT_SSH_AGENT` to define the value.
+- `--shell` - shell to use for remote ssh execution, default is `/bin/sh`. User can also set the environment variable `SPOT_SHELL` to define the value.  
 - `-i`, `--inventory=`: Specifies the inventory file or url to use for the task execution. Overrides the inventory file defined in the
   playbook file. User can also set the environment variable `$SPOT_INVENTORY` to define the default inventory file path or url.
 - `-u`, `--user=`: Specifies the SSH user to use when connecting to remote hosts. Overrides the user defined in the playbook file .
 - `-k`, `--key=`: Specifies the SSH key to use when connecting to remote hosts. Overrides the key defined in the playbook file.
 - `-s`, `--skip=`: Skips the specified commands during the task execution. Providing the `-s` flag multiple times with different command names skips multiple commands.
 - `-o`, `--only=`: Runs only the specified commands during the task execution. Providing the `-o` flag multiple times with different command names runs only multiple commands.
 - `-e`, `--env=`: Sets the environment variables to be used during the task execution. Providing the `-e` flag multiple times with different environment variables sets multiple environment variables, e.g., `-e VAR1:VALUE1 -e VAR2:VALUE2`.
-- `-E`, `--env-file=`: Sets the environment variables from file to be used during the task execution. Default is env.yml`
+- `-E`, `--env-file=`: Sets the environment variables from file to be used during the task execution. Default is env.yml. Can be also set with environment variable `SPOT_ENV_FILE`.
 - `--dry`: Enables dry-run mode, which prints out the commands to be executed without actually executing them.
 - `-v`, `--verbose`: Enables verbose mode, providing more detailed output and error messages during the task execution.
 - `--dbg`: Enables debug mode, providing even more detailed output and error messages during the task execution as well as diagnostic messages.
@@ -166,6 +167,7 @@ Spot supports the following command-line options:
 ```yaml
 user: umputun                       # default ssh user. Can be overridden by -u flag or by inventory or host definition
 ssh_key: keys/id_rsa                # ssh key
+ssh_shell: /bin/bash                # shell to use for remote ssh execution, default is /bin/sh
 inventory: /etc/spot/inventory.yml  # default inventory file. Can be overridden by --inventory flag
 
 # list of targets, i.e. hosts, inventory files or inventory URLs
@@ -235,6 +237,7 @@ In some cases the rich syntax of the full playbook is not needed and can felt ov
 ```yaml
 user: umputun                       # default ssh user. Can be overridden by -u flag or by inventory or host definition
 ssh_key: keys/id_rsa                # ssh key
+ssh_shell: /bin/bash                # shell to use for remote ssh execution, default is /bin/sh
 inventory: /etc/spot/inventory.yml  # default inventory file. Can be overridden by --inventory flag
 
 targets: ["devbox1", "devbox2", "h1.example.com:2222", "h2.example.com"] # list of host names from inventory and direct host ips

cmd/spot/main.go

@@ -37,13 +37,14 @@ type options struct {
 	Concurrent   int           `short:"c" long:"concurrent" description:"concurrent tasks" default:"1"`
 	SSHTimeout   time.Duration `long:"timeout" env:"SPOT_TIMEOUT" description:"ssh timeout" default:"30s"`
 	SSHAgent     bool          `long:"ssh-agent" env:"SPOT_SSH_AGENT" description:"use ssh-agent"`
+	SSHShell     string        `long:"shell" env:"SPOT_SHELL" description:"shell to use for ssh" default:"/bin/sh"`
 
 	// overrides
 	Inventory string            `short:"i" long:"inventory" description:"inventory file or url [$SPOT_INVENTORY]"`
 	SSHUser   string            `short:"u" long:"user" description:"ssh user"`
 	SSHKey    string            `short:"k" long:"key" description:"ssh key"`
 	Env       map[string]string `short:"e" long:"env" description:"environment variables for all commands"`
-	EnvFile   string            `short:"E" long:"env-file" description:"environment variables from file" default:"env.yml"`
+	EnvFile   string            `short:"E" long:"env-file" env:"SPOT_ENV_FILE" description:"environment variables from file" default:"env.yml"`
 
 	// commands filter
 	Skip []string `long:"skip" description:"skip commands"`
@@ -287,6 +288,7 @@ func makePlaybook(opts options, inventory string) (*config.PlayBook, error) {
 		Environment:  env,
 		User:         opts.SSHUser,
 		AdHocCommand: opts.PositionalArgs.AdHocCmd,
+		SSHShell:     opts.SSHShell,
 	}
 
 	exPlaybookFile, err := expandPath(opts.PlaybookFile)
@@ -334,7 +336,11 @@ func makeRunner(opts options, pbook *config.PlayBook) (*runner.Process, error) {
 		ColorWriter: executor.NewColorizedWriter(os.Stdout, "", "", "", nil),
 		Verbose:     opts.Verbose,
 		Dry:         opts.Dry,
+		SSHShell:    opts.SSHShell,
 	}
+	log.Printf("[DEBUG] runner created: concurrency:%d, connector: %s, ssh_shell:%q, verbose:%v, dry:%v, only:%v, skip:%v",
+		r.Concurrency, r.Connector, r.SSHShell, r.Verbose, r.Dry, r.Only, r.Skip)
+
 	return &r, nil
 }
 

cmd/spot/main_test.go

@@ -26,9 +26,21 @@ func Test_main(t *testing.T) {
 	hostAndPort, teardown := startTestContainer(t)
 	defer teardown()
 
-	args := []string{"simplotask", "--dbg", "--playbook=testdata/conf-local.yml", "--user=test", "--key=testdata/test_ssh_key", "--target=" + hostAndPort}
-	os.Args = args
-	main()
+	t.Run("with system shell set", func(t *testing.T) {
+		args := []string{"simplotask", "--dbg", "--playbook=testdata/conf-local.yml", "--user=test",
+			"--key=testdata/test_ssh_key", "--target=" + hostAndPort}
+		os.Args = args
+		main()
+	})
+
+	t.Run("with system shell not set", func(t *testing.T) {
+		args := []string{"simplotask", "--dbg", "--playbook=testdata/conf-local.yml", "--user=test",
+			"--key=testdata/test_ssh_key", "--target=" + hostAndPort}
+		os.Args = args
+		err := os.Setenv("SHELL", "")
+		require.NoError(t, err)
+		main()
+	})
 }
 
 func Test_runCompleted(t *testing.T) {

pkg/config/command.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"os"
 	"reflect"
 	"sort"
 	"strings"
@@ -29,7 +30,8 @@ type Cmd struct {
 	Options     CmdOptions        `yaml:"options" toml:"options,omitempty"`
 	Condition   string            `yaml:"cond" toml:"cond,omitempty"`
 
-	Secrets map[string]string `yaml:"-" toml:"-"` // loaded secrets, filled by playbook
+	Secrets  map[string]string `yaml:"-" toml:"-"` // loaded secrets, filled by playbook
+	SSHShell string            `yaml:"-" toml:"-"` // shell to use for ssh commands, filled by playbook
 }
 
 // CmdOptions defines options for a command
@@ -136,7 +138,7 @@ func (cmd *Cmd) scriptCommand(inp string) string {
 
 	// add environment variables
 	envs := cmd.genEnv()
-	res := "sh -c '"
+	res := cmd.shell() + " -c '"
 	if len(envs) > 0 {
 		res += strings.Join(envs, "; ") + "; "
 	}
@@ -169,8 +171,8 @@ func (cmd *Cmd) scriptFile(inp string) (r io.Reader) {
 	var buf bytes.Buffer
 
 	if !cmd.hasShebang(inp) {
-		buf.WriteString("#!/bin/sh\n") // add default shebang if not present
-		buf.WriteString("set -e\n")    // add 'set -e' to make the script exit on error
+		buf.WriteString("#!" + cmd.shell() + "\n") // add default shebang if not present
+		buf.WriteString("set -e\n")                // add 'set -e' to make the script exit on error
 	}
 
 	envs := cmd.genEnv()
@@ -390,3 +392,18 @@ func (cmd *Cmd) validate() error {
 	}
 	return nil
 }
+
+func (cmd *Cmd) shell() string {
+	if cmd.SSHShell == "" {
+		return "/bin/sh"
+	}
+	if cmd.Options.Local {
+		envShell := os.Getenv("SHELL")
+		if envShell == "" {
+			return "/bin/sh"
+		}
+		return envShell
+	}
+
+	return cmd.SSHShell
+}

pkg/config/command_test.go

@@ -23,7 +23,7 @@ func TestCmd_GetScript(t *testing.T) {
 			cmd: &Cmd{
 				Script: "echo Hello, World!",
 			},
-			expectedScript:   `sh -c 'echo Hello, World!'`,
+			expectedScript:   `/bin/sh -c 'echo Hello, World!'`,
 			expectedContents: nil,
 		},
 		{
@@ -93,7 +93,7 @@ export FOO='bar'
 					"GREETING": "Hello, World!",
 				},
 			},
-			expectedScript:   `sh -c 'GREETING="Hello, World!"; echo $GREETING'`,
+			expectedScript:   `/bin/sh -c 'GREETING="Hello, World!"; echo $GREETING'`,
 			expectedContents: nil,
 		},
 		{
@@ -181,7 +181,7 @@ func TestCmd_getScriptCommand(t *testing.T) {
 		cmd := c.Tasks[0].Commands[3]
 		assert.Equal(t, "git", cmd.Name, "name")
 		res := cmd.scriptCommand(cmd.Script)
-		assert.Equal(t, `sh -c 'git clone https://example.com/remark42.git /srv || true; cd /srv; git pull'`, res)
+		assert.Equal(t, `/bin/sh -c 'git clone https://example.com/remark42.git /srv || true; cd /srv; git pull'`, res)
 	})
 
 	t.Run("no-script", func(t *testing.T) {
@@ -195,7 +195,35 @@ func TestCmd_getScriptCommand(t *testing.T) {
 		cmd := c.Tasks[0].Commands[4]
 		assert.Equal(t, "docker", cmd.Name)
 		res := cmd.scriptCommand(cmd.Script)
-		assert.Equal(t, `sh -c 'BAR="qux"; FOO="bar"; docker pull umputun/remark42:latest; docker stop remark42 || true; docker rm remark42 || true; docker run -d --name remark42 -p 8080:8080 umputun/remark42:latest'`, res)
+		assert.Equal(t, `/bin/sh -c 'BAR="qux"; FOO="bar"; docker pull umputun/remark42:latest; docker stop remark42 || true; docker rm remark42 || true; docker run -d --name remark42 -p 8080:8080 umputun/remark42:latest'`, res)
+	})
+}
+
+func TestCmd_getScriptCommandCustomShell(t *testing.T) {
+	c, err := New("testdata/f1.yml", &Overrides{SSHShell: "/bin/bash"}, nil)
+	require.NoError(t, err)
+	t.Logf("%+v", c)
+	assert.Equal(t, 1, len(c.Tasks), "single task")
+
+	t.Run("script", func(t *testing.T) {
+		cmd := c.Tasks[0].Commands[3]
+		assert.Equal(t, "git", cmd.Name, "name")
+		res := cmd.scriptCommand(cmd.Script)
+		assert.Equal(t, `/bin/bash -c 'git clone https://example.com/remark42.git /srv || true; cd /srv; git pull'`, res)
+	})
+
+	t.Run("no-script", func(t *testing.T) {
+		cmd := c.Tasks[0].Commands[1]
+		assert.Equal(t, "copy configuration", cmd.Name)
+		res := cmd.scriptCommand(cmd.Script)
+		assert.Equal(t, "", res)
+	})
+
+	t.Run("script with env", func(t *testing.T) {
+		cmd := c.Tasks[0].Commands[4]
+		assert.Equal(t, "docker", cmd.Name)
+		res := cmd.scriptCommand(cmd.Script)
+		assert.Equal(t, `/bin/bash -c 'BAR="qux"; FOO="bar"; docker pull umputun/remark42:latest; docker stop remark42 || true; docker rm remark42 || true; docker run -d --name remark42 -p 8080:8080 umputun/remark42:latest'`, res)
 	})
 }
 
@@ -219,6 +247,14 @@ func TestCmd_getScriptFile(t *testing.T) {
 			},
 			expected: "#!/bin/bash\nset -e\necho 'Hello, World!'\n",
 		},
+		{
+			name: "with non-default shell",
+			cmd: &Cmd{
+				Script:   "echo 'Hello, World!'",
+				SSHShell: "/bin/zsh",
+			},
+			expected: "#!/bin/zsh\nset -e\necho 'Hello, World!'\n",
+		},
 		{
 			name: "with one environment variable",
 			cmd: &Cmd{
@@ -501,7 +537,7 @@ func TestCmd_GetWait(t *testing.T) {
 					Command: "echo Hello, World!",
 				},
 			},
-			expectedCmd: `sh -c 'echo Hello, World!'`,
+			expectedCmd: `/bin/sh -c 'echo Hello, World!'`,
 		},
 		{
 			name: "multi-line wait command",
@@ -551,13 +587,13 @@ func TestCmd_GetCondition(t *testing.T) {
 		{
 			name:           "single-line wait command",
 			cmd:            &Cmd{Condition: "echo Hello, World!"},
-			expectedCmd:    `sh -c 'echo Hello, World!'`,
+			expectedCmd:    `/bin/sh -c 'echo Hello, World!'`,
 			expectedInvert: false,
 		},
 		{
 			name:           "single-line wait command inverted",
 			cmd:            &Cmd{Condition: "! echo Hello, World!"},
-			expectedCmd:    `sh -c 'echo Hello, World!'`,
+			expectedCmd:    `/bin/sh -c 'echo Hello, World!'`,
 			expectedInvert: true,
 		},
 		{

pkg/config/playbook.go

@@ -26,6 +26,7 @@ import (
 type PlayBook struct {
 	User      string            `yaml:"user" toml:"user"`           // ssh user
 	SSHKey    string            `yaml:"ssh_key" toml:"ssh_key"`     // ssh key
+	SSHShell  string            `yaml:"ssh_shell" toml:"ssh_shell"` // ssh shell to use
 	Inventory string            `yaml:"inventory" toml:"inventory"` // inventory file or url
 	Targets   map[string]Target `yaml:"targets" toml:"targets"`     // list of targets/environments
 	Tasks     []Task            `yaml:"tasks" toml:"tasks"`         // list of tasks
@@ -85,6 +86,7 @@ type Overrides struct {
 	Inventory    string
 	Environment  map[string]string
 	AdHocCommand string
+	SSHShell     string
 }
 
 // InventoryData defines inventory data format
@@ -150,8 +152,9 @@ func New(fname string, overrides *Overrides, secProvider SecretsProvider) (res *
 
 	// log loaded config info
 	log.Printf("[INFO] playbook loaded with %d tasks", len(res.Tasks))
-	for _, tsk := range res.Tasks {
-		for _, c := range tsk.Commands {
+	for i, tsk := range res.Tasks {
+		for j, c := range tsk.Commands {
+			res.Tasks[i].Commands[j].SSHShell = res.shell() // set secrets for all commands in task
 			log.Printf("[DEBUG] load command %q (task: %s)", c.Name, tsk.Name)
 		}
 	}
@@ -267,7 +270,7 @@ func unmarshalPlaybookFile(fname string, data []byte, overrides *Overrides, res
 		}
 		res.Targets = map[string]Target{"default": target}
 		return nil
-	} else { //nolint
+	} else { // nolint
 		errs = multierror.Append(errs, err)
 	}
 
@@ -296,7 +299,8 @@ func (p *PlayBook) Task(name string) (*Task, error) {
 	searchTask := func(tsk []Task, name string) (*Task, error) {
 		if name == "ad-hoc" && p.overrides.AdHocCommand != "" {
 			// special case for ad-hoc command, make a fake task with a single command from overrides.AdHocCommand
-			return &Task{Name: "ad-hoc", Commands: []Cmd{{Name: "ad-hoc", Script: p.overrides.AdHocCommand}}}, nil
+			return &Task{Name: "ad-hoc", Commands: []Cmd{
+				{Name: "ad-hoc", Script: p.overrides.AdHocCommand, SSHShell: p.shell()}}}, nil
 		}
 		for _, t := range tsk {
 			if strings.EqualFold(t.Name, name) {
@@ -585,3 +589,13 @@ func (p *PlayBook) loadSecrets() error {
 	}
 	return nil
 }
+
+func (p *PlayBook) shell() string {
+	if p.overrides != nil && p.overrides.SSHShell != "" {
+		return p.overrides.SSHShell
+	}
+	if p.SSHShell != "" {
+		return p.SSHShell
+	}
+	return "/bin/sh"
+}

pkg/config/playbook_test.go

@@ -22,6 +22,7 @@ func TestPlaybook_New(t *testing.T) {
 		t.Logf("%+v", c)
 		assert.Equal(t, 1, len(c.Tasks), "single task")
 		assert.Equal(t, "umputun", c.User, "user")
+		assert.Equal(t, "/bin/sh", c.Tasks[0].Commands[0].SSHShell, "ssh shell")
 
 		tsk := c.Tasks[0]
 		assert.Equal(t, 5, len(tsk.Commands), "5 commands")
@@ -182,12 +183,38 @@ func TestPlaybook_New(t *testing.T) {
 		assert.Equal(t, 5, len(tsk.Commands))
 		assert.Equal(t, "docker", tsk.Commands[4].Name)
 		assert.Equal(t, map[string]string{"SEC1": "VAL1", "SEC2": "VAL2"}, tsk.Commands[4].Secrets)
+		assert.Equal(t, []string{"VAL1", "VAL2"}, p.AllSecretValues())
 	})
 
 	t.Run("playbook prohibited all target", func(t *testing.T) {
 		_, err := New("testdata/playbook-with-all-group.yml", nil, nil)
 		require.ErrorContains(t, err, "config testdata/playbook-with-all-group.yml is invalid: target \"all\" is reserved for all hosts")
 	})
+
+	t.Run("playbook with custom ssh shell set", func(t *testing.T) {
+		c, err := New("testdata/with-ssh-shell.yml", nil, nil)
+		require.NoError(t, err)
+		t.Logf("%+v", c)
+		assert.Equal(t, 1, len(c.Tasks), "single task")
+		assert.Equal(t, "umputun", c.User, "user")
+		assert.Equal(t, "/bin/bash", c.Tasks[0].Commands[0].SSHShell, "ssh shell")
+
+		tsk := c.Tasks[0]
+		assert.Equal(t, 5, len(tsk.Commands), "5 commands")
+		assert.Equal(t, "deploy-remark42", tsk.Name, "task name")
+	})
+	t.Run("playbook with custom ssh shell override", func(t *testing.T) {
+		c, err := New("testdata/with-ssh-shell.yml", &Overrides{SSHShell: "/bin/zsh"}, nil)
+		require.NoError(t, err)
+		t.Logf("%+v", c)
+		assert.Equal(t, 1, len(c.Tasks), "single task")
+		assert.Equal(t, "umputun", c.User, "user")
+		assert.Equal(t, "/bin/zsh", c.Tasks[0].Commands[0].SSHShell, "ssh shell")
+
+		tsk := c.Tasks[0]
+		assert.Equal(t, 5, len(tsk.Commands), "5 commands")
+		assert.Equal(t, "deploy-remark42", tsk.Name, "task name")
+	})
 }
 
 func TestPlayBook_Task(t *testing.T) {
@@ -216,6 +243,18 @@ func TestPlayBook_Task(t *testing.T) {
 		assert.Equal(t, 1, len(tsk.Commands))
 		assert.Equal(t, "ad-hoc", tsk.Name)
 		assert.Equal(t, "echo 123", tsk.Commands[0].Script)
+		assert.Equal(t, "/bin/sh", tsk.Commands[0].SSHShell)
+	})
+
+	t.Run("adhoc with custom shell", func(t *testing.T) {
+		c, err := New("", &Overrides{AdHocCommand: "echo 123", User: "umputun", SSHShell: "/bin/zsh"}, nil)
+		require.NoError(t, err)
+		tsk, err := c.Task("ad-hoc")
+		require.NoError(t, err)
+		assert.Equal(t, 1, len(tsk.Commands))
+		assert.Equal(t, "ad-hoc", tsk.Name)
+		assert.Equal(t, "echo 123", tsk.Commands[0].Script)
+		assert.Equal(t, "/bin/zsh", tsk.Commands[0].SSHShell)
 	})
 }
 
@@ -480,6 +519,8 @@ hosts:
 	// create test HTTP server
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch filepath.Ext(r.URL.Path) {
+		case ".bad":
+			w.WriteHeader(http.StatusInternalServerError)
 		case ".toml":
 			http.ServeFile(w, r, tomlFile.Name())
 		default:
@@ -499,6 +540,7 @@ hosts:
 		{"load YAML from URL without extension", ts.URL + "/inventory", false},
 		{"load TOML from file", tomlFile.Name(), false},
 		{"load TOML from URL", ts.URL + "/inventory.toml", false},
+		{"load from URL with bad status", ts.URL + "/blah.bad", true},
 		{"invalid URL", "http://not-a-valid-url", true},
 		{"file not found", "nonexistent-file.yaml", true},
 	}