From f2c15074ac70f90aaa5126db3a6a625dbabaa5ed Mon Sep 17 00:00:00 2001 From: Wink Saville Date: Tue, 12 Jan 2021 16:02:54 -0800 Subject: [PATCH] Fix issue 416 (#423) This is a solution to issue #416 where environment variables created or changed in the previous step are not usable in the next step because the rc.ExprEval is from the beginning of the previous step. This change refactors setupEnv so that before interpolating the environment variables a NewExpressionEvaluator is created. Fixes: 416 --- pkg/runner/run_context.go | 12 +++----- pkg/runner/step_context.go | 58 +++++++++++++++++++++++++------------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 0baf54e..6c99e6f 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -202,15 +202,11 @@ func (rc *RunContext) newStepExecutor(step *model.Step) common.Executor { Outputs: make(map[string]string), } - _ = sc.setupEnv()(ctx) - - if sc.Env != nil { - err := rc.JobContainer.UpdateFromGithubEnv(&sc.Env)(ctx) - if err != nil { - return err - } + exprEval, err := sc.setupEnv(ctx) + if err != nil { + return err } - rc.ExprEval = sc.NewExpressionEvaluator() + rc.ExprEval = exprEval; runStep, err := rc.EvalBool(sc.Step.If) if err != nil { diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index e135c9b..56e51a5 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -79,33 +79,53 @@ func (sc *StepContext) Executor() common.Executor { return common.NewErrorExecutor(fmt.Errorf("Unable to determine how to run job:%s step:%+v", rc.Run, step)) } -func (sc *StepContext) setupEnv() common.Executor { +func (sc *StepContext) mergeEnv() map[string]string { rc := sc.RunContext job := rc.Run.Job() step := sc.Step - return func(ctx context.Context) error { - var env map[string]string - c := job.Container() - if c != nil { - env = mergeMaps(rc.GetEnv(), c.Env, step.GetEnv()) - } else { - env = mergeMaps(rc.GetEnv(), step.GetEnv()) - } - if (rc.ExtraPath != nil) && (len(rc.ExtraPath) > 0) { - s := append(rc.ExtraPath, `/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin`) - env["PATH"] = strings.Join(s, `:`) - } + var env map[string]string + c := job.Container() + if c != nil { + env = mergeMaps(rc.GetEnv(), c.Env, step.GetEnv()) + } else { + env = mergeMaps(rc.GetEnv(), step.GetEnv()) + } - for k, v := range env { - env[k] = rc.ExprEval.Interpolate(v) - } - sc.Env = rc.withGithubEnv(env) - log.Debugf("setupEnv => %v", sc.Env) - return nil + if (rc.ExtraPath != nil) && (len(rc.ExtraPath) > 0) { + s := append(rc.ExtraPath, `/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin`) + env["PATH"] = strings.Join(s, `:`) + } + + sc.Env = rc.withGithubEnv(env) + return env +} + + +func (sc *StepContext) interpolateEnv(exprEval ExpressionEvaluator) { + for k, v := range sc.Env { + sc.Env[k] = exprEval.Interpolate(v) } } + +func (sc *StepContext) setupEnv(ctx context.Context) (ExpressionEvaluator, error) { + rc := sc.RunContext + sc.Env = sc.mergeEnv() + if sc.Env != nil { + err := rc.JobContainer.UpdateFromGithubEnv(&sc.Env)(ctx) + if err != nil { + return nil, err + } + } + evaluator := sc.NewExpressionEvaluator() + sc.interpolateEnv(evaluator) + + log.Debugf("setupEnv: %v", sc.Env) + return evaluator, nil +} + + func (sc *StepContext) setupShellCommand() common.Executor { rc := sc.RunContext step := sc.Step