From 8a473943c3dffbbf47c2a2ef8b261e845646fa6e Mon Sep 17 00:00:00 2001
From: Markus Wolf <KnisterPeter@users.noreply.github.com>
Date: Wed, 8 Jun 2022 17:36:08 +0200
Subject: [PATCH] fix: skip local actions pre step in any case (#1204)
* fix: skip local actions pre step in any case
We should skip local actions pre step, as it is not supported by github.
In turn we may need to late prepare remote actions which are run
as steps in a local composite action.
Fixes #1193
* test: remove obsolete test case
Since local actions does not run any pre-step anymore we don't test this case.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
pkg/runner/step_action_local.go | 24 +++++----------
pkg/runner/step_action_local_test.go | 46 ----------------------------
pkg/runner/step_action_remote.go | 43 +++++++++++++++++---------
3 files changed, 35 insertions(+), 78 deletions(-)
diff --git a/pkg/runner/step_action_local.go b/pkg/runner/step_action_local.go
index ad0e5af..575d78e 100644
--- a/pkg/runner/step_action_local.go
+++ b/pkg/runner/step_action_local.go
@@ -28,9 +28,16 @@ func (sal *stepActionLocal) pre() common.Executor {
sal.env = map[string]string{}
return func(ctx context.Context) error {
+ return nil
+ }
+}
+
+func (sal *stepActionLocal) main() common.Executor {
+ return runStepExecutor(sal, stepStageMain, func(ctx context.Context) error {
if common.Dryrun(ctx) {
return nil
}
+
actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses)
localReader := func(ctx context.Context) actionYamlReader {
@@ -55,23 +62,6 @@ func (sal *stepActionLocal) pre() common.Executor {
sal.action = actionModel
- // 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 {
- if common.Dryrun(ctx) {
- return nil
- }
- actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses)
return sal.runAction(sal, actionDir, nil)(ctx)
})
}
diff --git a/pkg/runner/step_action_local_test.go b/pkg/runner/step_action_local_test.go
index d0b0dfb..09f7597 100644
--- a/pkg/runner/step_action_local_test.go
+++ b/pkg/runner/step_action_local_test.go
@@ -92,52 +92,6 @@ func TestStepActionLocalTest(t *testing.T) {
salm.AssertExpectations(t)
}
-func TestStepActionLocalPre(t *testing.T) {
- cm := &containerMock{}
- salm := &stepActionLocalMocks{}
-
- ctx := context.Background()
-
- 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)
-
- cm.AssertExpectations(t)
- salm.AssertExpectations(t)
-}
-
func TestStepActionLocalPost(t *testing.T) {
table := []struct {
name string
diff --git a/pkg/runner/step_action_remote.go b/pkg/runner/step_action_remote.go
index edf4218..7f55c68 100644
--- a/pkg/runner/step_action_remote.go
+++ b/pkg/runner/step_action_remote.go
@@ -32,11 +32,14 @@ var (
stepActionRemoteNewCloneExecutor = common.NewGitCloneExecutor
)
-func (sar *stepActionRemote) pre() common.Executor {
- sar.env = map[string]string{}
-
+func (sar *stepActionRemote) prepareActionExecutor() common.Executor {
return common.NewPipelineExecutor(
func(ctx context.Context) error {
+ if sar.remoteAction != nil && sar.action != nil {
+ // we are already good to run
+ return nil
+ }
+
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)
@@ -88,24 +91,34 @@ func (sar *stepActionRemote) pre() common.Executor {
func(ctx context.Context) error {
sar.RunContext.setupActionInputs(sar)
return nil
- },
+ })
+}
+
+func (sar *stepActionRemote) pre() common.Executor {
+ sar.env = map[string]string{}
+
+ return common.NewPipelineExecutor(
+ sar.prepareActionExecutor(),
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 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
- }
+ return common.NewPipelineExecutor(
+ sar.prepareActionExecutor(),
+ runStepExecutor(sar, stepStageMain, func(ctx context.Context) error {
+ 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, "/", "-"))
+ actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), strings.ReplaceAll(sar.Step.Uses, "/", "-"))
- return common.NewPipelineExecutor(
- sar.runAction(sar, actionDir, sar.remoteAction),
- )(ctx)
- })
+ return common.NewPipelineExecutor(
+ sar.runAction(sar, actionDir, sar.remoteAction),
+ )(ctx)
+ }),
+ )
}
func (sar *stepActionRemote) post() common.Executor {