From f6a93c0b5552c69a4f1cb4088e1d9323edf9101b Mon Sep 17 00:00:00 2001 From: Marc Cataford Date: Tue, 20 Aug 2024 23:35:49 -0400 Subject: [PATCH] feat: support for cascading workflow/job/step env --- WORKFLOW_SUPPORT.md | 6 +- internal/runner/driver.go | 2 +- internal/runner/mock_container_driver.go | 4 +- internal/runner/podman_driver.go | 27 ++++- internal/runner/runner.go | 8 +- internal/runner/runner_flow_test.go | 11 +- internal/runner/runner_job_test.go | 9 +- internal/workflow/models.go | 30 +++++- internal/workflow/models_test.go | 128 +++++++++++++++++++++++ 9 files changed, 199 insertions(+), 26 deletions(-) diff --git a/WORKFLOW_SUPPORT.md b/WORKFLOW_SUPPORT.md index 60fd03d..48ca16d 100644 --- a/WORKFLOW_SUPPORT.md +++ b/WORKFLOW_SUPPORT.md @@ -9,7 +9,7 @@ syntax](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for - [ ] run-name - [ ] on - [ ] permissions -- [ ] env +- [x] env - [ ] defaults - [x] jobs - [x] jobs..name @@ -20,7 +20,7 @@ syntax](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for - [ ] jobs..environment - [ ] jobs..concurrency - [ ] jobs..outputs - - [ ] jobs..env + - [x] jobs..env - [x] jobs..defaults - [ ] jobs..run.shell - [x] jobs..run.working-directory @@ -42,7 +42,7 @@ syntax](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for - [x] jobs..steps[*].working-directory - [ ] jobs..steps[*].shell - [ ] jobs..steps[*].with - - [ ] jobs..steps[*].env + - [x] jobs..steps[*].env - [X] jobs..steps[*].continue-on-error - [ ] jobs..steps[*].timeout-minutes diff --git a/internal/runner/driver.go b/internal/runner/driver.go index f92af5c..099a582 100644 --- a/internal/runner/driver.go +++ b/internal/runner/driver.go @@ -8,7 +8,7 @@ type ContainerDriver interface { Pull(string) error Start(string, string) error Stop(string) error - Exec(containerId string, command string, cwd string) CommandResult + Exec(containerId string, command string, cwd string, env map[string]string) CommandResult } // Represents the outcome of a command call made by the driver. diff --git a/internal/runner/mock_container_driver.go b/internal/runner/mock_container_driver.go index 8616517..4f90647 100644 --- a/internal/runner/mock_container_driver.go +++ b/internal/runner/mock_container_driver.go @@ -49,12 +49,12 @@ func (d *MockDriver) Stop(uri string) error { } -func (d *MockDriver) Exec(containerName string, command string, cwd string) CommandResult { +func (d *MockDriver) Exec(containerName string, command string, cwd string, env map[string]string) CommandResult { if _, init := d.calls["Exec"]; !init { d.calls["Exec"] = []MockCall{} } - args := []string{containerName, command, cwd} + args := []string{containerName, command, cwd, fmt.Sprintf("%#v", env)} d.calls["Exec"] = append(d.calls["Exec"], MockCall{fname: "Exec", args: args}) mockKeys := []string{ diff --git a/internal/runner/podman_driver.go b/internal/runner/podman_driver.go index bd9edff..a393136 100644 --- a/internal/runner/podman_driver.go +++ b/internal/runner/podman_driver.go @@ -79,13 +79,34 @@ func (d PodmanDriver) Stop(containerName string) error { return nil } -func (d PodmanDriver) Exec(containerId string, command string, cwd string) CommandResult { - cmd := exec.Command("podman", "exec", "--workdir", cwd, containerId, "bash", "-c", command) +func (d PodmanDriver) Exec(containerId string, command string, cwd string, env map[string]string) CommandResult { + envArgs := []string{} + + for key, value := range env { + envArgs = append(envArgs, fmt.Sprintf("-e=%s=%s", key, value)) + } + + commandArgs := []string{ + "exec", + "--workdir", + cwd, + } + + commandArgs = append(commandArgs, envArgs...) + + commandArgs = append(commandArgs, containerId, + "bash", + "-c", + command, + ) + + cmd := exec.Command("podman", commandArgs...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - err := cmd.Run() + fmt.Printf("%#v", cmd) + return CommandResult{ Error: err, ExitCode: cmd.ProcessState.ExitCode(), diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 8506971..5ad2ed8 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -103,8 +103,8 @@ func (r Runner) runJob(jobContext context.Context, jobTracker *TaskTracker, jobW // // If the command raises an error while in the container or fails to run // the command at all, an error is returned, otherwise nil. -func (r *Runner) RunCommandInContainer(containerId string, command string, stepCwd string) error { - result := r.Driver.Exec(containerId, command, stepCwd) +func (r *Runner) RunCommandInContainer(containerId string, command string, stepCwd string, stepEnv map[string]string) error { + result := r.Driver.Exec(containerId, command, stepCwd, stepEnv) if result.Error != nil { return result.Error @@ -133,12 +133,14 @@ func (r *Runner) RunJobInContainer(imageUri string, containerId string, jobConte logger.Info("Started %s", containerId) for stepIndex, step := range job.Steps { stepCwd := jobContext.Value("workflow").(workflow.Workflow).GetWorkingDirectory(job.Name, stepIndex) + stepEnv := jobContext.Value("workflow").(workflow.Workflow).GetEnv(job.Name, stepIndex) + logger.Info("Run: %s", step.Run) logger.Info("Using working directory %s", stepCwd) var stepError error if step.Run != "" { - stepError = r.RunCommandInContainer(containerId, step.Run, stepCwd) + stepError = r.RunCommandInContainer(containerId, step.Run, stepCwd, stepEnv) } if stepError != nil && !step.ContinueOnError { diff --git a/internal/runner/runner_flow_test.go b/internal/runner/runner_flow_test.go index 189d357..2ac0a48 100644 --- a/internal/runner/runner_flow_test.go +++ b/internal/runner/runner_flow_test.go @@ -5,6 +5,7 @@ import ( logger "courgette/internal/logging" workflow "courgette/internal/workflow" "errors" + "fmt" "testing" ) @@ -14,13 +15,13 @@ func init() { func TestRunnerRunCommandInContainerReturnsErrorFromDriver(t *testing.T) { mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{ExitCode: 0, Error: errors.New("test")}, "test-container", "test-command", ".") + mockDriver.WithMockedCall("Exec", CommandResult{ExitCode: 0, Error: errors.New("test")}, "test-container", "test-command", ".", fmt.Sprintf("%#v", map[string]string{})) runner := Runner{ Driver: &mockDriver, } - err := runner.RunCommandInContainer("test-container", "test-command", ".") + err := runner.RunCommandInContainer("test-container", "test-command", ".", map[string]string{}) if err == nil { t.Errorf("Expected error, got nil.") @@ -29,13 +30,13 @@ func TestRunnerRunCommandInContainerReturnsErrorFromDriver(t *testing.T) { func TestRunnerRunCommandInContainerReturnsErrorIfCommandExitCodeNonzero(t *testing.T) { mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{ExitCode: 1, Error: nil}, "test-container", "test-command", ".") + mockDriver.WithMockedCall("Exec", CommandResult{ExitCode: 1, Error: nil}, "test-container", "test-command", ".", fmt.Sprintf("%#v", map[string]string{})) runner := Runner{ Driver: &mockDriver, } - err := runner.RunCommandInContainer("test-container", "test-command", ".") + err := runner.RunCommandInContainer("test-container", "test-command", ".", map[string]string{}) if err == nil { t.Errorf("Expected error, got nil.") @@ -44,7 +45,7 @@ func TestRunnerRunCommandInContainerReturnsErrorIfCommandExitCodeNonzero(t *test func TestRunJobInContainerSchedulesStoppingContainers(t *testing.T) { mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{ExitCode: 1, Error: nil}, "test-container", "test-command", ".") + mockDriver.WithMockedCall("Exec", CommandResult{ExitCode: 1, Error: nil}, "test-container", "test-command", ".", fmt.Sprintf("%#v", map[string]string{})) runner := NewRunner(&mockDriver, map[string]string{}) diff --git a/internal/runner/runner_job_test.go b/internal/runner/runner_job_test.go index 77edf51..3a916f1 100644 --- a/internal/runner/runner_job_test.go +++ b/internal/runner/runner_job_test.go @@ -4,6 +4,7 @@ import ( "context" workflow "courgette/internal/workflow" "errors" + "fmt" "testing" ) @@ -70,7 +71,7 @@ jobs: workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1") + mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1", fmt.Sprintf("%#v", map[string]string{})) runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) jobContext := context.WithValue(context.Background(), "workflow", *workflow) @@ -98,7 +99,7 @@ jobs: workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1") + mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1", fmt.Sprintf("%#v", map[string]string{})) runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) jobContext := context.WithValue(context.Background(), "workflow", *workflow) @@ -130,7 +131,7 @@ jobs: workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1", ".") + mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1", ".", fmt.Sprintf("%#v", map[string]string{})) runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) jobContext := context.WithValue(context.Background(), "workflow", *workflow) @@ -161,7 +162,7 @@ jobs: workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) mockDriver := NewMockDriver() - mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1", ".") + mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1", ".", fmt.Sprintf("%#v", map[string]string{})) runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) jobContext := context.WithValue(context.Background(), "workflow", *workflow) diff --git a/internal/workflow/models.go b/internal/workflow/models.go index fefacd4..41ff373 100644 --- a/internal/workflow/models.go +++ b/internal/workflow/models.go @@ -2,6 +2,7 @@ package workflow import ( "errors" + "maps" ) type Job struct { @@ -11,7 +12,8 @@ type Job struct { // Job name; this isn't guaranteed to be unique. Name string `yaml:"name"` // If truthy, job-level failure does not bubble up to the workflow. - ContinueOnError bool `yaml:"continue-on-error"` + ContinueOnError bool `yaml:"continue-on-error"` + Env map[string]string `yaml:"env"` Defaults struct { Run struct { WorkingDirectory string `yaml:"working-directory"` @@ -38,9 +40,10 @@ func (j Job) Validate() []error { } type Step struct { - Run string `yaml:"run"` - WorkingDirectory string `yaml:"working-directory"` - ContinueOnError bool `yaml:"continue-on-error"` + Run string `yaml:"run"` + WorkingDirectory string `yaml:"working-directory"` + ContinueOnError bool `yaml:"continue-on-error"` + Env map[string]string `yaml:"env"` } func (s Step) Validate() []error { @@ -55,7 +58,8 @@ func (s Step) Validate() []error { type Workflow struct { SourcePath string - Jobs map[string]Job `yaml:"jobs"` + Jobs map[string]Job `yaml:"jobs"` + Env map[string]string `yaml:"env"` } // Returns the given workflow's job+step working directory inside the container @@ -79,6 +83,22 @@ func (w Workflow) GetWorkingDirectory(jobName string, stepIndex int) string { return "." } +// Returns the merged map of environment variants defined by: +// - Workflow-level "env" +// - Job-level "env" +// - Step-level "env" +// The environment is merged in order, and name collisions overwrite +// previous values such that jobs can overwrite workflows, and steps, jobs. +func (w Workflow) GetEnv(jobName string, stepIndex int) map[string]string { + finalEnv := map[string]string{} + + maps.Copy(finalEnv, w.Env) + maps.Copy(finalEnv, w.Jobs[jobName].Env) + maps.Copy(finalEnv, w.Jobs[jobName].Steps[stepIndex].Env) + + return finalEnv +} + func (w Workflow) Validate() []error { validationErrors := []error{} diff --git a/internal/workflow/models_test.go b/internal/workflow/models_test.go index 8a541be..8326d89 100644 --- a/internal/workflow/models_test.go +++ b/internal/workflow/models_test.go @@ -1,6 +1,7 @@ package workflow import ( + "reflect" "testing" ) @@ -78,3 +79,130 @@ jobs: } } + +func TestWorkflowEnv(t *testing.T) { + sample := ` +env: + TEST: 1 +jobs: + jobA: + runs-on: default + steps: + - run: echo "test" + ` + + workflow, _ := FromYamlBytes([]byte(sample)) + + env := workflow.GetEnv("jobA", 0) + + if !reflect.DeepEqual(env, map[string]string{"TEST": "1"}) { + t.Errorf("Unexpected env: %#v", env) + } +} + +func TestJobEnv(t *testing.T) { + sample := ` +jobs: + jobA: + env: + TEST: 1 + runs-on: default + steps: + - run: echo "test" + ` + + workflow, _ := FromYamlBytes([]byte(sample)) + + env := workflow.GetEnv("jobA", 0) + + if !reflect.DeepEqual(env, map[string]string{"TEST": "1"}) { + t.Errorf("Unexpected env: %#v", env) + } +} + +func TestStepEnv(t *testing.T) { + sample := ` +jobs: + jobA: + + runs-on: default + steps: + - run: echo "test" + env: + TEST: 1 + ` + + workflow, _ := FromYamlBytes([]byte(sample)) + + env := workflow.GetEnv("jobA", 0) + + if !reflect.DeepEqual(env, map[string]string{"TEST": "1"}) { + t.Errorf("Unexpected env: %#v", env) + } +} + +func TestJobEnvOverwritesWorkflowEnv(t *testing.T) { + sample := ` + env: + TEST: 1 + jobs: + jobA: + env: + TEST: 2 + runs-on: default + steps: + - run: echo "test" + ` + + workflow, _ := FromYamlBytes([]byte(sample)) + + env := workflow.GetEnv("jobA", 0) + + if !reflect.DeepEqual(env, map[string]string{"TEST": "2"}) { + t.Errorf("Unexpected env: %#v", env) + } +} + +func TestStepEnvOverwritesWorkflowEnv(t *testing.T) { + sample := ` + env: + TEST: 1 + jobs: + jobA: + runs-on: default + steps: + - run: echo "test" + env: + TEST: 2 + ` + + workflow, _ := FromYamlBytes([]byte(sample)) + + env := workflow.GetEnv("jobA", 0) + + if !reflect.DeepEqual(env, map[string]string{"TEST": "2"}) { + t.Errorf("Unexpected env: %#v", env) + } +} + +func TestStepEnvOverwritesJobEnv(t *testing.T) { + sample := ` + jobs: + jobA: + env: + TEST: 1 + runs-on: default + steps: + - run: echo "test" + env: + TEST: 2 + ` + + workflow, _ := FromYamlBytes([]byte(sample)) + + env := workflow.GetEnv("jobA", 0) + + if !reflect.DeepEqual(env, map[string]string{"TEST": "2"}) { + t.Errorf("Unexpected env: %#v", env) + } +}