Skip to content

Commit

Permalink
Added workflow_run.workflows references check
Browse files Browse the repository at this point in the history
  • Loading branch information
gmazzo committed Feb 20, 2024
1 parent 5281740 commit a860a2c
Show file tree
Hide file tree
Showing 18 changed files with 242 additions and 24 deletions.
36 changes: 36 additions & 0 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ List of checks:
- [Environment variable names](#check-env-var-names)
- [Permissions](#permissions)
- [Reusable workflows](#check-reusable-workflows)
- [Workflow references at `workflow_run`](#check-workflow-run-refs)
- [ID naming convention](#id-naming-convention)
- [Contexts and special functions availability](#ctx-spfunc-availability)
- [Deprecated workflow commands](#check-deprecated-workflow-commands)
Expand Down Expand Up @@ -2385,6 +2386,41 @@ as `{version: string}`. In the downstream job, actionlint can report an error at

Note that this check only works with local reusable workflow (starting with `./`).

<a name="check-workflow-run-refss"></a>
## Workflows references at `workflow_run`
[`workflow_run` is a trigger event](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run) to run a workflow when another workflow is completed.

actionlint checks that all workflows referenced by name at `workflow_run.workflows` exists.

Example input:

```yaml
# file: .github/workflows/build.yml
on: push
```
```yaml
# file: .github/workflows/test.yml
name: Tests
on: push
```
```yaml
# file: .github/workflows/notify-errors.yml
on:
workflow_run:
types: [completed]
workflows:
- build
- test # ERROR: This workflow does not exist, its name is `Tests`
```
Output:

```
:6:9: Workflow "test" is not defined [workflow_run]
|
6 | - test # ERROR: This workflow does not exist, its name is `Tests`
| ^~~~
```

<a name="id-naming-convention"></a>
## ID naming convention

Expand Down
2 changes: 1 addition & 1 deletion fuzz/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func FuzzCheck(data []byte) int {
v.AddPass(rule)
}

if err := v.Visit(w); err != nil {
if err := v.Visit("", w); err != nil {
return 0
}

Expand Down
19 changes: 15 additions & 4 deletions linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ func (l *Linter) LintFiles(filepaths []string, project *Project) ([]*Error, erro
dbg := l.debugWriter()
acf := NewLocalActionsCacheFactory(dbg)
rwcf := NewLocalReusableWorkflowCacheFactory(cwd, dbg)
rwr := NewRuleWorkflowRun()

type workspace struct {
path string
Expand Down Expand Up @@ -350,7 +351,7 @@ func (l *Linter) LintFiles(filepaths []string, project *Project) ([]*Error, erro
w.path = r // Use relative path if possible
}
}
errs, err := l.check(w.path, src, proj, proc, ac, rwc)
errs, err := l.check(w.path, src, proj, proc, ac, rwc, rwr)
if err != nil {
return fmt.Errorf("fatal error while checking %s: %w", w.path, err)
}
Expand All @@ -372,6 +373,12 @@ func (l *Linter) LintFiles(filepaths []string, project *Project) ([]*Error, erro
// called safely.
proc.wait()

rwErrs := rwr.ComputeMissingReferences()
for i := range ws {
w := &ws[i]
w.errs = append(w.errs, rwErrs[w.path]...)
}

total := 0
for i := range ws {
total += len(ws[i].errs)
Expand Down Expand Up @@ -429,7 +436,7 @@ func (l *Linter) LintFile(path string, project *Project) ([]*Error, error) {
dbg := l.debugWriter()
localActions := NewLocalActionsCache(project, dbg)
localReusableWorkflows := NewLocalReusableWorkflowCache(project, l.cwd, dbg)
errs, err := l.check(path, src, project, proc, localActions, localReusableWorkflows)
errs, err := l.check(path, src, project, proc, localActions, localReusableWorkflows, nil)
proc.wait()
if err != nil {
return nil, err
Expand Down Expand Up @@ -461,7 +468,7 @@ func (l *Linter) Lint(path string, content []byte, project *Project) ([]*Error,
dbg := l.debugWriter()
localActions := NewLocalActionsCache(project, dbg)
localReusableWorkflows := NewLocalReusableWorkflowCache(project, l.cwd, dbg)
errs, err := l.check(path, content, project, proc, localActions, localReusableWorkflows)
errs, err := l.check(path, content, project, proc, localActions, localReusableWorkflows, nil)
proc.wait()
if err != nil {
return nil, err
Expand All @@ -481,6 +488,7 @@ func (l *Linter) check(
proc *concurrentProcess,
localActions *LocalActionsCache,
localReusableWorkflows *LocalReusableWorkflowCache,
ruleWorkflowRun *RuleWorkflowRun,
) ([]*Error, error) {
// Note: This method is called to check multiple files in parallel.
// It must be thread safe assuming fields of Linter are not modified while running.
Expand Down Expand Up @@ -555,6 +563,9 @@ func (l *Linter) check(
} else {
l.log("Rule \"pyflakes\" was disabled since pyflakes command name was empty")
}
if ruleWorkflowRun != nil {
rules = append(rules, ruleWorkflowRun)
}
if l.onRulesCreated != nil {
rules = l.onRulesCreated(rules)
}
Expand All @@ -575,7 +586,7 @@ func (l *Linter) check(
}
}

if err := v.Visit(w); err != nil {
if err := v.Visit(path, w); err != nil {
l.debug("error occurred while visiting workflow syntax tree: %v", err)
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions pass.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type Pass interface {
// VisitJobPost is callback when visiting Job node after visiting its children. It returns internal error when it cannot continue the process
VisitJobPost(node *Job) error
// VisitWorkflowPre is callback when visiting Workflow node before visiting its children. It returns internal error when it cannot continue the process
VisitWorkflowPre(node *Workflow) error
VisitWorkflowPre(path string, node *Workflow) error
// VisitWorkflowPost is callback when visiting Workflow node after visiting its children. It returns internal error when it cannot continue the process
VisitWorkflowPost(node *Workflow) error
}
Expand Down Expand Up @@ -47,14 +47,14 @@ func (v *Visitor) reportElapsedTime(what string, start time.Time) {
}

// Visit visits given syntax tree in depth-first order
func (v *Visitor) Visit(n *Workflow) error {
func (v *Visitor) Visit(path string, n *Workflow) error {
var t time.Time
if v.dbg != nil {
t = time.Now()
}

for _, p := range v.passes {
if err := p.VisitWorkflowPre(n); err != nil {
if err := p.VisitWorkflowPre(path, n); err != nil {
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (r *RuleBase) VisitJobPre(node *Job) error { return nil }
func (r *RuleBase) VisitJobPost(node *Job) error { return nil }

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (r *RuleBase) VisitWorkflowPre(node *Workflow) error { return nil }
func (r *RuleBase) VisitWorkflowPre(path string, node *Workflow) error { return nil }

// VisitWorkflowPost is callback when visiting Workflow node after visiting its children.
func (r *RuleBase) VisitWorkflowPost(node *Workflow) error { return nil }
Expand Down
2 changes: 1 addition & 1 deletion rule_env_var.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (rule *RuleEnvVar) VisitJobPre(n *Job) error {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleEnvVar) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleEnvVar) VisitWorkflowPre(_ string, n *Workflow) error {
rule.checkEnv(n.Env)
return nil
}
Expand Down
6 changes: 3 additions & 3 deletions rule_env_var_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func testValidateEnvVarName(t *testing.T, name string) []*Error {
}
w := &Workflow{Env: e}
r := NewRuleEnvVar()
if err := r.VisitWorkflowPre(w); err != nil {
if err := r.VisitWorkflowPre("", w); err != nil {
t.Fatal(err)
}
return r.Errs()
Expand Down Expand Up @@ -54,7 +54,7 @@ func TestRuleEnvVarCheckInvalidName(t *testing.T) {
func TestRuleEnvVarSkipEdgeCaseEnv(t *testing.T) {
w := &Workflow{Env: nil}
r := NewRuleEnvVar()
if err := r.VisitWorkflowPre(w); err != nil {
if err := r.VisitWorkflowPre("", w); err != nil {
t.Fatal(err)
}
if errs := r.Errs(); len(errs) > 0 {
Expand All @@ -67,7 +67,7 @@ func TestRuleEnvVarSkipEdgeCaseEnv(t *testing.T) {
},
}
r = NewRuleEnvVar()
if err := r.VisitWorkflowPre(w); err != nil {
if err := r.VisitWorkflowPre("", w); err != nil {
t.Fatal(err)
}
if errs := r.Errs(); len(errs) > 0 {
Expand Down
2 changes: 1 addition & 1 deletion rule_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewRuleEvents() *RuleEvents {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleEvents) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleEvents) VisitWorkflowPre(_ string, n *Workflow) error {
for _, e := range n.On {
rule.checkEvent(e)
}
Expand Down
2 changes: 1 addition & 1 deletion rule_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewRuleExpression(actionsCache *LocalActionsCache, workflowCache *LocalReus
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleExpression) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleExpression) VisitWorkflowPre(_ string, n *Workflow) error {
rule.checkString(n.Name, "")

for _, e := range n.On {
Expand Down
2 changes: 1 addition & 1 deletion rule_glob.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func NewRuleGlob() *RuleGlob {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleGlob) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleGlob) VisitWorkflowPre(_ string, n *Workflow) error {
for _, e := range n.On {
if w, ok := e.(*WebhookEvent); ok {
rule.checkGitRefGlobs(w.Branches)
Expand Down
2 changes: 1 addition & 1 deletion rule_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (rule *RulePermissions) VisitJobPre(n *Job) error {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RulePermissions) VisitWorkflowPre(n *Workflow) error {
func (rule *RulePermissions) VisitWorkflowPre(_ string, n *Workflow) error {
rule.checkPermissions(n.Permissions)
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion rule_pyflakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (rule *RulePyflakes) VisitJobPost(n *Job) error {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RulePyflakes) VisitWorkflowPre(n *Workflow) error {
func (rule *RulePyflakes) VisitWorkflowPre(_ string, n *Workflow) error {
if n.Defaults != nil && n.Defaults.Run != nil {
rule.workflowShellIsPython = getShellIsPythonKind(n.Defaults.Run.Shell)
}
Expand Down
2 changes: 1 addition & 1 deletion rule_shell_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (rule *RuleShellName) VisitJobPost(n *Job) error {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleShellName) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleShellName) VisitWorkflowPre(_ string, n *Workflow) error {
if n.Defaults != nil && n.Defaults.Run != nil {
rule.checkShellName(n.Defaults.Run.Shell)
}
Expand Down
2 changes: 1 addition & 1 deletion rule_shellcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (rule *RuleShellcheck) VisitJobPost(n *Job) error {
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleShellcheck) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleShellcheck) VisitWorkflowPre(_ string, n *Workflow) error {
if n.Defaults != nil && n.Defaults.Run != nil && n.Defaults.Run.Shell != nil {
rule.workflowShell = n.Defaults.Run.Shell.Value
}
Expand Down
2 changes: 1 addition & 1 deletion rule_workflow_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func NewRuleWorkflowCall(workflowPath string, cache *LocalReusableWorkflowCache)
}

// VisitWorkflowPre is callback when visiting Workflow node before visiting its children.
func (rule *RuleWorkflowCall) VisitWorkflowPre(n *Workflow) error {
func (rule *RuleWorkflowCall) VisitWorkflowPre(_ string, n *Workflow) error {
for _, e := range n.On {
if e, ok := e.(*WorkflowCallEvent); ok {
rule.workflowCallEventPos = e.Pos
Expand Down
6 changes: 3 additions & 3 deletions rule_workflow_call_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestRuleWorkflowCallNestedWorkflowCalls(t *testing.T) {
c := NewLocalReusableWorkflowCache(nil, "", nil)
r := NewRuleWorkflowCall("", c)

if err := r.VisitWorkflowPre(w); err != nil {
if err := r.VisitWorkflowPre("", w); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -126,7 +126,7 @@ func TestRuleWorkflowCallWriteEventNodeToMetadataCache(t *testing.T) {
c := NewLocalReusableWorkflowCache(&Project{cwd, nil}, cwd, nil)
r := NewRuleWorkflowCall("test-workflow.yaml", c)

if err := r.VisitWorkflowPre(w); err != nil {
if err := r.VisitWorkflowPre("", w); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -330,7 +330,7 @@ func TestRuleWorkflowCallCheckReusableWorkflowCall(t *testing.T) {
},
},
}
if err := r.VisitWorkflowPre(w); err != nil {
if err := r.VisitWorkflowPre("", w); err != nil {
t.Fatal(err)
}

Expand Down

0 comments on commit a860a2c

Please sign in to comment.