Skip to content

Commit

Permalink
feat: run pre step from actions (nektos#1110)
Browse files Browse the repository at this point in the history 
This PR implements running pre steps for remote actions.
This includes remote actions using inside local composite actions.
  • Loading branch information
@KnisterPeter
KnisterPeter authored and github-actions committed May 24, 2022
1 parent de6aeb0 commit c899e29
Show file tree
Hide file tree
Showing 12 changed files with 376 additions and 111 deletions.
182 changes: 142 additions & 40 deletions pkg/runner/action.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,27 @@ 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()
Expand Down Expand Up @@ -139,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)}
Expand All @@ -172,7 +175,7 @@ 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
}

Expand Down Expand Up @@ -372,6 +375,29 @@ func newStepContainer(ctx context.Context, step step, image string, cmd []string
return stepContainer
}

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()

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
}

func populateEnvsFromSavedState(env *map[string]string, step actionStep, rc *RunContext) {
stepResult := rc.StepResults[step.getStepModel().ID]
if stepResult != nil {
Expand Down Expand Up @@ -424,6 +450,78 @@ 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)
Expand Down Expand Up @@ -467,37 +565,41 @@ func runPostStep(step actionStep) common.Executor {
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 = ""
}

populateEnvsFromSavedState(step.getEnv(), step, rc)
actionLocation := ""
if actionPath != "" {
actionLocation = path.Join(actionDir, actionPath)
} else {
actionLocation = actionDir
}

// 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 = ""
}
_, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc)

actionLocation := ""
if actionPath != "" {
actionLocation = path.Join(actionDir, actionPath)
} else {
actionLocation = actionDir
}
switch action.Runs.Using {
case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16:

_, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc)
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)

Expand Down
10 changes: 2 additions & 8 deletions pkg/runner/action_composite.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,12 @@ func execAsComposite(step actionStep) common.Executor {
return func(ctx context.Context) error {
compositerc := step.getCompositeRunContext()

var err error
steps := step.getCompositeSteps()
compositerc.updateCompositeRunContext(rc, step)

ctx = WithCompositeLogger(ctx, &compositerc.Masks)

// todo: pre should be run in the pre step
err = steps.pre(ctx)
if err == nil {
compositerc.updateCompositeRunContext(rc, step)
err = steps.main(ctx)
}
compositerc.updateCompositeRunContext(rc, step)
err := steps.main(ctx)

// Map outputs from composite RunContext to job RunContext
eval := compositerc.NewExpressionEvaluator()
Expand Down
1 change: 1 addition & 0 deletions pkg/runner/runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
2 changes: 1 addition & 1 deletion pkg/runner/step.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func isStepEnabled(ctx context.Context, expr string, step step) (bool, error) {

runStep, err := EvalBool(rc.NewStepExpressionEvaluator(step), expr)
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
Expand Down
22 changes: 15 additions & 7 deletions pkg/runner/step_action_local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,9 @@ type stepActionLocal struct {
}

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, stepStageMain, 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 {
Expand All @@ -58,6 +52,20 @@ func (sal *stepActionLocal) main() 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 {
actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses)
return sal.runAction(sal, actionDir, nil)(ctx)
})
}
Expand Down
43 changes: 41 additions & 2 deletions pkg/runner/step_action_local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,60 @@ 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 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)

cm.AssertExpectations(t)
salm.AssertExpectations(t)
}

func TestStepActionLocalPost(t *testing.T) {
Expand Down
Loading

0 comments on commit c899e29

Please sign in to comment.