Skip to content

Commit

Permalink
implement pre and post steps (#1089)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@KnisterPeter
KnisterPeter authored May 24, 2022
1 parent ebb408f commit 943a0e6
Show file tree
Hide file tree
Showing 37 changed files with 1,551 additions and 341 deletions.
16 changes: 8 additions & 8 deletions pkg/exprparser/functions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
36 changes: 30 additions & 6 deletions pkg/exprparser/interpreter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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())
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/exprparser/interpreter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions pkg/model/action.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions pkg/model/step_result.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 943a0e6

Please sign in to comment.