diff --git a/docs/checks.md b/docs/checks.md index dda5cbc42..26129db02 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -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) @@ -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 `./`). + +## 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` + | ^~~~ +``` + ## ID naming convention diff --git a/fuzz/check.go b/fuzz/check.go index b6d5badf6..d2e718bd8 100644 --- a/fuzz/check.go +++ b/fuzz/check.go @@ -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 } diff --git a/linter.go b/linter.go index 0d440f73d..82091cd7d 100644 --- a/linter.go +++ b/linter.go @@ -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 @@ -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) } @@ -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) @@ -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 @@ -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 @@ -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. @@ -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) } @@ -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 } diff --git a/pass.go b/pass.go index e626fe221..e57523e2f 100644 --- a/pass.go +++ b/pass.go @@ -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 } @@ -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 } } diff --git a/rule.go b/rule.go index e6eeb1987..d717643e4 100644 --- a/rule.go +++ b/rule.go @@ -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 } diff --git a/rule_env_var.go b/rule_env_var.go index 0970cb2b3..af06936b3 100644 --- a/rule_env_var.go +++ b/rule_env_var.go @@ -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 } diff --git a/rule_env_var_test.go b/rule_env_var_test.go index 5d5aa6210..9496ce7d2 100644 --- a/rule_env_var_test.go +++ b/rule_env_var_test.go @@ -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() @@ -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 { @@ -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 { diff --git a/rule_events.go b/rule_events.go index f012215e2..0952c18c0 100644 --- a/rule_events.go +++ b/rule_events.go @@ -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) } diff --git a/rule_expression.go b/rule_expression.go index e7413661a..f2f8abbaa 100644 --- a/rule_expression.go +++ b/rule_expression.go @@ -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 { diff --git a/rule_glob.go b/rule_glob.go index 64bbba68e..07d32a6fb 100644 --- a/rule_glob.go +++ b/rule_glob.go @@ -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) diff --git a/rule_permissions.go b/rule_permissions.go index 8a08c65fa..48bf20605 100644 --- a/rule_permissions.go +++ b/rule_permissions.go @@ -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 } diff --git a/rule_pyflakes.go b/rule_pyflakes.go index b09201a15..d059229c5 100644 --- a/rule_pyflakes.go +++ b/rule_pyflakes.go @@ -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) } diff --git a/rule_shell_name.go b/rule_shell_name.go index e867b4669..747dc9610 100644 --- a/rule_shell_name.go +++ b/rule_shell_name.go @@ -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) } diff --git a/rule_shellcheck.go b/rule_shellcheck.go index b10ba220d..9581a0a67 100644 --- a/rule_shellcheck.go +++ b/rule_shellcheck.go @@ -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 } diff --git a/rule_workflow_call.go b/rule_workflow_call.go index 46411121c..a8850b878 100644 --- a/rule_workflow_call.go +++ b/rule_workflow_call.go @@ -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 diff --git a/rule_workflow_call_test.go b/rule_workflow_call_test.go index cf33e8b24..48ccc5bf3 100644 --- a/rule_workflow_call_test.go +++ b/rule_workflow_call_test.go @@ -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) } @@ -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) } @@ -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) } diff --git a/rule_workflow_run.go b/rule_workflow_run.go new file mode 100644 index 000000000..5086e9542 --- /dev/null +++ b/rule_workflow_run.go @@ -0,0 +1,97 @@ +package actionlint + +import ( + "path/filepath" + "strings" + "sync" +) + +type RuleWorkflowRun struct { + RuleBase + refs map[string]*workflowRef + mutex sync.Mutex +} + +type workflowRef struct { + seen bool + refAt []*workflowRefBy +} + +type workflowRefBy struct { + path string + *Pos +} + +func NewRuleWorkflowRun() *RuleWorkflowRun { + return &RuleWorkflowRun{ + RuleBase: RuleBase{ + name: "workflow_run", + desc: "Checks referenced workflows in `workflow_run` exists", + }, + refs: make(map[string]*workflowRef), + mutex: sync.Mutex{}, + } +} + +func (rule *RuleWorkflowRun) resolve(workflow string) *workflowRef { + rule.mutex.Lock() + ref := rule.refs[workflow] + if ref == nil { + ref = &workflowRef{refAt: make([]*workflowRefBy, 0)} + rule.refs[workflow] = ref + } + defer rule.mutex.Unlock() + return ref +} + +func (w *Workflow) computeName(path string) string { + if w.Name != nil { + return w.Name.Value + } + return strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) +} + +func (rule *RuleWorkflowRun) VisitWorkflowPre(path string, node *Workflow) error { + rule.workflowSeen(path, node) + + for _, event := range node.On { + switch e := event.(type) { + case *WebhookEvent: + for _, ref := range e.Workflows { + rule.workflowReferenced(path, ref) + } + } + } + return nil +} + +func (rule *RuleWorkflowRun) workflowSeen(path string, node *Workflow) { + name := node.computeName(path) + rule.Debug("workflowSeen: %q\n", name) + rule.resolve(name).seen = true +} + +func (rule *RuleWorkflowRun) workflowReferenced(path string, refAt *String) { + rule.Debug("workflowReferenced: %q\n", refAt.Value) + ref := rule.resolve(refAt.Value) + ref.refAt = append(ref.refAt, &workflowRefBy{path, refAt.Pos}) +} + +func (rule *RuleWorkflowRun) ComputeMissingReferences() map[string][]*Error { + errs := make(map[string][]*Error) + + rule.mutex.Lock() + for workflow, refs := range rule.refs { + if !refs.seen { + for _, refAt := range refs.refAt { + errs[refAt.path] = append( + errs[refAt.path], + errorfAt(refAt.Pos, rule.name, "Workflow %q is not defined", workflow), + ) + } + } + } + rule.Debug("ComputeMissingReferences: %q\n", errs) + defer rule.mutex.Unlock() + return errs +} diff --git a/rule_workflow_run_test.go b/rule_workflow_run_test.go new file mode 100644 index 000000000..992c25cdf --- /dev/null +++ b/rule_workflow_run_test.go @@ -0,0 +1,74 @@ +package actionlint + +import ( + "reflect" + "testing" +) + +func TestRuleWorkflowRunCheck(t *testing.T) { + type testWorkflow struct { + path string + node *Workflow + } + + type expectedErr struct { + path string + workflows []*testWorkflow + } + + pos := &Pos{1, 1} + + expectErr := func(expected ...expectedErr) map[string][]*Error { + errs := make(map[string][]*Error) + for _, e := range expected { + for _, w := range e.workflows { + name := w.node.computeName(w.path) + errs[e.path] = append(errs[e.path], errorfAt(pos, "workflow_run", "Workflow %q is not defined", name)) + } + } + return errs + } + + w1 := &testWorkflow{path: "repo/w1.yml", node: &Workflow{}} + w2 := &testWorkflow{path: "repo/w2.yml", node: &Workflow{Name: &String{Value: "A Workflow", Pos: pos}}} + w3 := &testWorkflow{path: "repo/w3.yml", node: &Workflow{ + Name: &String{Value: "anotherWorkflow", Pos: &Pos{1, 1}}, + On: []Event{&WebhookEvent{ + Hook: &String{Value: "workflow_run"}, + Workflows: []*String{ + {Value: "w1", Pos: pos}, + {Value: "A Workflow", Pos: pos}, + }}}, + }} + + tests := []struct { + name string + workflows []*testWorkflow + expectErrs map[string][]*Error + }{ + {"only w1", []*testWorkflow{w1}, expectErr()}, + {"only w2", []*testWorkflow{w2}, expectErr()}, + {"w1 and w2", []*testWorkflow{w1, w2}, expectErr()}, + {"only w3", []*testWorkflow{w3}, expectErr(expectedErr{"repo/w3.yml", []*testWorkflow{w1, w2}})}, + {"w1 and w3", []*testWorkflow{w1, w3}, expectErr(expectedErr{"repo/w3.yml", []*testWorkflow{w2}})}, + {"w2 and w3", []*testWorkflow{w2, w3}, expectErr(expectedErr{"repo/w3.yml", []*testWorkflow{w1}})}, + {"all", []*testWorkflow{w1, w2, w3}, expectErr()}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + r := NewRuleWorkflowRun() + for _, w := range tc.workflows { + if err := r.VisitWorkflowPre(w.path, w.node); err != nil { + t.Fatal(err) + } + } + + errs := r.ComputeMissingReferences() + + if !reflect.DeepEqual(tc.expectErrs, errs) { + t.Fatalf("Wanted %d errors but have: %v", len(tc.expectErrs), errs) + } + }) + } +}