From 51e9a32db7dc9084cc601f51374eb734628439ac Mon Sep 17 00:00:00 2001
From: Simone Gotti <simone.gotti@gmail.com>
Date: Wed, 11 Sep 2019 15:17:38 +0200
Subject: [PATCH] runconfig: set task default shell

Currently, if no shell is defined in the task and in the step, the executor will
use an hardcoded default shell.

This will cause changed run behavior if we add an option to globally set the
agola default shell.

To avoid this set the task shell to the default shell inside the runconfig if
it's empty so future executions will always use this value.
---
 internal/runconfig/runconfig.go        | 8 ++++++++
 internal/runconfig/runconfig_test.go   | 3 +++
 internal/services/executor/executor.go | 2 ++
 3 files changed, 13 insertions(+)

diff --git a/internal/runconfig/runconfig.go b/internal/runconfig/runconfig.go
index a695f94..b7cf301 100644
--- a/internal/runconfig/runconfig.go
+++ b/internal/runconfig/runconfig.go
@@ -26,6 +26,10 @@ import (
 	errors "golang.org/x/xerrors"
 )
 
+const (
+	defaultShell = "/bin/sh -e"
+)
+
 func genRuntime(c *config.Config, ce *config.Runtime, variables map[string]string) *rstypes.Runtime {
 	containers := []*rstypes.Container{}
 	for _, cc := range ce.Containers {
@@ -206,6 +210,10 @@ func GenRunConfigTasks(uuid util.UUIDGenerator, c *config.Config, runName string
 			DockerRegistriesAuth: make(map[string]rstypes.DockerRegistryAuth),
 		}
 
+		if t.Shell == "" {
+			t.Shell = defaultShell
+		}
+
 		if c.DockerRegistriesAuth != nil {
 			for regname, auth := range c.DockerRegistriesAuth {
 				t.DockerRegistriesAuth[regname] = rstypes.DockerRegistryAuth{
diff --git a/internal/runconfig/runconfig_test.go b/internal/runconfig/runconfig_test.go
index 92f7bd4..39ee913 100644
--- a/internal/runconfig/runconfig_test.go
+++ b/internal/runconfig/runconfig_test.go
@@ -801,6 +801,7 @@ func TestGenRunConfig(t *testing.T) {
 							},
 						},
 					},
+					Shell: "/bin/sh -e",
 					Environment: map[string]string{
 						"ENV01":             "ENV01",
 						"ENVFROMVARIABLE01": "VARVALUE01",
@@ -876,6 +877,7 @@ func TestGenRunConfig(t *testing.T) {
 							},
 						},
 					},
+					Shell:       "/bin/sh -e",
 					Environment: map[string]string{},
 					Steps: rstypes.Steps{
 						&rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}},
@@ -973,6 +975,7 @@ func TestGenRunConfig(t *testing.T) {
 							},
 						},
 					},
+					Shell:       "/bin/sh -e",
 					Environment: map[string]string{},
 					Steps: rstypes.Steps{
 						&rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}},
diff --git a/internal/services/executor/executor.go b/internal/services/executor/executor.go
index c60d2a7..be400e3 100644
--- a/internal/services/executor/executor.go
+++ b/internal/services/executor/executor.go
@@ -109,6 +109,8 @@ func (e *Executor) doRunStep(ctx context.Context, s *types.RunStep, t *types.Exe
 	}
 	defer outf.Close()
 
+	// TODO(sgotti) this line is used only for old runconfig versions that don't
+	// set a task default shell in the runconfig
 	shell := defaultShell
 	if t.Shell != "" {
 		shell = t.Shell