From 943a0e6eea2f67783018b9d3bc375a6a7dd65ab3 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Tue, 24 May 2022 15:36:06 +0200 Subject: [PATCH] implement pre and post steps (#1089) * feat: add post step to actions and add state command This commit includes requried changes for running post steps for local and remote actions. This allows general cleanup work to be done after executing an action. Communication is allowed between this steps, by using the action state. * feat: collect pre and post steps for composite actions * refactor: move composite action logic into own file * refactor: restructure composite handling * feat: run composite post steps during post step lifecycle * refactor: remove duplicate log output * feat: run all composite post actions in a step Since composite actions could have multiple pre/post steps inside, we need to run all of them in a single top-level pre/post step. This PR includes a test case for this and the correct order of steps to be executed. * refactor: remove unused lines of code * refactor: simplify test expression * fix: use composite job logger * fix: make step output more readable * fix: enforce running all post executor To make sure every post executor/step is executed, it is chained with it's own Finally executor. * fix: do not run post step if no step result is available Having no step result means we do not run any step (neither pre nor main) and we do not need to run post. * fix: setup defaults If no pre-if or post-if is given, it should default to 'always()'. This could be set even if there is no pre or post step. In fact this is required for composite actions and included post steps to run. * fix: output step related if expression * test: update expectation * feat: run pre step from actions (#1110) This PR implements running pre steps for remote actions. This includes remote actions using inside local composite actions. * fix: set correct expr default status checks For post-if conditions the default status check should be always(), while for all other if expression the default status check is success() References: https://docs.github.com/en/actions/learn-github-actions/expressions#status-check-functions https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runspost-if * fix: remove code added during rebase --- pkg/exprparser/functions_test.go | 16 +- pkg/exprparser/interpreter.go | 36 +- pkg/exprparser/interpreter_test.go | 10 +- pkg/model/action.go | 12 + pkg/model/step_result.go | 1 + pkg/runner/action.go | 356 +++++++------ pkg/runner/action_composite.go | 195 +++++++ pkg/runner/action_test.go | 70 ++- pkg/runner/command.go | 15 + pkg/runner/command_test.go | 18 + pkg/runner/expression.go | 14 +- pkg/runner/expression_test.go | 5 +- pkg/runner/job_executor.go | 17 +- pkg/runner/run_context.go | 3 +- pkg/runner/run_context_test.go | 3 +- pkg/runner/runner_test.go | 1 + pkg/runner/step.go | 63 ++- pkg/runner/step_action_local.go | 67 ++- pkg/runner/step_action_local_test.go | 242 ++++++++- pkg/runner/step_action_remote.go | 160 ++++-- pkg/runner/step_action_remote_test.go | 480 +++++++++++++++--- pkg/runner/step_docker.go | 6 +- pkg/runner/step_run.go | 6 +- pkg/runner/step_test.go | 18 +- .../last-action/action.yml | 7 + .../last-action/main.js | 0 .../last-action/post.js | 17 + .../push.yml | 15 + .../action-with-pre-and-post/action.yml | 13 + .../action-with-pre-and-post/main.js | 3 + .../action-with-pre-and-post/post.js | 3 + .../action-with-pre-and-post/pre.js | 1 + .../composite_action/action.yml | 12 + .../last-action/action.yml | 7 + .../last-action/main.js | 0 .../last-action/post.js | 7 + .../push.yml | 11 + 37 files changed, 1560 insertions(+), 350 deletions(-) create mode 100644 pkg/runner/action_composite.go create mode 100644 pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/action.yml create mode 100644 pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/main.js create mode 100644 pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/post.js create mode 100644 pkg/runner/testdata/uses-action-with-pre-and-post-step/push.yml create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/action.yml create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/main.js create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/post.js create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/pre.js create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/composite_action/action.yml create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/action.yml create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/main.js create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/post.js create mode 100644 pkg/runner/testdata/uses-composite-with-pre-and-post-steps/push.yml diff --git a/pkg/exprparser/functions_test.go b/pkg/exprparser/functions_test.go index 3c3d3477..009e9113 100644 --- a/pkg/exprparser/functions_test.go +++ b/pkg/exprparser/functions_test.go @@ -37,7 +37,7 @@ func TestFunctionContains(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -66,7 +66,7 @@ func TestFunctionStartsWith(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -95,7 +95,7 @@ func TestFunctionEndsWith(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -122,7 +122,7 @@ func TestFunctionJoin(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -148,7 +148,7 @@ func TestFunctionToJSON(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -171,7 +171,7 @@ func TestFunctionFromJSON(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -197,7 +197,7 @@ func TestFunctionHashFiles(t *testing.T) { t.Run(tt.name, func(t *testing.T) { workdir, err := filepath.Abs("testdata") assert.Nil(t, err) - output, err := NewInterpeter(env, Config{WorkingDir: workdir}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{WorkingDir: workdir}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -234,7 +234,7 @@ func TestFunctionFormat(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) if tt.error != nil { assert.Equal(t, tt.error, err.Error()) } else { diff --git a/pkg/exprparser/interpreter.go b/pkg/exprparser/interpreter.go index a9f8fafb..6388b55c 100644 --- a/pkg/exprparser/interpreter.go +++ b/pkg/exprparser/interpreter.go @@ -30,8 +30,32 @@ type Config struct { Context string } +type DefaultStatusCheck int + +const ( + DefaultStatusCheckNone DefaultStatusCheck = iota + DefaultStatusCheckSuccess + DefaultStatusCheckAlways + DefaultStatusCheckCanceled + DefaultStatusCheckFailure +) + +func (dsc DefaultStatusCheck) String() string { + switch dsc { + case DefaultStatusCheckSuccess: + return "success" + case DefaultStatusCheckAlways: + return "always" + case DefaultStatusCheckCanceled: + return "cancelled" + case DefaultStatusCheckFailure: + return "failure" + } + return "" +} + type Interpreter interface { - Evaluate(input string, isIfExpression bool) (interface{}, error) + Evaluate(input string, defaultStatusCheck DefaultStatusCheck) (interface{}, error) } type interperterImpl struct { @@ -46,9 +70,9 @@ func NewInterpeter(env *EvaluationEnvironment, config Config) Interpreter { } } -func (impl *interperterImpl) Evaluate(input string, isIfExpression bool) (interface{}, error) { +func (impl *interperterImpl) Evaluate(input string, defaultStatusCheck DefaultStatusCheck) (interface{}, error) { input = strings.TrimPrefix(input, "${{") - if isIfExpression && input == "" { + if defaultStatusCheck != DefaultStatusCheckNone && input == "" { input = "success()" } parser := actionlint.NewExprParser() @@ -57,7 +81,7 @@ func (impl *interperterImpl) Evaluate(input string, isIfExpression bool) (interf return nil, fmt.Errorf("Failed to parse: %s", err.Message) } - if isIfExpression { + if defaultStatusCheck != DefaultStatusCheckNone { hasStatusCheckFunction := false actionlint.VisitExprNode(exprNode, func(node, _ actionlint.ExprNode, entering bool) { if funcCallNode, ok := node.(*actionlint.FuncCallNode); entering && ok { @@ -72,7 +96,7 @@ func (impl *interperterImpl) Evaluate(input string, isIfExpression bool) (interf exprNode = &actionlint.LogicalOpNode{ Kind: actionlint.LogicalOpNodeKindAnd, Left: &actionlint.FuncCallNode{ - Callee: "success", + Callee: defaultStatusCheck.String(), Args: []actionlint.ExprNode{}, }, Right: exprNode, @@ -361,7 +385,7 @@ func (impl *interperterImpl) coerceToNumber(value reflect.Value) reflect.Value { } // try to parse the string as a number - evaluated, err := impl.Evaluate(value.String(), false) + evaluated, err := impl.Evaluate(value.String(), DefaultStatusCheckNone) if err != nil { return reflect.ValueOf(math.NaN()) } diff --git a/pkg/exprparser/interpreter_test.go b/pkg/exprparser/interpreter_test.go index 4eae2532..2547aae5 100644 --- a/pkg/exprparser/interpreter_test.go +++ b/pkg/exprparser/interpreter_test.go @@ -29,7 +29,7 @@ func TestLiterals(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -93,7 +93,7 @@ func TestOperators(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) if tt.error != "" { assert.NotNil(t, err) assert.Equal(t, tt.error, err.Error()) @@ -146,7 +146,7 @@ func TestOperatorsCompare(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) @@ -509,7 +509,7 @@ func TestOperatorsBooleanEvaluation(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) if expected, ok := tt.expected.(float64); ok && math.IsNaN(expected) { @@ -607,7 +607,7 @@ func TestContexts(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, false) + output, err := NewInterpeter(env, Config{}).Evaluate(tt.input, DefaultStatusCheckNone) assert.Nil(t, err) assert.Equal(t, tt.expected, output) diff --git a/pkg/model/action.go b/pkg/model/action.go index 1cfce632..9a69e642 100644 --- a/pkg/model/action.go +++ b/pkg/model/action.go @@ -49,6 +49,10 @@ type ActionRuns struct { Using ActionRunsUsing `yaml:"using"` Env map[string]string `yaml:"env"` Main string `yaml:"main"` + Pre string `yaml:"pre"` + PreIf string `yaml:"pre-if"` + Post string `yaml:"post"` + PostIf string `yaml:"post-if"` Image string `yaml:"image"` Entrypoint string `yaml:"entrypoint"` Args []string `yaml:"args"` @@ -90,5 +94,13 @@ func ReadAction(in io.Reader) (*Action, error) { return nil, err } + // set defaults + if a.Runs.PreIf == "" { + a.Runs.PreIf = "always()" + } + if a.Runs.PostIf == "" { + a.Runs.PostIf = "always()" + } + return a, nil } diff --git a/pkg/model/step_result.go b/pkg/model/step_result.go index 86e5ebf3..49b7705d 100644 --- a/pkg/model/step_result.go +++ b/pkg/model/step_result.go @@ -42,4 +42,5 @@ type StepResult struct { Outputs map[string]string `json:"outputs"` Conclusion stepStatus `json:"conclusion"` Outcome stepStatus `json:"outcome"` + State map[string]string } diff --git a/pkg/runner/action.go b/pkg/runner/action.go index aeac196e..fca55aa6 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -24,6 +24,8 @@ type actionStep interface { step getActionModel() *model.Action + getCompositeRunContext() *RunContext + getCompositeSteps() *compositeSteps } type readAction func(step *model.Step, actionDir string, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) @@ -98,14 +100,37 @@ func readActionImpl(step *model.Step, actionDir string, actionPath string, readF return action, err } +func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir string, actionPath string, containerActionDir string) error { + rc := step.getRunContext() + stepModel := step.getStepModel() + + if stepModel.Type() != model.StepTypeUsesActionRemote { + return nil + } + if err := removeGitIgnore(actionDir); err != nil { + return err + } + + var containerActionDirCopy string + containerActionDirCopy = strings.TrimSuffix(containerActionDir, actionPath) + log.Debug(containerActionDirCopy) + + if !strings.HasSuffix(containerActionDirCopy, `/`) { + containerActionDirCopy += `/` + } + return rc.JobContainer.CopyDir(containerActionDirCopy, actionDir+"/", rc.Config.UseGitIgnore)(ctx) +} + func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction) common.Executor { rc := step.getRunContext() stepModel := step.getStepModel() + return func(ctx context.Context) error { actionPath := "" if remoteAction != nil && remoteAction.Path != "" { actionPath = remoteAction.Path } + action := step.getActionModel() log.Debugf("About to run action %v", action) @@ -127,6 +152,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction // time, we don't have all environment prepared mergeIntoMap(step.getEnv(), rc.withGithubEnv(map[string]string{})) + populateEnvsFromSavedState(step.getEnv(), step, rc) populateEnvsFromInput(step.getEnv(), action, rc) actionLocation := path.Join(actionDir, actionPath) @@ -134,27 +160,9 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction log.Debugf("type=%v actionDir=%s actionPath=%s workdir=%s actionCacheDir=%s actionName=%s containerActionDir=%s", stepModel.Type(), actionDir, actionPath, rc.Config.Workdir, rc.ActionCacheDir(), actionName, containerActionDir) - maybeCopyToActionDir := func() error { - if stepModel.Type() != model.StepTypeUsesActionRemote { - return nil - } - if err := removeGitIgnore(actionDir); err != nil { - return err - } - - var containerActionDirCopy string - containerActionDirCopy = strings.TrimSuffix(containerActionDir, actionPath) - log.Debug(containerActionDirCopy) - - if !strings.HasSuffix(containerActionDirCopy, `/`) { - containerActionDirCopy += `/` - } - return rc.JobContainer.CopyDir(containerActionDirCopy, actionDir+"/", rc.Config.UseGitIgnore)(ctx) - } - switch action.Runs.Using { case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16: - if err := maybeCopyToActionDir(); err != nil { + if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { return err } containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Main)} @@ -167,10 +175,11 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction } return execAsDocker(ctx, step, actionName, location, remoteAction == nil) case model.ActionRunsUsingComposite: - if err := maybeCopyToActionDir(); err != nil { + if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { return err } - return execAsComposite(step, containerActionDir)(ctx) + + return execAsComposite(step)(ctx) default: return fmt.Errorf(fmt.Sprintf("The runs.using key must be one of: %v, got %s", []string{ model.ActionRunsUsingDocker, @@ -359,134 +368,36 @@ func newStepContainer(ctx context.Context, step step, image string, cmd []string return stepContainer } -func execAsComposite(step actionStep, containerActionDir string) common.Executor { - rc := step.getRunContext() +func (rc *RunContext) setupActionInputs(step actionStep) { + if step.getActionModel() == nil { + // e.g. local checkout skip has no action model + return + } + + stepModel := step.getStepModel() action := step.getActionModel() - return func(ctx context.Context) error { - eval := rc.NewExpressionEvaluator() - - inputs := make(map[string]interface{}) - for k, input := range action.Inputs { - inputs[k] = eval.Interpolate(input.Default) - } - if step.getStepModel().With != nil { - for k, v := range step.getStepModel().With { - inputs[k] = eval.Interpolate(v) - } - } - - env := make(map[string]string) - for k, v := range rc.Env { - env[k] = eval.Interpolate(v) - } - for k, v := range step.getStepModel().Environment() { - env[k] = eval.Interpolate(v) - } - - // run with the global config but without secrets - configCopy := *rc.Config - configCopy.Secrets = nil - - // create a run context for the composite action to run in - compositerc := &RunContext{ - Name: rc.Name, - JobName: rc.JobName, - Run: &model.Run{ - JobID: "composite-job", - Workflow: &model.Workflow{ - Name: rc.Run.Workflow.Name, - Jobs: map[string]*model.Job{ - "composite-job": {}, - }, - }, - }, - Config: &configCopy, - StepResults: map[string]*model.StepResult{}, - JobContainer: rc.JobContainer, - Inputs: inputs, - ActionPath: containerActionDir, - ActionRepository: rc.ActionRepository, - ActionRef: rc.ActionRef, - Env: env, - Masks: rc.Masks, - ExtraPath: rc.ExtraPath, - } - - ctx = WithCompositeLogger(ctx, &compositerc.Masks) - - // We need to inject a composite RunContext related command - // handler into the current running job container - // We need this, to support scoping commands to the composite action - // executing. - rawLogger := common.Logger(ctx).WithField("raw_output", true) - logWriter := common.NewLineWriter(compositerc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) - oldout, olderr := compositerc.JobContainer.ReplaceLogWriter(logWriter, logWriter) - defer (func() { - rc.JobContainer.ReplaceLogWriter(oldout, olderr) - })() - - err := compositerc.compositeExecutor(action)(ctx) - - // Map outputs from composite RunContext to job RunContext - eval = compositerc.NewExpressionEvaluator() - for outputName, output := range action.Outputs { - rc.setOutput(ctx, map[string]string{ - "name": outputName, - }, eval.Interpolate(output.Value)) - } - - rc.Masks = compositerc.Masks - rc.ExtraPath = compositerc.ExtraPath - - return err + eval := rc.NewExpressionEvaluator() + inputs := make(map[string]interface{}) + for k, input := range action.Inputs { + inputs[k] = eval.Interpolate(input.Default) } + if stepModel.With != nil { + for k, v := range stepModel.With { + inputs[k] = eval.Interpolate(v) + } + } + + rc.Inputs = inputs } -// Executor returns a pipeline executor for all the steps in the job -func (rc *RunContext) compositeExecutor(action *model.Action) common.Executor { - steps := make([]common.Executor, 0) - - sf := &stepFactoryImpl{} - - for i, step := range action.Runs.Steps { - if step.ID == "" { - step.ID = fmt.Sprintf("%d", i) +func populateEnvsFromSavedState(env *map[string]string, step actionStep, rc *RunContext) { + stepResult := rc.StepResults[step.getStepModel().ID] + if stepResult != nil { + for name, value := range stepResult.State { + envName := fmt.Sprintf("STATE_%s", name) + (*env)[envName] = value } - - // create a copy of the step, since this composite action could - // run multiple times and we might modify the instance - stepcopy := step - - step, err := sf.newStep(&stepcopy, rc) - if err != nil { - return common.NewErrorExecutor(err) - } - stepExec := common.NewPipelineExecutor(step.pre(), step.main(), step.post()) - - steps = append(steps, func(ctx context.Context) error { - err := stepExec(ctx) - if err != nil { - common.Logger(ctx).Errorf("%v", err) - common.SetJobError(ctx, err) - } else if ctx.Err() != nil { - common.Logger(ctx).Errorf("%v", ctx.Err()) - common.SetJobError(ctx, ctx.Err()) - } - return nil - }) - } - - steps = append(steps, common.JobError) - return func(ctx context.Context) error { - return common.NewPipelineExecutor(steps...)(common.WithJobErrorContainer(ctx)) } } @@ -531,3 +442,162 @@ func getOsSafeRelativePath(s, prefix string) string { return actionName } + +func shouldRunPreStep(step actionStep) common.Conditional { + return func(ctx context.Context) bool { + log := common.Logger(ctx) + + if step.getActionModel() == nil { + log.Debugf("skip pre step for '%s': no action model available", step.getStepModel()) + return false + } + + return true + } +} + +func hasPreStep(step actionStep) common.Conditional { + return func(ctx context.Context) bool { + action := step.getActionModel() + return action.Runs.Using == model.ActionRunsUsingComposite || + ((action.Runs.Using == model.ActionRunsUsingNode12 || + action.Runs.Using == model.ActionRunsUsingNode16) && + action.Runs.Pre != "") + } +} + +func runPreStep(step actionStep) common.Executor { + return func(ctx context.Context) error { + common.Logger(ctx).Debugf("run pre step for '%s'", step.getStepModel()) + + rc := step.getRunContext() + stepModel := step.getStepModel() + action := step.getActionModel() + + switch action.Runs.Using { + case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16: + // todo: refactor into step + var actionDir string + var actionPath string + if _, ok := step.(*stepActionRemote); ok { + actionPath = newRemoteAction(stepModel.Uses).Path + actionDir = fmt.Sprintf("%s/%s", rc.ActionCacheDir(), strings.ReplaceAll(stepModel.Uses, "/", "-")) + } else { + actionDir = filepath.Join(rc.Config.Workdir, stepModel.Uses) + actionPath = "" + } + + actionLocation := "" + if actionPath != "" { + actionLocation = path.Join(actionDir, actionPath) + } else { + actionLocation = actionDir + } + + _, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) + + if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { + return err + } + + containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Pre)} + log.Debugf("executing remote job container: %s", containerArgs) + + return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) + + case model.ActionRunsUsingComposite: + step.getCompositeRunContext().updateCompositeRunContext(step.getRunContext(), step) + return step.getCompositeSteps().pre(ctx) + + default: + return nil + } + } +} + +func shouldRunPostStep(step actionStep) common.Conditional { + return func(ctx context.Context) bool { + log := common.Logger(ctx) + stepResults := step.getRunContext().getStepsContext() + stepResult := stepResults[step.getStepModel().ID] + + if stepResult == nil { + log.Debugf("skip post step for '%s'; step was not executed", step.getStepModel()) + return false + } + + if stepResult.Conclusion == model.StepStatusSkipped { + log.Debugf("skip post step for '%s'; main step was skipped", step.getStepModel()) + return false + } + + if step.getActionModel() == nil { + log.Debugf("skip post step for '%s': no action model available", step.getStepModel()) + return false + } + + return true + } +} + +func hasPostStep(step actionStep) common.Conditional { + return func(ctx context.Context) bool { + action := step.getActionModel() + return action.Runs.Using == model.ActionRunsUsingComposite || + ((action.Runs.Using == model.ActionRunsUsingNode12 || + action.Runs.Using == model.ActionRunsUsingNode16) && + action.Runs.Post != "") + } +} + +func runPostStep(step actionStep) common.Executor { + return func(ctx context.Context) error { + common.Logger(ctx).Debugf("run post step for '%s'", step.getStepModel()) + + rc := step.getRunContext() + stepModel := step.getStepModel() + action := step.getActionModel() + + // todo: refactor into step + var actionDir string + var actionPath string + if _, ok := step.(*stepActionRemote); ok { + actionPath = newRemoteAction(stepModel.Uses).Path + actionDir = fmt.Sprintf("%s/%s", rc.ActionCacheDir(), strings.ReplaceAll(stepModel.Uses, "/", "-")) + } else { + actionDir = filepath.Join(rc.Config.Workdir, stepModel.Uses) + actionPath = "" + } + + actionLocation := "" + if actionPath != "" { + actionLocation = path.Join(actionDir, actionPath) + } else { + actionLocation = actionDir + } + + _, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) + + switch action.Runs.Using { + case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16: + + populateEnvsFromSavedState(step.getEnv(), step, rc) + + containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)} + log.Debugf("executing remote job container: %s", containerArgs) + + return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) + + case model.ActionRunsUsingComposite: + if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { + return err + } + + step.getCompositeRunContext().updateCompositeRunContext(step.getRunContext(), step) + return step.getCompositeSteps().post(ctx) + + default: + return nil + } + } +} diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go new file mode 100644 index 00000000..924bc7b5 --- /dev/null +++ b/pkg/runner/action_composite.go @@ -0,0 +1,195 @@ +package runner + +import ( + "context" + "fmt" + + "github.com/nektos/act/pkg/common" + "github.com/nektos/act/pkg/model" +) + +func evaluteCompositeInputAndEnv(parent *RunContext, step actionStep) (inputs map[string]interface{}, env map[string]string) { + eval := parent.NewExpressionEvaluator() + + inputs = make(map[string]interface{}) + for k, input := range step.getActionModel().Inputs { + inputs[k] = eval.Interpolate(input.Default) + } + if step.getStepModel().With != nil { + for k, v := range step.getStepModel().With { + inputs[k] = eval.Interpolate(v) + } + } + + env = make(map[string]string) + for k, v := range parent.Env { + env[k] = eval.Interpolate(v) + } + for k, v := range step.getStepModel().Environment() { + env[k] = eval.Interpolate(v) + } + + return inputs, env +} + +func newCompositeRunContext(parent *RunContext, step actionStep, actionPath string) *RunContext { + inputs, env := evaluteCompositeInputAndEnv(parent, step) + + // run with the global config but without secrets + configCopy := *(parent.Config) + configCopy.Secrets = nil + + // create a run context for the composite action to run in + compositerc := &RunContext{ + Name: parent.Name, + JobName: parent.JobName, + Run: &model.Run{ + JobID: "composite-job", + Workflow: &model.Workflow{ + Name: parent.Run.Workflow.Name, + Jobs: map[string]*model.Job{ + "composite-job": {}, + }, + }, + }, + Config: &configCopy, + StepResults: map[string]*model.StepResult{}, + JobContainer: parent.JobContainer, + Inputs: inputs, + ActionPath: actionPath, + ActionRepository: parent.ActionRepository, + ActionRef: parent.ActionRef, + Env: env, + Masks: parent.Masks, + ExtraPath: parent.ExtraPath, + } + + return compositerc +} + +// This updates a composite context inputs, env and masks. +// This is needed to re-evalute/update that context between pre/main/post steps. +// Some of the inputs/env may requires the results of in-between steps. +func (rc *RunContext) updateCompositeRunContext(parent *RunContext, step actionStep) { + inputs, env := evaluteCompositeInputAndEnv(parent, step) + + rc.Inputs = inputs + rc.Env = env + rc.Masks = append(rc.Masks, parent.Masks...) +} + +func execAsComposite(step actionStep) common.Executor { + rc := step.getRunContext() + action := step.getActionModel() + + return func(ctx context.Context) error { + compositerc := step.getCompositeRunContext() + + steps := step.getCompositeSteps() + + ctx = WithCompositeLogger(ctx, &compositerc.Masks) + + compositerc.updateCompositeRunContext(rc, step) + err := steps.main(ctx) + + // Map outputs from composite RunContext to job RunContext + eval := compositerc.NewExpressionEvaluator() + for outputName, output := range action.Outputs { + rc.setOutput(ctx, map[string]string{ + "name": outputName, + }, eval.Interpolate(output.Value)) + } + + rc.Masks = append(rc.Masks, compositerc.Masks...) + rc.ExtraPath = compositerc.ExtraPath + + return err + } +} + +type compositeSteps struct { + pre common.Executor + main common.Executor + post common.Executor +} + +// Executor returns a pipeline executor for all the steps in the job +func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps { + steps := make([]common.Executor, 0) + preSteps := make([]common.Executor, 0) + var postExecutor common.Executor + + sf := &stepFactoryImpl{} + + for i, step := range action.Runs.Steps { + if step.ID == "" { + step.ID = fmt.Sprintf("%d", i) + } + + // create a copy of the step, since this composite action could + // run multiple times and we might modify the instance + stepcopy := step + + step, err := sf.newStep(&stepcopy, rc) + if err != nil { + return &compositeSteps{ + main: common.NewErrorExecutor(err), + } + } + + preSteps = append(preSteps, step.pre()) + + steps = append(steps, func(ctx context.Context) error { + err := step.main()(ctx) + if err != nil { + common.Logger(ctx).Errorf("%v", err) + common.SetJobError(ctx, err) + } else if ctx.Err() != nil { + common.Logger(ctx).Errorf("%v", ctx.Err()) + common.SetJobError(ctx, ctx.Err()) + } + return nil + }) + + // run the post executor in reverse order + if postExecutor != nil { + postExecutor = step.post().Finally(postExecutor) + } else { + postExecutor = step.post() + } + } + + steps = append(steps, common.JobError) + return &compositeSteps{ + pre: rc.newCompositeCommandExecutor(common.NewPipelineExecutor(preSteps...)), + main: rc.newCompositeCommandExecutor(func(ctx context.Context) error { + return common.NewPipelineExecutor(steps...)(common.WithJobErrorContainer(ctx)) + }), + post: rc.newCompositeCommandExecutor(postExecutor), + } +} + +func (rc *RunContext) newCompositeCommandExecutor(executor common.Executor) common.Executor { + return func(ctx context.Context) error { + ctx = WithCompositeLogger(ctx, &rc.Masks) + + // We need to inject a composite RunContext related command + // handler into the current running job container + // We need this, to support scoping commands to the composite action + // executing. + rawLogger := common.Logger(ctx).WithField("raw_output", true) + logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { + if rc.Config.LogOutput { + rawLogger.Infof("%s", s) + } else { + rawLogger.Debugf("%s", s) + } + return true + }) + + oldout, olderr := rc.JobContainer.ReplaceLogWriter(logWriter, logWriter) + defer rc.JobContainer.ReplaceLogWriter(oldout, olderr) + + return executor(ctx) + } +} diff --git a/pkg/runner/action_test.go b/pkg/runner/action_test.go index 7e1c1297..14a51021 100644 --- a/pkg/runner/action_test.go +++ b/pkg/runner/action_test.go @@ -44,8 +44,10 @@ runs: expected: &model.Action{ Name: "name", Runs: model.ActionRuns{ - Using: "node16", - Main: "main.js", + Using: "node16", + Main: "main.js", + PreIf: "always()", + PostIf: "always()", }, }, }, @@ -57,8 +59,10 @@ runs: expected: &model.Action{ Name: "name", Runs: model.ActionRuns{ - Using: "node16", - Main: "main.js", + Using: "node16", + Main: "main.js", + PreIf: "always()", + PostIf: "always()", }, }, }, @@ -138,14 +142,15 @@ runs: func TestActionRunner(t *testing.T) { table := []struct { - name string - step actionStep + name string + step actionStep + expectedEnv map[string]string }{ { - name: "Test", + name: "with-input", step: &stepActionRemote{ Step: &model.Step{ - Uses: "repo@ref", + Uses: "org/repo/path@ref", }, RunContext: &RunContext{ Config: &Config{}, @@ -172,6 +177,47 @@ func TestActionRunner(t *testing.T) { }, env: map[string]string{}, }, + expectedEnv: map[string]string{"INPUT_KEY": "default value"}, + }, + { + name: "restore-saved-state", + step: &stepActionRemote{ + Step: &model.Step{ + ID: "step", + Uses: "org/repo/path@ref", + }, + RunContext: &RunContext{ + ActionRepository: "org/repo", + ActionPath: "path", + ActionRef: "ref", + Config: &Config{}, + Run: &model.Run{ + JobID: "job", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "job": { + Name: "job", + }, + }, + }, + }, + CurrentStep: "post-step", + StepResults: map[string]*model.StepResult{ + "step": { + State: map[string]string{ + "name": "state value", + }, + }, + }, + }, + action: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + }, + }, + env: map[string]string{}, + }, + expectedEnv: map[string]string{"STATE_name": "state value"}, }, } @@ -183,8 +229,14 @@ func TestActionRunner(t *testing.T) { cm.On("CopyDir", "/var/run/act/actions/dir/", "dir/", false).Return(func(ctx context.Context) error { return nil }) envMatcher := mock.MatchedBy(func(env map[string]string) bool { - return env["INPUT_KEY"] == "default value" + for k, v := range tt.expectedEnv { + if env[k] != v { + return false + } + } + return true }) + cm.On("Exec", []string{"node", "/var/run/act/actions/dir/path"}, envMatcher, "", "").Return(func(ctx context.Context) error { return nil }) tt.step.getRunContext().JobContainer = cm diff --git a/pkg/runner/command.go b/pkg/runner/command.go index 3182f83a..ce77bdf9 100755 --- a/pkg/runner/command.go +++ b/pkg/runner/command.go @@ -63,6 +63,9 @@ func (rc *RunContext) commandHandler(ctx context.Context) common.LineHandler { case resumeCommand: resumeCommand = "" logger.Infof(" \U00002699 %s", line) + case "save-state": + logger.Infof(" \U0001f4be %s", line) + rc.saveState(ctx, kvPairs, arg) default: logger.Infof(" \U00002753 %s", line) } @@ -141,3 +144,15 @@ func unescapeKvPairs(kvPairs map[string]string) map[string]string { } return kvPairs } + +func (rc *RunContext) saveState(ctx context.Context, kvPairs map[string]string, arg string) { + if rc.CurrentStep != "" { + stepResult := rc.StepResults[rc.CurrentStep] + if stepResult != nil { + if stepResult.State == nil { + stepResult.State = map[string]string{} + } + stepResult.State[kvPairs["name"]] = arg + } + } +} diff --git a/pkg/runner/command_test.go b/pkg/runner/command_test.go index bd7bbdc7..1c72768e 100644 --- a/pkg/runner/command_test.go +++ b/pkg/runner/command_test.go @@ -173,3 +173,21 @@ func TestAddmaskUsemask(t *testing.T) { a.Equal("[testjob] \U00002699 ***\n[testjob] \U00002699 ::set-output:: = token=***\n", re) } + +func TestSaveState(t *testing.T) { + rc := &RunContext{ + CurrentStep: "step", + StepResults: map[string]*model.StepResult{ + "step": { + State: map[string]string{}, + }, + }, + } + + ctx := context.Background() + + handler := rc.commandHandler(ctx) + handler("::save-state name=state-name::state-value\n") + + assert.Equal(t, "state-value", rc.StepResults["step"].State["state-name"]) +} diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index 5ff311d6..ad03c212 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -12,7 +12,7 @@ import ( // ExpressionEvaluator is the interface for evaluating expressions type ExpressionEvaluator interface { - evaluate(string, bool) (interface{}, error) + evaluate(string, exprparser.DefaultStatusCheck) (interface{}, error) EvaluateYamlNode(node *yaml.Node) error Interpolate(string) string } @@ -115,9 +115,9 @@ type expressionEvaluator struct { interpreter exprparser.Interpreter } -func (ee expressionEvaluator) evaluate(in string, isIfExpression bool) (interface{}, error) { +func (ee expressionEvaluator) evaluate(in string, defaultStatusCheck exprparser.DefaultStatusCheck) (interface{}, error) { log.Debugf("evaluating expression '%s'", in) - evaluated, err := ee.interpreter.Evaluate(in, isIfExpression) + evaluated, err := ee.interpreter.Evaluate(in, defaultStatusCheck) log.Debugf("expression '%s' evaluated to '%t'", in, evaluated) return evaluated, err } @@ -131,7 +131,7 @@ func (ee expressionEvaluator) evaluateScalarYamlNode(node *yaml.Node) error { return nil } expr, _ := rewriteSubExpression(in, false) - res, err := ee.evaluate(expr, false) + res, err := ee.evaluate(expr, exprparser.DefaultStatusCheckNone) if err != nil { return err } @@ -201,7 +201,7 @@ func (ee expressionEvaluator) Interpolate(in string) string { } expr, _ := rewriteSubExpression(in, true) - evaluated, err := ee.evaluate(expr, false) + evaluated, err := ee.evaluate(expr, exprparser.DefaultStatusCheckNone) if err != nil { log.Errorf("Unable to interpolate expression '%s': %s", expr, err) return "" @@ -216,10 +216,10 @@ func (ee expressionEvaluator) Interpolate(in string) string { } // EvalBool evaluates an expression against given evaluator -func EvalBool(evaluator ExpressionEvaluator, expr string) (bool, error) { +func EvalBool(evaluator ExpressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) { nextExpr, _ := rewriteSubExpression(expr, false) - evaluated, err := evaluator.evaluate(nextExpr, true) + evaluated, err := evaluator.evaluate(nextExpr, defaultStatusCheck) if err != nil { return false, err } diff --git a/pkg/runner/expression_test.go b/pkg/runner/expression_test.go index 944070c3..f6a23015 100644 --- a/pkg/runner/expression_test.go +++ b/pkg/runner/expression_test.go @@ -7,6 +7,7 @@ import ( "sort" "testing" + "github.com/nektos/act/pkg/exprparser" "github.com/nektos/act/pkg/model" assert "github.com/stretchr/testify/assert" yaml "gopkg.in/yaml.v3" @@ -135,7 +136,7 @@ func TestEvaluateRunContext(t *testing.T) { table := table t.Run(table.in, func(t *testing.T) { assertObject := assert.New(t) - out, err := ee.evaluate(table.in, false) + out, err := ee.evaluate(table.in, exprparser.DefaultStatusCheckNone) if table.errMesg == "" { assertObject.NoError(err, table.in) assertObject.Equal(table.out, out, table.in) @@ -175,7 +176,7 @@ func TestEvaluateStep(t *testing.T) { table := table t.Run(table.in, func(t *testing.T) { assertObject := assert.New(t) - out, err := ee.evaluate(table.in, false) + out, err := ee.evaluate(table.in, exprparser.DefaultStatusCheckNone) if table.errMesg == "" { assertObject.NoError(err, table.in) assertObject.Equal(table.out, out, table.in) diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index e128aaa7..6b56867c 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -21,7 +21,7 @@ type jobInfo interface { func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executor { steps := make([]common.Executor, 0) preSteps := make([]common.Executor, 0) - postSteps := make([]common.Executor, 0) + var postExecutor common.Executor steps = append(steps, func(ctx context.Context) error { if len(info.matrix()) > 0 { @@ -72,10 +72,15 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo })(withStepLogger(ctx, stepName)) }) - postSteps = append([]common.Executor{step.post()}, postSteps...) + // run the post exector in reverse order + if postExecutor != nil { + postExecutor = step.post().Finally(postExecutor) + } else { + postExecutor = step.post() + } } - postSteps = append(postSteps, func(ctx context.Context) error { + postExecutor = postExecutor.Finally(func(ctx context.Context) error { jobError := common.JobError(ctx) if jobError != nil { info.result("failure") @@ -93,7 +98,9 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo pipeline := make([]common.Executor, 0) pipeline = append(pipeline, preSteps...) pipeline = append(pipeline, steps...) - pipeline = append(pipeline, postSteps...) - return common.NewPipelineExecutor(pipeline...).Finally(info.interpolateOutputs()).Finally(info.closeContainer()) + return common.NewPipelineExecutor(pipeline...). + Finally(postExecutor). + Finally(info.interpolateOutputs()). + Finally(info.closeContainer()) } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 66c0d104..001f97ac 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -20,6 +20,7 @@ import ( "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/container" + "github.com/nektos/act/pkg/exprparser" "github.com/nektos/act/pkg/model" ) @@ -346,7 +347,7 @@ func (rc *RunContext) hostname() string { func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { job := rc.Run.Job() l := common.Logger(ctx) - runJob, err := EvalBool(rc.ExprEval, job.If.Value) + runJob, err := EvalBool(rc.ExprEval, job.If.Value, exprparser.DefaultStatusCheckSuccess) if err != nil { return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, err) } diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index 401a21ab..ebd2f678 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/nektos/act/pkg/exprparser" "github.com/nektos/act/pkg/model" log "github.com/sirupsen/logrus" @@ -155,7 +156,7 @@ func TestRunContext_EvalBool(t *testing.T) { table := table t.Run(table.in, func(t *testing.T) { assertObject := assert.New(t) - b, err := EvalBool(rc.ExprEval, table.in) + b, err := EvalBool(rc.ExprEval, table.in, exprparser.DefaultStatusCheckSuccess) if table.wantErr { assertObject.Error(err) } diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index fa27995e..224d1012 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -173,6 +173,7 @@ func TestRunEvent(t *testing.T) { {workdir, "job-status-check", "push", "job 'fail' failed", platforms}, {workdir, "if-expressions", "push", "Job 'mytest' failed", platforms}, {workdir, "actions-environment-and-context-tests", "push", "", platforms}, + {workdir, "uses-action-with-pre-and-post-step", "push", "", platforms}, {"../model/testdata", "strategy", "push", "", platforms}, // TODO: move all testdata into pkg so we can validate it with planner and runner // {"testdata", "issue-228", "push", "", platforms, }, // TODO [igni]: Remove this once everything passes {"../model/testdata", "container-volumes", "push", "", platforms}, diff --git a/pkg/runner/step.go b/pkg/runner/step.go index b6ed65a4..e4fb8703 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/nektos/act/pkg/common" + "github.com/nektos/act/pkg/exprparser" "github.com/nektos/act/pkg/model" log "github.com/sirupsen/logrus" ) @@ -18,14 +19,49 @@ type step interface { getRunContext() *RunContext getStepModel() *model.Step getEnv() *map[string]string + getIfExpression(stage stepStage) string } -func runStepExecutor(step step, executor common.Executor) common.Executor { +type stepStage int + +const ( + stepStagePre stepStage = iota + stepStageMain + stepStagePost +) + +func (s stepStage) String() string { + switch s { + case stepStagePre: + return "Pre" + case stepStageMain: + return "Main" + case stepStagePost: + return "Post" + } + return "Unknown" +} + +func (s stepStage) getStepName(stepModel *model.Step) string { + switch s { + case stepStagePre: + return fmt.Sprintf("pre-%s", stepModel.ID) + case stepStageMain: + return stepModel.ID + case stepStagePost: + return fmt.Sprintf("post-%s", stepModel.ID) + } + return "unknown" +} + +func runStepExecutor(step step, stage stepStage, executor common.Executor) common.Executor { return func(ctx context.Context) error { rc := step.getRunContext() stepModel := step.getStepModel() - rc.CurrentStep = stepModel.ID + ifExpression := step.getIfExpression(stage) + rc.CurrentStep = stage.getStepName(stepModel) + rc.StepResults[rc.CurrentStep] = &model.StepResult{ Outcome: model.StepStatusSuccess, Conclusion: model.StepStatusSuccess, @@ -37,7 +73,7 @@ func runStepExecutor(step step, executor common.Executor) common.Executor { return err } - runStep, err := isStepEnabled(ctx, step) + runStep, err := isStepEnabled(ctx, ifExpression, step, stage) if err != nil { rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusFailure rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusFailure @@ -45,7 +81,7 @@ func runStepExecutor(step step, executor common.Executor) common.Executor { } if !runStep { - log.Debugf("Skipping step '%s' due to '%s'", stepModel.String(), stepModel.If.Value) + log.Debugf("Skipping step '%s' due to '%s'", stepModel, ifExpression) rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusSkipped rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusSkipped return nil @@ -55,14 +91,14 @@ func runStepExecutor(step step, executor common.Executor) common.Executor { if strings.Contains(stepString, "::add-mask::") { stepString = "add-mask command" } - common.Logger(ctx).Infof("\u2B50 Run %s", stepString) + common.Logger(ctx).Infof("\u2B50 Run %s %s", stage, stepString) err = executor(ctx) if err == nil { - common.Logger(ctx).Infof(" \u2705 Success - %s", stepString) + common.Logger(ctx).Infof(" \u2705 Success - %s %s", stage, stepString) } else { - common.Logger(ctx).Errorf(" \u274C Failure - %s", stepString) + common.Logger(ctx).Errorf(" \u274C Failure - %s %s", stage, stepString) rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusFailure if stepModel.ContinueOnError { @@ -129,12 +165,19 @@ func mergeEnv(step step) { mergeIntoMap(env, rc.withGithubEnv(*env)) } -func isStepEnabled(ctx context.Context, step step) (bool, error) { +func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage) (bool, error) { rc := step.getRunContext() - runStep, err := EvalBool(rc.NewStepExpressionEvaluator(step), step.getStepModel().If.Value) + var defaultStatusCheck exprparser.DefaultStatusCheck + if stage == stepStagePost { + defaultStatusCheck = exprparser.DefaultStatusCheckAlways + } else { + defaultStatusCheck = exprparser.DefaultStatusCheckSuccess + } + + runStep, err := EvalBool(rc.NewStepExpressionEvaluator(step), expr, defaultStatusCheck) if err != nil { - return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", step.getStepModel().If.Value, err) + return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", expr, err) } return runStep, nil diff --git a/pkg/runner/step_action_local.go b/pkg/runner/step_action_local.go index de3c9cc3..be4303aa 100644 --- a/pkg/runner/step_action_local.go +++ b/pkg/runner/step_action_local.go @@ -11,28 +11,23 @@ import ( "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/model" - log "github.com/sirupsen/logrus" ) type stepActionLocal struct { - Step *model.Step - RunContext *RunContext - runAction runAction - readAction readAction - env map[string]string - action *model.Action + Step *model.Step + RunContext *RunContext + compositeRunContext *RunContext + compositeSteps *compositeSteps + runAction runAction + readAction readAction + env map[string]string + action *model.Action } func (sal *stepActionLocal) pre() common.Executor { - return func(ctx context.Context) error { - return nil - } -} - -func (sal *stepActionLocal) main() common.Executor { sal.env = map[string]string{} - return runStepExecutor(sal, func(ctx context.Context) error { + return func(ctx context.Context) error { actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses) localReader := func(ctx context.Context) actionYamlReader { @@ -56,16 +51,27 @@ func (sal *stepActionLocal) main() common.Executor { } sal.action = actionModel - log.Debugf("Read action %v from '%s'", sal.action, "Unknown") + // run local pre step only for composite actions, to allow to run + // inside pre steps + if sal.action.Runs.Using == model.ActionRunsUsingComposite { + sal.RunContext.setupActionInputs(sal) + return runStepExecutor(sal, stepStagePre, runPreStep(sal)).If(hasPreStep(sal)).If(shouldRunPreStep(sal))(ctx) + } + + return nil + } +} + +func (sal *stepActionLocal) main() common.Executor { + return runStepExecutor(sal, stepStageMain, func(ctx context.Context) error { + actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses) return sal.runAction(sal, actionDir, nil)(ctx) }) } func (sal *stepActionLocal) post() common.Executor { - return func(ctx context.Context) error { - return nil - } + return runStepExecutor(sal, stepStagePost, runPostStep(sal)).If(hasPostStep(sal)).If(shouldRunPostStep(sal)) } func (sal *stepActionLocal) getRunContext() *RunContext { @@ -80,6 +86,31 @@ func (sal *stepActionLocal) getEnv() *map[string]string { return &sal.env } +func (sal *stepActionLocal) getIfExpression(stage stepStage) string { + switch stage { + case stepStageMain: + return sal.Step.If.Value + case stepStagePost: + return sal.action.Runs.PostIf + } + return "" +} + func (sal *stepActionLocal) getActionModel() *model.Action { return sal.action } + +func (sal *stepActionLocal) getCompositeRunContext() *RunContext { + if sal.compositeRunContext == nil { + actionDir := filepath.Join(sal.RunContext.Config.Workdir, sal.Step.Uses) + _, containerActionDir := getContainerActionPaths(sal.getStepModel(), actionDir, sal.RunContext) + + sal.compositeRunContext = newCompositeRunContext(sal.RunContext, sal, containerActionDir) + sal.compositeSteps = sal.compositeRunContext.compositeExecutor(sal.action) + } + return sal.compositeRunContext +} + +func (sal *stepActionLocal) getCompositeSteps() *compositeSteps { + return sal.compositeSteps +} diff --git a/pkg/runner/step_action_local_test.go b/pkg/runner/step_action_local_test.go index 58607299..d0b0dfb0 100644 --- a/pkg/runner/step_action_local_test.go +++ b/pkg/runner/step_action_local_test.go @@ -2,12 +2,14 @@ package runner import ( "context" + "strings" "testing" "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "gopkg.in/yaml.v3" ) type stepActionLocalMocks struct { @@ -80,22 +82,252 @@ func TestStepActionLocalTest(t *testing.T) { return nil }) - err := sal.main()(ctx) + err := sal.pre()(ctx) + assert.Nil(t, err) + err = sal.main()(ctx) assert.Nil(t, err) cm.AssertExpectations(t) salm.AssertExpectations(t) } -func TestStepActionLocalPrePost(t *testing.T) { +func TestStepActionLocalPre(t *testing.T) { + cm := &containerMock{} + salm := &stepActionLocalMocks{} + ctx := context.Background() - sal := &stepActionLocal{} + sal := &stepActionLocal{ + readAction: salm.readAction, + RunContext: &RunContext{ + StepResults: map[string]*model.StepResult{}, + ExprEval: &expressionEvaluator{}, + Config: &Config{ + Workdir: "/tmp", + }, + Run: &model.Run{ + JobID: "1", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "1": { + Defaults: model.Defaults{ + Run: model.RunDefaults{ + Shell: "bash", + }, + }, + }, + }, + }, + }, + JobContainer: cm, + }, + Step: &model.Step{ + ID: "1", + Uses: "./path/to/action", + }, + } + + salm.On("readAction", sal.Step, "/tmp/path/to/action", "", mock.Anything, mock.Anything). + Return(&model.Action{}, nil) err := sal.pre()(ctx) assert.Nil(t, err) - err = sal.post()(ctx) - assert.Nil(t, err) + cm.AssertExpectations(t) + salm.AssertExpectations(t) +} + +func TestStepActionLocalPost(t *testing.T) { + table := []struct { + name string + stepModel *model.Step + actionModel *model.Action + initialStepResults map[string]*model.StepResult + expectedPostStepResult *model.StepResult + err error + mocks struct { + env bool + exec bool + } + }{ + { + name: "main-success", + stepModel: &model.Step{ + ID: "step", + Uses: "./local/action", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "always()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: &model.StepResult{ + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + exec bool + }{ + env: true, + exec: true, + }, + }, + { + name: "main-failed", + stepModel: &model.Step{ + ID: "step", + Uses: "./local/action", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "always()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusFailure, + Outcome: model.StepStatusFailure, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: &model.StepResult{ + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + exec bool + }{ + env: true, + exec: true, + }, + }, + { + name: "skip-if-failed", + stepModel: &model.Step{ + ID: "step", + Uses: "./local/action", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "success()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusFailure, + Outcome: model.StepStatusFailure, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: &model.StepResult{ + Conclusion: model.StepStatusSkipped, + Outcome: model.StepStatusSkipped, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + exec bool + }{ + env: true, + exec: false, + }, + }, + { + name: "skip-if-main-skipped", + stepModel: &model.Step{ + ID: "step", + If: yaml.Node{Value: "failure()"}, + Uses: "./local/action", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "always()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusSkipped, + Outcome: model.StepStatusSkipped, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: nil, + mocks: struct { + env bool + exec bool + }{ + env: false, + exec: false, + }, + }, + } + + for _, tt := range table { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + cm := &containerMock{} + + sal := &stepActionLocal{ + env: map[string]string{}, + RunContext: &RunContext{ + Config: &Config{ + GitHubInstance: "https://github.com", + }, + JobContainer: cm, + Run: &model.Run{ + JobID: "1", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "1": {}, + }, + }, + }, + StepResults: tt.initialStepResults, + }, + Step: tt.stepModel, + action: tt.actionModel, + } + + if tt.mocks.env { + cm.On("UpdateFromImageEnv", &sal.env).Return(func(ctx context.Context) error { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sal.env).Return(func(ctx context.Context) error { return nil }) + cm.On("UpdateFromPath", &sal.env).Return(func(ctx context.Context) error { return nil }) + } + if tt.mocks.exec { + suffixMatcher := func(suffix string) interface{} { + return mock.MatchedBy(func(array []string) bool { + return strings.HasSuffix(array[1], suffix) + }) + } + cm.On("Exec", suffixMatcher("pkg/runner/local/action/post.js"), sal.env, "", "").Return(func(ctx context.Context) error { return tt.err }) + } + + err := sal.post()(ctx) + + assert.Equal(t, tt.err, err) + assert.Equal(t, tt.expectedPostStepResult, sal.RunContext.StepResults["post-step"]) + cm.AssertExpectations(t) + }) + } } diff --git a/pkg/runner/step_action_remote.go b/pkg/runner/step_action_remote.go index f1b532a8..edf4218b 100644 --- a/pkg/runner/step_action_remote.go +++ b/pkg/runner/step_action_remote.go @@ -6,6 +6,7 @@ import ( "io" "io/ioutil" "os" + "path" "path/filepath" "regexp" "strings" @@ -13,88 +14,102 @@ import ( "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/model" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" ) type stepActionRemote struct { - Step *model.Step - RunContext *RunContext - readAction readAction - runAction runAction - action *model.Action - env map[string]string -} - -func (sar *stepActionRemote) pre() common.Executor { - return func(ctx context.Context) error { - return nil - } + Step *model.Step + RunContext *RunContext + compositeRunContext *RunContext + compositeSteps *compositeSteps + readAction readAction + runAction runAction + action *model.Action + env map[string]string + remoteAction *remoteAction } var ( stepActionRemoteNewCloneExecutor = common.NewGitCloneExecutor ) -func (sar *stepActionRemote) main() common.Executor { +func (sar *stepActionRemote) pre() common.Executor { sar.env = map[string]string{} - return runStepExecutor(sar, func(ctx context.Context) error { - remoteAction := newRemoteAction(sar.Step.Uses) - if remoteAction == nil { - return fmt.Errorf("Expected format {org}/{repo}[/path]@ref. Actual '%s' Input string was not in a correct format", sar.Step.Uses) - } + return common.NewPipelineExecutor( + func(ctx context.Context) error { + sar.remoteAction = newRemoteAction(sar.Step.Uses) + if sar.remoteAction == nil { + return fmt.Errorf("Expected format {org}/{repo}[/path]@ref. Actual '%s' Input string was not in a correct format", sar.Step.Uses) + } - remoteAction.URL = sar.RunContext.Config.GitHubInstance + sar.remoteAction.URL = sar.RunContext.Config.GitHubInstance + github := sar.RunContext.getGithubContext() + if sar.remoteAction.IsCheckout() && isLocalCheckout(github, sar.Step) && !sar.RunContext.Config.NoSkipCheckout { + common.Logger(ctx).Debugf("Skipping local actions/checkout because workdir was already copied") + return nil + } + + actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), strings.ReplaceAll(sar.Step.Uses, "/", "-")) + gitClone := stepActionRemoteNewCloneExecutor(common.NewGitCloneExecutorInput{ + URL: sar.remoteAction.CloneURL(), + Ref: sar.remoteAction.Ref, + Dir: actionDir, + Token: github.Token, + }) + var ntErr common.Executor + if err := gitClone(ctx); err != nil { + if err.Error() == "short SHA references are not supported" { + err = errors.Cause(err) + return fmt.Errorf("Unable to resolve action `%s`, the provided ref `%s` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `%s` instead", sar.Step.Uses, sar.remoteAction.Ref, err.Error()) + } else if err.Error() != "some refs were not updated" { + return err + } else { + ntErr = common.NewInfoExecutor("Non-terminating error while running 'git clone': %v", err) + } + } + + remoteReader := func(ctx context.Context) actionYamlReader { + return func(filename string) (io.Reader, io.Closer, error) { + f, err := os.Open(filepath.Join(actionDir, sar.remoteAction.Path, filename)) + return f, f, err + } + } + + return common.NewPipelineExecutor( + ntErr, + func(ctx context.Context) error { + actionModel, err := sar.readAction(sar.Step, actionDir, sar.remoteAction.Path, remoteReader(ctx), ioutil.WriteFile) + sar.action = actionModel + return err + }, + )(ctx) + }, + func(ctx context.Context) error { + sar.RunContext.setupActionInputs(sar) + return nil + }, + runStepExecutor(sar, stepStagePre, runPreStep(sar)).If(hasPreStep(sar)).If(shouldRunPreStep(sar))) +} + +func (sar *stepActionRemote) main() common.Executor { + return runStepExecutor(sar, stepStageMain, func(ctx context.Context) error { github := sar.RunContext.getGithubContext() - if remoteAction.IsCheckout() && isLocalCheckout(github, sar.Step) && !sar.RunContext.Config.NoSkipCheckout { + if sar.remoteAction.IsCheckout() && isLocalCheckout(github, sar.Step) && !sar.RunContext.Config.NoSkipCheckout { common.Logger(ctx).Debugf("Skipping local actions/checkout because workdir was already copied") return nil } actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), strings.ReplaceAll(sar.Step.Uses, "/", "-")) - gitClone := stepActionRemoteNewCloneExecutor(common.NewGitCloneExecutorInput{ - URL: remoteAction.CloneURL(), - Ref: remoteAction.Ref, - Dir: actionDir, - Token: github.Token, - }) - var ntErr common.Executor - if err := gitClone(ctx); err != nil { - if err.Error() == "short SHA references are not supported" { - err = errors.Cause(err) - return fmt.Errorf("Unable to resolve action `%s`, the provided ref `%s` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `%s` instead", sar.Step.Uses, remoteAction.Ref, err.Error()) - } else if err.Error() != "some refs were not updated" { - return err - } else { - ntErr = common.NewInfoExecutor("Non-terminating error while running 'git clone': %v", err) - } - } - - remoteReader := func(ctx context.Context) actionYamlReader { - return func(filename string) (io.Reader, io.Closer, error) { - f, err := os.Open(filepath.Join(actionDir, remoteAction.Path, filename)) - return f, f, err - } - } return common.NewPipelineExecutor( - ntErr, - func(ctx context.Context) error { - actionModel, err := sar.readAction(sar.Step, actionDir, remoteAction.Path, remoteReader(ctx), ioutil.WriteFile) - sar.action = actionModel - log.Debugf("Read action %v from '%s'", sar.action, "Unknown") - return err - }, - sar.runAction(sar, actionDir, remoteAction), + sar.runAction(sar, actionDir, sar.remoteAction), )(ctx) }) } func (sar *stepActionRemote) post() common.Executor { - return func(ctx context.Context) error { - return nil - } + return runStepExecutor(sar, stepStagePost, runPostStep(sar)).If(hasPostStep(sar)).If(shouldRunPostStep(sar)) } func (sar *stepActionRemote) getRunContext() *RunContext { @@ -109,10 +124,43 @@ func (sar *stepActionRemote) getEnv() *map[string]string { return &sar.env } +func (sar *stepActionRemote) getIfExpression(stage stepStage) string { + switch stage { + case stepStagePre: + github := sar.RunContext.getGithubContext() + if sar.remoteAction.IsCheckout() && isLocalCheckout(github, sar.Step) && !sar.RunContext.Config.NoSkipCheckout { + // skip local checkout pre step + return "false" + } + return sar.action.Runs.PreIf + case stepStageMain: + return sar.Step.If.Value + case stepStagePost: + return sar.action.Runs.PostIf + } + return "" +} + func (sar *stepActionRemote) getActionModel() *model.Action { return sar.action } +func (sar *stepActionRemote) getCompositeRunContext() *RunContext { + if sar.compositeRunContext == nil { + actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), strings.ReplaceAll(sar.Step.Uses, "/", "-")) + actionLocation := path.Join(actionDir, sar.remoteAction.Path) + _, containerActionDir := getContainerActionPaths(sar.getStepModel(), actionLocation, sar.RunContext) + + sar.compositeRunContext = newCompositeRunContext(sar.RunContext, sar, containerActionDir) + sar.compositeSteps = sar.compositeRunContext.compositeExecutor(sar.action) + } + return sar.compositeRunContext +} + +func (sar *stepActionRemote) getCompositeSteps() *compositeSteps { + return sar.compositeSteps +} + type remoteAction struct { URL string Org string diff --git a/pkg/runner/step_action_remote_test.go b/pkg/runner/step_action_remote_test.go index fd5ebcd8..209bee5c 100644 --- a/pkg/runner/step_action_remote_test.go +++ b/pkg/runner/step_action_remote_test.go @@ -2,6 +2,7 @@ package runner import ( "context" + "errors" "strings" "testing" @@ -9,6 +10,7 @@ import ( "github.com/nektos/act/pkg/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "gopkg.in/yaml.v3" ) type stepActionRemoteMocks struct { @@ -25,78 +27,430 @@ func (sarm *stepActionRemoteMocks) runAction(step actionStep, actionDir string, return args.Get(0).(func(context.Context) error) } -func TestStepActionRemoteTest(t *testing.T) { - ctx := context.Background() - - cm := &containerMock{} - - sarm := &stepActionRemoteMocks{} - - clonedAction := false - - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input common.NewGitCloneExecutorInput) common.Executor { - return func(ctx context.Context) error { - clonedAction = true - return nil +func TestStepActionRemote(t *testing.T) { + table := []struct { + name string + stepModel *model.Step + result *model.StepResult + mocks struct { + env bool + cloned bool + read bool + run bool } - } - defer (func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - })() - - sar := &stepActionRemote{ - RunContext: &RunContext{ - Config: &Config{ - GitHubInstance: "github.com", + runError error + }{ + { + name: "run-successful", + stepModel: &model.Step{ + ID: "step", + Uses: "remote/action@v1", }, - Run: &model.Run{ - JobID: "1", - Workflow: &model.Workflow{ - Jobs: map[string]*model.Job{ - "1": {}, + result: &model.StepResult{ + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + cloned bool + read bool + run bool + }{ + env: true, + cloned: true, + read: true, + run: true, + }, + }, + { + name: "run-skipped", + stepModel: &model.Step{ + ID: "step", + Uses: "remote/action@v1", + If: yaml.Node{Value: "false"}, + }, + result: &model.StepResult{ + Conclusion: model.StepStatusSkipped, + Outcome: model.StepStatusSkipped, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + cloned bool + read bool + run bool + }{ + env: true, + cloned: true, + read: true, + run: false, + }, + }, + { + name: "run-error", + stepModel: &model.Step{ + ID: "step", + Uses: "remote/action@v1", + }, + result: &model.StepResult{ + Conclusion: model.StepStatusFailure, + Outcome: model.StepStatusFailure, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + cloned bool + read bool + run bool + }{ + env: true, + cloned: true, + read: true, + run: true, + }, + runError: errors.New("error"), + }, + } + + for _, tt := range table { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + cm := &containerMock{} + sarm := &stepActionRemoteMocks{} + + clonedAction := false + + origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor + stepActionRemoteNewCloneExecutor = func(input common.NewGitCloneExecutorInput) common.Executor { + return func(ctx context.Context) error { + clonedAction = true + return nil + } + } + defer (func() { + stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor + })() + + sar := &stepActionRemote{ + RunContext: &RunContext{ + Config: &Config{ + GitHubInstance: "github.com", + }, + Run: &model.Run{ + JobID: "1", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "1": {}, + }, + }, + }, + StepResults: map[string]*model.StepResult{}, + JobContainer: cm, + }, + Step: tt.stepModel, + readAction: sarm.readAction, + runAction: sarm.runAction, + } + + suffixMatcher := func(suffix string) interface{} { + return mock.MatchedBy(func(actionDir string) bool { + return strings.HasSuffix(actionDir, suffix) + }) + } + + if tt.mocks.env { + cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil }) + cm.On("UpdateFromPath", &sar.env).Return(func(ctx context.Context) error { return nil }) + } + if tt.mocks.read { + sarm.On("readAction", sar.Step, suffixMatcher("act/remote-action@v1"), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) + } + if tt.mocks.run { + sarm.On("runAction", sar, suffixMatcher("act/remote-action@v1"), newRemoteAction(sar.Step.Uses)).Return(func(ctx context.Context) error { return tt.runError }) + } + + err := sar.pre()(ctx) + if err == nil { + err = sar.main()(ctx) + } + + assert.Equal(t, tt.runError, err) + assert.Equal(t, tt.mocks.cloned, clonedAction) + assert.Equal(t, tt.result, sar.RunContext.StepResults["step"]) + + sarm.AssertExpectations(t) + cm.AssertExpectations(t) + }) + } +} + +func TestStepActionRemotePre(t *testing.T) { + table := []struct { + name string + stepModel *model.Step + }{ + { + name: "run-pre", + stepModel: &model.Step{ + Uses: "org/repo/path@ref", + }, + }, + } + + for _, tt := range table { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + clonedAction := false + sarm := &stepActionRemoteMocks{} + + origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor + stepActionRemoteNewCloneExecutor = func(input common.NewGitCloneExecutorInput) common.Executor { + return func(ctx context.Context) error { + clonedAction = true + return nil + } + } + defer (func() { + stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor + })() + + sar := &stepActionRemote{ + Step: tt.stepModel, + RunContext: &RunContext{ + Config: &Config{ + GitHubInstance: "https://github.com", + }, + Run: &model.Run{ + JobID: "1", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "1": {}, + }, + }, + }, + }, + readAction: sarm.readAction, + } + + suffixMatcher := func(suffix string) interface{} { + return mock.MatchedBy(func(actionDir string) bool { + return strings.HasSuffix(actionDir, suffix) + }) + } + + sarm.On("readAction", sar.Step, suffixMatcher("org-repo-path@ref"), "path", mock.Anything, mock.Anything).Return(&model.Action{}, nil) + + err := sar.pre()(ctx) + + assert.Nil(t, err) + assert.Equal(t, true, clonedAction) + + sarm.AssertExpectations(t) + }) + } +} + +func TestStepActionRemotePost(t *testing.T) { + table := []struct { + name string + stepModel *model.Step + actionModel *model.Action + initialStepResults map[string]*model.StepResult + expectedEnv map[string]string + expectedPostStepResult *model.StepResult + err error + mocks struct { + env bool + exec bool + } + }{ + { + name: "main-success", + stepModel: &model.Step{ + ID: "step", + Uses: "remote/action@v1", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "always()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + State: map[string]string{ + "key": "value", }, }, }, - StepResults: map[string]*model.StepResult{}, - JobContainer: cm, + expectedEnv: map[string]string{ + "STATE_key": "value", + }, + expectedPostStepResult: &model.StepResult{ + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + exec bool + }{ + env: true, + exec: true, + }, }, - Step: &model.Step{ - Uses: "remote/action@v1", + { + name: "main-failed", + stepModel: &model.Step{ + ID: "step", + Uses: "remote/action@v1", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "always()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusFailure, + Outcome: model.StepStatusFailure, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: &model.StepResult{ + Conclusion: model.StepStatusSuccess, + Outcome: model.StepStatusSuccess, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + exec bool + }{ + env: true, + exec: true, + }, + }, + { + name: "skip-if-failed", + stepModel: &model.Step{ + ID: "step", + Uses: "remote/action@v1", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "success()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusFailure, + Outcome: model.StepStatusFailure, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: &model.StepResult{ + Conclusion: model.StepStatusSkipped, + Outcome: model.StepStatusSkipped, + Outputs: map[string]string{}, + }, + mocks: struct { + env bool + exec bool + }{ + env: true, + exec: false, + }, + }, + { + name: "skip-if-main-skipped", + stepModel: &model.Step{ + ID: "step", + If: yaml.Node{Value: "failure()"}, + Uses: "remote/action@v1", + }, + actionModel: &model.Action{ + Runs: model.ActionRuns{ + Using: "node16", + Post: "post.js", + PostIf: "always()", + }, + }, + initialStepResults: map[string]*model.StepResult{ + "step": { + Conclusion: model.StepStatusSkipped, + Outcome: model.StepStatusSkipped, + Outputs: map[string]string{}, + }, + }, + expectedPostStepResult: nil, + mocks: struct { + env bool + exec bool + }{ + env: false, + exec: false, + }, }, - readAction: sarm.readAction, - runAction: sarm.runAction, } - suffixMatcher := func(suffix string) interface{} { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) + for _, tt := range table { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + cm := &containerMock{} + + sar := &stepActionRemote{ + env: map[string]string{}, + RunContext: &RunContext{ + Config: &Config{ + GitHubInstance: "https://github.com", + }, + JobContainer: cm, + Run: &model.Run{ + JobID: "1", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "1": {}, + }, + }, + }, + StepResults: tt.initialStepResults, + }, + Step: tt.stepModel, + action: tt.actionModel, + } + + if tt.mocks.env { + cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil }) + cm.On("UpdateFromPath", &sar.env).Return(func(ctx context.Context) error { return nil }) + } + if tt.mocks.exec { + cm.On("Exec", []string{"node", "/var/run/act/actions/remote-action@v1/post.js"}, sar.env, "", "").Return(func(ctx context.Context) error { return tt.err }) + } + + err := sar.post()(ctx) + + assert.Equal(t, tt.err, err) + if tt.expectedEnv != nil { + for key, value := range tt.expectedEnv { + assert.Equal(t, value, sar.env[key]) + } + } + assert.Equal(t, tt.expectedPostStepResult, sar.RunContext.StepResults["post-step"]) + cm.AssertExpectations(t) }) } - - cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil }) - cm.On("UpdateFromPath", &sar.env).Return(func(ctx context.Context) error { return nil }) - - sarm.On("readAction", sar.Step, suffixMatcher("act/remote-action@v1"), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) - sarm.On("runAction", sar, suffixMatcher("act/remote-action@v1"), newRemoteAction(sar.Step.Uses)).Return(func(ctx context.Context) error { return nil }) - - err := sar.main()(ctx) - - assert.Nil(t, err) - assert.True(t, clonedAction) - sarm.AssertExpectations(t) - cm.AssertExpectations(t) -} - -func TestStepActionRemotePrePost(t *testing.T) { - ctx := context.Background() - - sar := &stepActionRemote{} - - err := sar.pre()(ctx) - assert.Nil(t, err) - - err = sar.post()(ctx) - assert.Nil(t, err) } diff --git a/pkg/runner/step_docker.go b/pkg/runner/step_docker.go index 3a274d0f..e3bda180 100644 --- a/pkg/runner/step_docker.go +++ b/pkg/runner/step_docker.go @@ -26,7 +26,7 @@ func (sd *stepDocker) pre() common.Executor { func (sd *stepDocker) main() common.Executor { sd.env = map[string]string{} - return runStepExecutor(sd, sd.runUsesContainer()) + return runStepExecutor(sd, stepStageMain, sd.runUsesContainer()) } func (sd *stepDocker) post() common.Executor { @@ -47,6 +47,10 @@ func (sd *stepDocker) getEnv() *map[string]string { return &sd.env } +func (sd *stepDocker) getIfExpression(stage stepStage) string { + return sd.Step.If.Value +} + func (sd *stepDocker) runUsesContainer() common.Executor { rc := sd.RunContext step := sd.Step diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index a26b31a5..9a6fe34e 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -27,7 +27,7 @@ func (sr *stepRun) pre() common.Executor { func (sr *stepRun) main() common.Executor { sr.env = map[string]string{} - return runStepExecutor(sr, common.NewPipelineExecutor( + return runStepExecutor(sr, stepStageMain, common.NewPipelineExecutor( sr.setupShellCommandExecutor(), func(ctx context.Context) error { return sr.getRunContext().JobContainer.Exec(sr.cmd, sr.env, "", sr.Step.WorkingDirectory)(ctx) @@ -53,6 +53,10 @@ func (sr *stepRun) getEnv() *map[string]string { return &sr.env } +func (sr *stepRun) getIfExpression(stage stepStage) string { + return sr.Step.If.Value +} + func (sr *stepRun) setupShellCommandExecutor() common.Executor { return func(ctx context.Context) error { scriptName, script, err := sr.setupShellCommand(ctx) diff --git a/pkg/runner/step_test.go b/pkg/runner/step_test.go index b87efeb0..91d4538d 100644 --- a/pkg/runner/step_test.go +++ b/pkg/runner/step_test.go @@ -230,49 +230,49 @@ func TestIsStepEnabled(t *testing.T) { // success() step := createTestStep(t, "if: success()") - assertObject.True(isStepEnabled(context.Background(), step)) + assertObject.True(isStepEnabled(context.Background(), step.getIfExpression(stepStageMain), step, stepStageMain)) step = createTestStep(t, "if: success()") step.getRunContext().StepResults["a"] = &model.StepResult{ Conclusion: model.StepStatusSuccess, } - assertObject.True(isStepEnabled(context.Background(), step)) + assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) step = createTestStep(t, "if: success()") step.getRunContext().StepResults["a"] = &model.StepResult{ Conclusion: model.StepStatusFailure, } - assertObject.False(isStepEnabled(context.Background(), step)) + assertObject.False(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) // failure() step = createTestStep(t, "if: failure()") - assertObject.False(isStepEnabled(context.Background(), step)) + assertObject.False(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) step = createTestStep(t, "if: failure()") step.getRunContext().StepResults["a"] = &model.StepResult{ Conclusion: model.StepStatusSuccess, } - assertObject.False(isStepEnabled(context.Background(), step)) + assertObject.False(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) step = createTestStep(t, "if: failure()") step.getRunContext().StepResults["a"] = &model.StepResult{ Conclusion: model.StepStatusFailure, } - assertObject.True(isStepEnabled(context.Background(), step)) + assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) // always() step = createTestStep(t, "if: always()") - assertObject.True(isStepEnabled(context.Background(), step)) + assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) step = createTestStep(t, "if: always()") step.getRunContext().StepResults["a"] = &model.StepResult{ Conclusion: model.StepStatusSuccess, } - assertObject.True(isStepEnabled(context.Background(), step)) + assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) step = createTestStep(t, "if: always()") step.getRunContext().StepResults["a"] = &model.StepResult{ Conclusion: model.StepStatusFailure, } - assertObject.True(isStepEnabled(context.Background(), step)) + assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) } diff --git a/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/action.yml b/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/action.yml new file mode 100644 index 00000000..1ba0fc61 --- /dev/null +++ b/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/action.yml @@ -0,0 +1,7 @@ +name: "last action check" +description: "last action check" + +runs: + using: "node16" + main: main.js + post: post.js diff --git a/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/main.js b/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/main.js new file mode 100644 index 00000000..e69de29b diff --git a/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/post.js b/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/post.js new file mode 100644 index 00000000..e147e37d --- /dev/null +++ b/pkg/runner/testdata/uses-action-with-pre-and-post-step/last-action/post.js @@ -0,0 +1,17 @@ +const pre = process.env['ACTION_OUTPUT_PRE']; +const main = process.env['ACTION_OUTPUT_MAIN']; +const post = process.env['ACTION_OUTPUT_POST']; + +console.log({pre, main, post}); + +if (pre !== 'pre') { + throw new Error(`Expected 'pre' but got '${pre}'`); +} + +if (main !== 'main') { + throw new Error(`Expected 'main' but got '${main}'`); +} + +if (post !== 'post') { + throw new Error(`Expected 'post' but got '${post}'`); +} diff --git a/pkg/runner/testdata/uses-action-with-pre-and-post-step/push.yml b/pkg/runner/testdata/uses-action-with-pre-and-post-step/push.yml new file mode 100644 index 00000000..0c3b7933 --- /dev/null +++ b/pkg/runner/testdata/uses-action-with-pre-and-post-step/push.yml @@ -0,0 +1,15 @@ +name: uses-action-with-pre-and-post-step +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: ./uses-action-with-pre-and-post-step/last-action + - uses: nektos/act-test-actions/js-with-pre-and-post-step@main + with: + pre: true + post: true + - run: | + cat $GITHUB_ENV diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/action.yml b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/action.yml new file mode 100644 index 00000000..63be7989 --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/action.yml @@ -0,0 +1,13 @@ +name: "Action with pre and post" +description: "Action with pre and post" + +inputs: + step: + description: "step" + required: true + +runs: + using: "node16" + pre: pre.js + main: main.js + post: post.js diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/main.js b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/main.js new file mode 100644 index 00000000..21dd65b9 --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/main.js @@ -0,0 +1,3 @@ +const { appendFileSync } = require('fs'); +const step = process.env['INPUT_STEP']; +appendFileSync(process.env['GITHUB_ENV'], `;${step}`, { encoding:'utf-8' }) diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/post.js b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/post.js new file mode 100644 index 00000000..d3fff73f --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/post.js @@ -0,0 +1,3 @@ +const { appendFileSync } = require('fs'); +const step = process.env['INPUT_STEP']; +appendFileSync(process.env['GITHUB_ENV'], `;${step}-post`, { encoding:'utf-8' }) diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/pre.js b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/pre.js new file mode 100644 index 00000000..b17cb694 --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/action-with-pre-and-post/pre.js @@ -0,0 +1 @@ +console.log('pre'); diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/composite_action/action.yml b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/composite_action/action.yml new file mode 100644 index 00000000..2d12207a --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/composite_action/action.yml @@ -0,0 +1,12 @@ +name: "Test Composite Action" +description: "Test action uses composite" + +runs: + using: "composite" + steps: + - uses: ./uses-composite-with-pre-and-post-steps/action-with-pre-and-post + with: + step: step1 + - uses: ./uses-composite-with-pre-and-post-steps/action-with-pre-and-post + with: + step: step2 diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/action.yml b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/action.yml new file mode 100644 index 00000000..1ba0fc61 --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/action.yml @@ -0,0 +1,7 @@ +name: "last action check" +description: "last action check" + +runs: + using: "node16" + main: main.js + post: post.js diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/main.js b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/main.js new file mode 100644 index 00000000..e69de29b diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/post.js b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/post.js new file mode 100644 index 00000000..bc6d7edc --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/last-action/post.js @@ -0,0 +1,7 @@ +const output = process.env['STEP_OUTPUT_TEST']; +const expected = 'empty;step1;step2;step2-post;step1-post'; + +console.log(output); +if (output !== expected) { + throw new Error(`Expected '${expected}' but got '${output}'`); +} diff --git a/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/push.yml b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/push.yml new file mode 100644 index 00000000..922d38a4 --- /dev/null +++ b/pkg/runner/testdata/uses-composite-with-pre-and-post-steps/push.yml @@ -0,0 +1,11 @@ +name: uses-composite-with-pre-and-post-steps +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: ./uses-composite-with-pre-and-post-steps/last-action + - uses: actions/checkout@v2 + - run: echo -n "STEP_OUTPUT_TEST=empty" >> $GITHUB_ENV + - uses: ./uses-composite-with-pre-and-post-steps/composite_action