From 0af3f8ba6a66e6551bdb48c5d85c23e0ab95b687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Perpignane?= Date: Thu, 12 Sep 2024 23:36:46 +0200 Subject: [PATCH] bug/issue #2448 - manage special bash options when no shell is defined * bash without "-o pipefail" option when "bash" is not explicitely defined in the workflow * bonus: fix inverted expected and actual in TestGetGitHubContext assertions --- pkg/model/workflow.go | 12 ++++++++++-- pkg/model/workflow_test.go | 11 +++++++---- pkg/runner/run_context_test.go | 18 +++++++++--------- pkg/runner/step_run.go | 14 +++++++++----- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index e66bf4ce949..57e8f57dc61 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -572,6 +572,8 @@ type Step struct { Uses string `yaml:"uses"` Run string `yaml:"run"` WorkingDirectory string `yaml:"working-directory"` + // WorkflowShell is the shell really configured in the job, directly at step level or higher in defaults.run.shell + WorkflowShell string `yaml:"-"` Shell string `yaml:"shell"` Env yaml.Node `yaml:"env"` With map[string]string `yaml:"with"` @@ -614,8 +616,14 @@ func (s *Step) ShellCommand() string { //Reference: https://github.com/actions/runner/blob/8109c962f09d9acc473d92c595ff43afceddb347/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs#L9-L17 switch s.Shell { - case "", "bash": - shellCommand = "bash --noprofile --norc -e -o pipefail {0}" + case "": + shellCommand = "bash -e {0}" + case "bash": + if s.WorkflowShell == "" { + shellCommand = "bash -e {0}" + } else { + shellCommand = "bash --noprofile --norc -e -o pipefail {0}" + } case "pwsh": shellCommand = "pwsh -command . '{0}'" case "python": diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 03f29fd6967..92de8ca33fe 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -397,15 +397,18 @@ func TestReadWorkflow_Strategy(t *testing.T) { func TestStep_ShellCommand(t *testing.T) { tests := []struct { shell string + workflowShell string want string }{ - {"pwsh -v '. {0}'", "pwsh -v '. {0}'"}, - {"pwsh", "pwsh -command . '{0}'"}, - {"powershell", "powershell -command . '{0}'"}, + {"pwsh -v '. {0}'", "", "pwsh -v '. {0}'"}, + {"pwsh", "", "pwsh -command . '{0}'"}, + {"powershell", "", "powershell -command . '{0}'"}, + {"bash", "", "bash -e {0}"}, + {"bash", "bash", "bash --noprofile --norc -e -o pipefail {0}"}, } for _, tt := range tests { t.Run(tt.shell, func(t *testing.T) { - got := (&Step{Shell: tt.shell}).ShellCommand() + got := (&Step{Shell: tt.shell, WorkflowShell: tt.workflowShell}).ShellCommand() assert.Equal(t, got, tt.want) }) } diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index 692b4eefa75..0656aad881c 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -387,15 +387,15 @@ func TestGetGitHubContext(t *testing.T) { owner = o } - assert.Equal(t, ghc.RunID, "1") - assert.Equal(t, ghc.RunNumber, "1") - assert.Equal(t, ghc.RetentionDays, "0") - assert.Equal(t, ghc.Actor, actor) - assert.Equal(t, ghc.Repository, repo) - assert.Equal(t, ghc.RepositoryOwner, owner) - assert.Equal(t, ghc.RunnerPerflog, "/dev/null") - assert.Equal(t, ghc.Token, rc.Config.Secrets["GITHUB_TOKEN"]) - assert.Equal(t, ghc.Job, "job1") + assert.Equal(t, "1", ghc.RunID) + assert.Equal(t, "1", ghc.RunNumber) + assert.Equal(t, "0", ghc.RetentionDays) + assert.Equal(t, actor, ghc.Actor) + assert.Equal(t, repo, ghc.Repository) + assert.Equal(t, owner, ghc.RepositoryOwner) + assert.Equal(t, "/dev/null", ghc.RunnerPerflog) + assert.Equal(t, rc.Config.Secrets["GITHUB_TOKEN"], ghc.Token) + assert.Equal(t, "job1", ghc.Job) } func TestGetGithubContextRef(t *testing.T) { diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index 054ed702321..a905a20e1f6 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -166,16 +166,18 @@ func (sr *stepRun) setupShell(ctx context.Context) { step := sr.Step if step.Shell == "" { - step.Shell = rc.Run.Job().Defaults.Run.Shell + step.WorkflowShell = rc.Run.Job().Defaults.Run.Shell + } else { + step.WorkflowShell = step.Shell } - step.Shell = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, step.Shell) + step.WorkflowShell = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, step.WorkflowShell) - if step.Shell == "" { - step.Shell = rc.Run.Workflow.Defaults.Run.Shell + if step.WorkflowShell == "" { + step.WorkflowShell = rc.Run.Workflow.Defaults.Run.Shell } - if step.Shell == "" { + if step.WorkflowShell == "" { if _, ok := rc.JobContainer.(*container.HostEnvironment); ok { shellWithFallback := []string{"bash", "sh"} // Don't use bash on windows by default, if not using a docker container @@ -196,6 +198,8 @@ func (sr *stepRun) setupShell(ctx context.Context) { // Currently only linux containers are supported, use sh by default like actions/runner step.Shell = "sh" } + } else { + step.Shell = step.WorkflowShell } }