diff --git a/server/events/mocks/mock_env_step_runner.go b/server/events/mocks/mock_env_step_runner.go index 461812701b..c05782666e 100644 --- a/server/events/mocks/mock_env_step_runner.go +++ b/server/events/mocks/mock_env_step_runner.go @@ -25,27 +25,23 @@ func NewMockEnvStepRunner(options ...pegomock.Option) *MockEnvStepRunner { func (mock *MockEnvStepRunner) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockEnvStepRunner) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockEnvStepRunner) Run(ctx models.ProjectCommandContext, name string, cmd string, value string, path string, envs map[string]string) (string, string, error) { +func (mock *MockEnvStepRunner) Run(ctx models.ProjectCommandContext, cmd string, value string, path string, envs map[string]string) (string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockEnvStepRunner().") } - params := []pegomock.Param{ctx, name, cmd, value, path, envs} - result := pegomock.GetGenericMockFrom(mock).Invoke("Run", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + params := []pegomock.Param{ctx, cmd, value, path, envs} + result := pegomock.GetGenericMockFrom(mock).Invoke("Run", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 string - var ret1 string - var ret2 error + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(string) } if result[1] != nil { - ret1 = result[1].(string) - } - if result[2] != nil { - ret2 = result[2].(error) + ret1 = result[1].(error) } } - return ret0, ret1, ret2 + return ret0, ret1 } func (mock *MockEnvStepRunner) VerifyWasCalledOnce() *VerifierMockEnvStepRunner { @@ -85,8 +81,8 @@ type VerifierMockEnvStepRunner struct { timeout time.Duration } -func (verifier *VerifierMockEnvStepRunner) Run(ctx models.ProjectCommandContext, name string, cmd string, value string, path string, envs map[string]string) *MockEnvStepRunner_Run_OngoingVerification { - params := []pegomock.Param{ctx, name, cmd, value, path, envs} +func (verifier *VerifierMockEnvStepRunner) Run(ctx models.ProjectCommandContext, cmd string, value string, path string, envs map[string]string) *MockEnvStepRunner_Run_OngoingVerification { + params := []pegomock.Param{ctx, cmd, value, path, envs} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Run", params, verifier.timeout) return &MockEnvStepRunner_Run_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -96,12 +92,12 @@ type MockEnvStepRunner_Run_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockEnvStepRunner_Run_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string, string, string, string, map[string]string) { - ctx, name, cmd, value, path, envs := c.GetAllCapturedArguments() - return ctx[len(ctx)-1], name[len(name)-1], cmd[len(cmd)-1], value[len(value)-1], path[len(path)-1], envs[len(envs)-1] +func (c *MockEnvStepRunner_Run_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string, string, string, map[string]string) { + ctx, cmd, value, path, envs := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], cmd[len(cmd)-1], value[len(value)-1], path[len(path)-1], envs[len(envs)-1] } -func (c *MockEnvStepRunner_Run_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string, _param2 []string, _param3 []string, _param4 []string, _param5 []map[string]string) { +func (c *MockEnvStepRunner_Run_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string, _param2 []string, _param3 []string, _param4 []map[string]string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.ProjectCommandContext, len(params[0])) @@ -120,13 +116,9 @@ func (c *MockEnvStepRunner_Run_OngoingVerification) GetAllCapturedArguments() (_ for u, param := range params[3] { _param3[u] = param.(string) } - _param4 = make([]string, len(params[4])) + _param4 = make([]map[string]string, len(params[4])) for u, param := range params[4] { - _param4[u] = param.(string) - } - _param5 = make([]map[string]string, len(params[5])) - for u, param := range params[5] { - _param5[u] = param.(map[string]string) + _param4[u] = param.(map[string]string) } } return diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index b5240910d8..3bd813052a 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -67,7 +67,7 @@ type CustomStepRunner interface { // EnvStepRunner runs env steps. type EnvStepRunner interface { - Run(ctx models.ProjectCommandContext, name string, cmd string, value string, path string, envs map[string]string) (string, string, error) + Run(ctx models.ProjectCommandContext, cmd string, value string, path string, envs map[string]string) (string, error) } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_webhooks_sender.go WebhooksSender @@ -181,11 +181,10 @@ func (p *DefaultProjectCommandRunner) doPlan(ctx models.ProjectCommandContext) ( func (p *DefaultProjectCommandRunner) runSteps(steps []valid.Step, ctx models.ProjectCommandContext, absPath string) ([]string, error) { var outputs []string + envs := make(map[string]string) for _, step := range steps { - var envs = make(map[string]string) var out string var err error - var name string switch step.StepName { case "init": out, err = p.InitStepRunner.Run(ctx, step.ExtraArgs, absPath, envs) @@ -196,8 +195,10 @@ func (p *DefaultProjectCommandRunner) runSteps(steps []valid.Step, ctx models.Pr case "run": out, err = p.RunStepRunner.Run(ctx, step.RunCommand, absPath, envs) case "env": - name, out, err = p.EnvStepRunner.Run(ctx, step.EnvVarName, step.RunCommand, step.EnvVarValue, absPath, envs) - envs[name] = out + out, err = p.EnvStepRunner.Run(ctx, step.RunCommand, step.EnvVarValue, absPath, envs) + envs[step.EnvVarName] = out + // We reset out to the empty string because we don't want it to + // be printed to the PR, it's solely to set the environment variable. out = "" } diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index 024f446e0a..04be59abbf 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -14,6 +14,8 @@ package events_test import ( + "github.com/hashicorp/go-version" + "github.com/runatlantis/atlantis/server/events/runtime" "os" "testing" @@ -23,6 +25,7 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" mocks2 "github.com/runatlantis/atlantis/server/events/runtime/mocks" + tmocks "github.com/runatlantis/atlantis/server/events/terraform/mocks" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" . "github.com/runatlantis/atlantis/testing" @@ -34,8 +37,8 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { mockInit := mocks.NewMockStepRunner() mockPlan := mocks.NewMockStepRunner() mockApply := mocks.NewMockStepRunner() - mockEnv := mocks.NewMockEnvStepRunner() mockRun := mocks.NewMockCustomStepRunner() + realEnv := runtime.EnvStepRunner{} mockWorkingDir := mocks.NewMockWorkingDir() mockLocker := mocks.NewMockProjectLocker() @@ -46,14 +49,13 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { PlanStepRunner: mockPlan, ApplyStepRunner: mockApply, RunStepRunner: mockRun, - EnvStepRunner: mockEnv, + EnvStepRunner: &realEnv, PullApprovedChecker: nil, WorkingDir: mockWorkingDir, Webhooks: nil, WorkingDirLocker: events.NewDefaultWorkingDirLocker(), } - envs := make(map[string]string) repoDir, cleanup := TempDir(t) defer cleanup() When(mockWorkingDir.Clone( @@ -74,9 +76,17 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { LockKey: "lock-key", }, nil) + expEnvs := map[string]string{ + "name": "value", + } ctx := models.ProjectCommandContext{ Log: logging.NewNoopLogger(), Steps: []valid.Step{ + { + StepName: "env", + EnvVarName: "name", + EnvVarValue: "value", + }, { StepName: "run", }, @@ -89,41 +99,32 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { { StepName: "init", }, - { - StepName: "env", - EnvVarName: "name", - EnvVarValue: "value", - }, }, Workspace: "default", RepoRelDir: ".", } // Each step will output its step name. - When(mockInit.Run(ctx, nil, repoDir, envs)).ThenReturn("init", nil) - When(mockPlan.Run(ctx, nil, repoDir, envs)).ThenReturn("plan", nil) - When(mockApply.Run(ctx, nil, repoDir, envs)).ThenReturn("apply", nil) - When(mockRun.Run(ctx, "", repoDir, envs)).ThenReturn("run", nil) - When(mockEnv.Run(ctx, "name", "", "value", repoDir, envs)).ThenReturn("name", "value", nil) + When(mockInit.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("init", nil) + When(mockPlan.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("plan", nil) + When(mockApply.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("apply", nil) + When(mockRun.Run(ctx, "", repoDir, expEnvs)).ThenReturn("run", nil) res := runner.Plan(ctx) Assert(t, res.PlanSuccess != nil, "exp plan success") Equals(t, "https://lock-key", res.PlanSuccess.LockURL) Equals(t, "run\napply\nplan\ninit", res.PlanSuccess.TerraformOutput) - expSteps := []string{"env", "run", "apply", "plan", "init"} - var newEnv = map[string]string{"name": "value"} + expSteps := []string{"run", "apply", "plan", "init", "env"} for _, step := range expSteps { switch step { case "init": - mockInit.VerifyWasCalledOnce().Run(ctx, nil, repoDir, envs) + mockInit.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "plan": - mockPlan.VerifyWasCalledOnce().Run(ctx, nil, repoDir, envs) + mockPlan.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "apply": - mockApply.VerifyWasCalledOnce().Run(ctx, nil, repoDir, envs) + mockApply.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "run": - mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, envs) - case "env": - mockEnv.VerifyWasCalledOnce().Run(ctx, "name", "", "value", repoDir, newEnv) + mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, expEnvs) } } } @@ -237,6 +238,11 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { { description: "workflow with custom apply stage", steps: []valid.Step{ + { + StepName: "env", + EnvVarName: "key", + EnvVarValue: "value", + }, { StepName: "run", }, @@ -249,16 +255,16 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { { StepName: "init", }, - { - StepName: "env", - }, }, - expSteps: []string{"run", "apply", "plan", "init", "env"}, + expSteps: []string{"env", "run", "apply", "plan", "init"}, expOut: "run\napply\nplan\ninit", }, } for _, c := range cases { + if c.description != "workflow with custom apply stage" { + continue + } t.Run(c.description, func(t *testing.T) { RegisterMockTestingT(t) mockInit := mocks.NewMockStepRunner() @@ -284,7 +290,6 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { Webhooks: mockSender, WorkingDirLocker: events.NewDefaultWorkingDirLocker(), } - envs := make(map[string]string) repoDir, cleanup := TempDir(t) defer cleanup() When(mockWorkingDir.GetWorkingDir( @@ -301,10 +306,14 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { RepoRelDir: ".", PullMergeable: c.pullMergeable, } - When(mockInit.Run(ctx, nil, repoDir, envs)).ThenReturn("init", nil) - When(mockPlan.Run(ctx, nil, repoDir, envs)).ThenReturn("plan", nil) - When(mockApply.Run(ctx, nil, repoDir, envs)).ThenReturn("apply", nil) - When(mockRun.Run(ctx, "", repoDir, envs)).ThenReturn("run", nil) + expEnvs := map[string]string{ + "key": "value", + } + When(mockInit.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("init", nil) + When(mockPlan.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("plan", nil) + When(mockApply.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("apply", nil) + When(mockRun.Run(ctx, "", repoDir, expEnvs)).ThenReturn("run", nil) + When(mockEnv.Run(ctx, "", "value", repoDir, make(map[string]string))).ThenReturn("value", nil) When(mockApproved.PullIsApproved(ctx.BaseRepo, ctx.Pull)).ThenReturn(true, nil) res := runner.Apply(ctx) @@ -316,19 +325,114 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { case "approved": mockApproved.VerifyWasCalledOnce().PullIsApproved(ctx.BaseRepo, ctx.Pull) case "init": - mockInit.VerifyWasCalledOnce().Run(ctx, nil, repoDir, envs) + mockInit.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "plan": - mockPlan.VerifyWasCalledOnce().Run(ctx, nil, repoDir, envs) + mockPlan.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "apply": - mockApply.VerifyWasCalledOnce().Run(ctx, nil, repoDir, envs) + mockApply.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "run": - mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, envs) + mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, expEnvs) + case "env": + mockEnv.VerifyWasCalledOnce().Run(ctx, "", "value", repoDir, expEnvs) } } }) } } +// Test run and env steps. We don't use mocks for this test since we're +// not running any Terraform. +func TestDefaultProjectCommandRunner_RunEnvSteps(t *testing.T) { + RegisterMockTestingT(t) + tfClient := tmocks.NewMockClient() + tfVersion, err := version.NewVersion("0.12.0") + Ok(t, err) + run := runtime.RunStepRunner{ + TerraformExecutor: tfClient, + DefaultTFVersion: tfVersion, + } + env := runtime.EnvStepRunner{ + RunStepRunner: &run, + } + mockWorkingDir := mocks.NewMockWorkingDir() + mockLocker := mocks.NewMockProjectLocker() + + runner := events.DefaultProjectCommandRunner{ + Locker: mockLocker, + LockURLGenerator: mockURLGenerator{}, + RunStepRunner: &run, + EnvStepRunner: &env, + PullApprovedChecker: nil, + WorkingDir: mockWorkingDir, + Webhooks: nil, + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + } + + repoDir, cleanup := TempDir(t) + defer cleanup() + When(mockWorkingDir.Clone( + matchers.AnyPtrToLoggingSimpleLogger(), + matchers.AnyModelsRepo(), + matchers.AnyModelsRepo(), + matchers.AnyModelsPullRequest(), + AnyString(), + )).ThenReturn(repoDir, nil) + When(mockLocker.TryLock( + matchers.AnyPtrToLoggingSimpleLogger(), + matchers.AnyModelsPullRequest(), + matchers.AnyModelsUser(), + AnyString(), + matchers.AnyModelsProject(), + )).ThenReturn(&events.TryLockResponse{ + LockAcquired: true, + LockKey: "lock-key", + }, nil) + + ctx := models.ProjectCommandContext{ + Log: logging.NewNoopLogger(), + Steps: []valid.Step{ + { + StepName: "run", + RunCommand: "echo var=$var", + }, + { + StepName: "env", + EnvVarName: "var", + EnvVarValue: "value", + }, + { + StepName: "run", + RunCommand: "echo var=$var", + }, + { + StepName: "env", + EnvVarName: "dynamic_var", + RunCommand: "echo dynamic_value", + }, + { + StepName: "run", + RunCommand: "echo dynamic_var=$dynamic_var", + }, + // Test overriding the variable + { + StepName: "env", + EnvVarName: "dynamic_var", + EnvVarValue: "overridden", + }, + { + StepName: "run", + RunCommand: "echo dynamic_var=$dynamic_var", + }, + }, + Workspace: "default", + RepoRelDir: ".", + } + res := runner.Plan(ctx) + Assert(t, res.PlanSuccess != nil, "exp plan success") + Equals(t, "https://lock-key", res.PlanSuccess.LockURL) + Equals(t, "var=\n\nvar=value\n\ndynamic_var=dynamic_value\n\ndynamic_var=overridden\n", res.PlanSuccess.TerraformOutput) +} + type mockURLGenerator struct{} func (m mockURLGenerator) GenerateLockURL(lockID string) string { diff --git a/server/events/runtime/env_step_runner.go b/server/events/runtime/env_step_runner.go index 74afe19f48..28a8d481f2 100644 --- a/server/events/runtime/env_step_runner.go +++ b/server/events/runtime/env_step_runner.go @@ -1,26 +1,26 @@ package runtime import ( - "github.com/hashicorp/go-version" "github.com/runatlantis/atlantis/server/events/models" + "strings" ) // EnvStepRunner set environment variables. type EnvStepRunner struct { - TerraformExecutor TerraformExec - DefaultTFVersion *version.Version + RunStepRunner *RunStepRunner } -func (r *EnvStepRunner) Run(ctx models.ProjectCommandContext, name string, command string, value string, path string, envs map[string]string) (string, string, error) { +// Run runs the env step command. +// value is the value for the environment variable. If set this is returned as +// the value. Otherwise command is run and its output is the value returned. +func (r *EnvStepRunner) Run(ctx models.ProjectCommandContext, command string, value string, path string, envs map[string]string) (string, error) { if value != "" { - return name, value, nil + return value, nil } - - runStepRunner := RunStepRunner{ - TerraformExecutor: r.TerraformExecutor, - DefaultTFVersion: r.DefaultTFVersion, - } - res, err := runStepRunner.Run(ctx, command, path, envs) - - return name, res, err + res, err := r.RunStepRunner.Run(ctx, command, path, envs) + // Trim newline from res to support running `echo env_value` which has + // a newline. We don't recommend users run echo -n env_value to remove the + // newline because -n doesn't work in the sh shell which is what we use + // to run commands. + return strings.TrimSuffix(res, "\n"), err } diff --git a/server/events/runtime/env_step_runner_test.go b/server/events/runtime/env_step_runner_test.go index fe281e9667..62b6f255fc 100644 --- a/server/events/runtime/env_step_runner_test.go +++ b/server/events/runtime/env_step_runner_test.go @@ -9,6 +9,7 @@ import ( "github.com/runatlantis/atlantis/server/events/terraform/mocks" "github.com/runatlantis/atlantis/server/logging" + . "github.com/petergtz/pegomock" . "github.com/runatlantis/atlantis/testing" ) @@ -22,7 +23,7 @@ func TestEnvStepRunner_Run(t *testing.T) { }{ { Command: "echo 123", - ExpValue: "123\n", + ExpValue: "123", }, { Value: "test", @@ -34,13 +35,16 @@ func TestEnvStepRunner_Run(t *testing.T) { ExpValue: "test", }, } - terraform := mocks.NewMockClient() - projVersion, err := version.NewVersion("v0.11.0") + RegisterMockTestingT(t) + tfClient := mocks.NewMockClient() + tfVersion, err := version.NewVersion("0.12.0") Ok(t, err) - defaultVersion, _ := version.NewVersion("0.8") - r := runtime.EnvStepRunner{ - TerraformExecutor: terraform, - DefaultTFVersion: defaultVersion, + runStepRunner := runtime.RunStepRunner{ + TerraformExecutor: tfClient, + DefaultTFVersion: tfVersion, + } + envRunner := runtime.EnvStepRunner{ + RunStepRunner: &runStepRunner, } for _, c := range cases { t.Run(c.Command, func(t *testing.T) { @@ -67,10 +71,10 @@ func TestEnvStepRunner_Run(t *testing.T) { Log: logging.NewNoopLogger(), Workspace: "myworkspace", RepoRelDir: "mydir", - TerraformVersion: projVersion, + TerraformVersion: tfVersion, ProjectName: c.ProjectName, } - _, value, err := r.Run(ctx, "var", c.Command, c.Value, tmpDir, map[string]string(nil)) + value, err := envRunner.Run(ctx, c.Command, c.Value, tmpDir, map[string]string(nil)) if c.ExpErr != "" { ErrContains(t, c.ExpErr, err) return diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index 9d3c1ace91..cdd4149402 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/server/events/yaml/parser_validator_test.go @@ -745,6 +745,62 @@ workflows: }, }, }, + { + description: "env steps", + input: ` +version: 3 +projects: +- dir: "." +workflows: + default: + plan: + steps: + - env: + name: env_name + value: env_value + apply: + steps: + - env: + name: env_name + command: command and args +`, + exp: valid.RepoCfg{ + Version: 3, + Projects: []valid.Project{ + { + Dir: ".", + Workspace: "default", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + }, + }, + Workflows: map[string]valid.Workflow{ + "default": { + Name: "default", + Plan: valid.Stage{ + Steps: []valid.Step{ + { + StepName: "env", + EnvVarName: "env_name", + EnvVarValue: "env_value", + }, + }, + }, + Apply: valid.Stage{ + Steps: []valid.Step{ + { + StepName: "env", + EnvVarName: "env_name", + RunCommand: "command and args", + }, + }, + }, + }, + }, + }, + }, } tmpDir, cleanup := TempDir(t) diff --git a/server/events/yaml/raw/step.go b/server/events/yaml/raw/step.go index b4026ae7b9..2ebf61ba78 100644 --- a/server/events/yaml/raw/step.go +++ b/server/events/yaml/raw/step.go @@ -27,10 +27,11 @@ const ( // 1. A single string for a built-in command: // - init // - plan -// 2. A map for a built-in env with name and command +// 2. A map for an env step with name and command or value // - env: // name: test // command: echo 312 +// value: value // 3. A map for a built-in command and extra_args: // - plan: // extra_args: [-var-file=staging.tfvars] @@ -42,7 +43,7 @@ type Step struct { // Key will be set in case #1 and #3 above to the key. In case #2, there // could be multiple keys (since the element is a map) so we don't set Key. Key *string - // Map will be set in case #2 above. + // Env will be set in case #2 above. Env map[string]map[string]string // Map will be set in case #3 above. Map map[string]map[string][]string @@ -89,6 +90,8 @@ func (s Step) Validate() error { for k := range args { argKeys = append(argKeys, k) } + // Sort so tests can be deterministic. + sort.Strings(argKeys) // args should contain a single 'extra_args' key. if len(argKeys) > 1 { @@ -125,15 +128,26 @@ func (s Step) Validate() error { for k := range args { argKeys = append(argKeys, k) } - if len(argKeys) != 2 { - return fmt.Errorf("built-in steps only support two keys %s and %s or %s, found %d: %s", - NameArgKey, CommandArgKey, ValueArgKey, len(argKeys), strings.Join(argKeys, ",")) - } + // Sort so tests can be deterministic. + sort.Strings(argKeys) + + foundNameKey := false for k := range args { if k != NameArgKey && k != CommandArgKey && k != ValueArgKey { - return fmt.Errorf("built-in steps only support two keys %s and %s or %s, found %q in step %s", NameArgKey, CommandArgKey, ValueArgKey, k, stepName) + return fmt.Errorf("env steps only support keys %q, %q and %q, found key %q", NameArgKey, ValueArgKey, CommandArgKey, k) + } + if k == NameArgKey { + foundNameKey = true } } + if !foundNameKey { + return fmt.Errorf("env steps must have a %q key set", NameArgKey) + } + // If we have 3 keys at this point then they've set both command and value. + if len(argKeys) != 2 { + return fmt.Errorf("env steps only support one of the %q or %q keys, found both", + ValueArgKey, CommandArgKey) + } } return nil } @@ -257,12 +271,11 @@ func (s *Step) unmarshalGeneric(unmarshal func(interface{}) error) error { return nil } - // This represents a step with extra_args, ex: + // This represents an env step, ex: // env: // name: k // value: hi //optional // command: exec - // We validate if the key env var envStep map[string]map[string]string err = unmarshal(&envStep) if err == nil { diff --git a/server/events/yaml/raw/step_test.go b/server/events/yaml/raw/step_test.go index 001136c184..c99a49596c 100644 --- a/server/events/yaml/raw/step_test.go +++ b/server/events/yaml/raw/step_test.go @@ -73,8 +73,24 @@ key2: }, }, }, + // Env steps { - description: "env step", + description: "env step value", + input: ` +env: + value: direct_value + name: test`, + exp: raw.Step{ + Env: EnvType{ + "env": { + "value": "direct_value", + "name": "test", + }, + }, + }, + }, + { + description: "env step command", input: ` env: command: echo 123 @@ -314,20 +330,31 @@ func TestStep_Validate(t *testing.T) { }, expErr: "built-in steps only support a single extra_args key, found \"invalid\" in step init", }, - // { - // description: "non extra_arg key", - // input: raw.Step{ - // Map: MapType{ - // "init": { - // "invalid": nil, - // "zzzzzzz": nil, - // }, - // }, - // }, - // expErr: "built-in steps only support a single extra_args key, found 2: invalid,zzzzzzz", - // }, - { - description: "incorrect keys in env", + { + description: "non extra_arg key", + input: raw.Step{ + Map: MapType{ + "init": { + "invalid": nil, + "zzzzzzz": nil, + }, + }, + }, + expErr: "built-in steps only support a single extra_args key, found 2: invalid,zzzzzzz", + }, + { + description: "env step with no name key set", + input: raw.Step{ + Env: EnvType{ + "env": { + "value": "value", + }, + }, + }, + expErr: "env steps must have a \"name\" key set", + }, + { + description: "env step with invalid key", input: raw.Step{ Env: EnvType{ "env": { @@ -336,18 +363,20 @@ func TestStep_Validate(t *testing.T) { }, }, }, - expErr: "built-in steps only support two keys name and command or value, found \"abc\" in step env", + expErr: "env steps only support keys \"name\", \"value\" and \"command\", found key \"abc\"", }, { - description: "non two keys in env", + description: "env step with both command and value set", input: raw.Step{ Env: EnvType{ "env": { - "invalid": "", + "name": "name", + "command": "command", + "value": "value", }, }, }, - expErr: "built-in steps only support two keys name and command or value, found 1: invalid", + expErr: "env steps only support one of the \"value\" or \"command\" keys, found both", }, { // For atlantis.yaml v2, this wouldn't parse, but now there should diff --git a/server/events/yaml/valid/repo_cfg.go b/server/events/yaml/valid/repo_cfg.go index 626b7879f5..8fa20aebaf 100644 --- a/server/events/yaml/valid/repo_cfg.go +++ b/server/events/yaml/valid/repo_cfg.go @@ -72,8 +72,10 @@ type Stage struct { } type Step struct { - StepName string - ExtraArgs []string + StepName string + ExtraArgs []string + // RunCommand is either a custom run step or the command to run + // during an env step to populate the environment variable dynamically. RunCommand string // EnvVarName is the name of the // environment variable that should be set by this step. diff --git a/server/server.go b/server/server.go index 2eb3db823f..1274afb24c 100644 --- a/server/server.go +++ b/server/server.go @@ -272,6 +272,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } defaultTfVersion := terraformClient.DefaultVersion() pendingPlanFinder := &events.DefaultPendingPlanFinder{} + runStepRunner := &runtime.RunStepRunner{ + TerraformExecutor: terraformClient, + DefaultTFVersion: defaultTfVersion, + TerraformBinDir: terraformClient.TerraformBinDir(), + } commandRunner := &events.DefaultCommandRunner{ VCSClient: vcsClient, GithubPullGetter: githubClient, @@ -311,14 +316,9 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { CommitStatusUpdater: commitStatusUpdater, AsyncTFExec: terraformClient, }, - RunStepRunner: &runtime.RunStepRunner{ - TerraformExecutor: terraformClient, - DefaultTFVersion: defaultTfVersion, - TerraformBinDir: terraformClient.TerraformBinDir(), - }, + RunStepRunner: runStepRunner, EnvStepRunner: &runtime.EnvStepRunner{ - TerraformExecutor: terraformClient, - DefaultTFVersion: defaultTfVersion, + RunStepRunner: runStepRunner, }, PullApprovedChecker: vcsClient, WorkingDir: workingDir,