Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add workflow_run.workflows references check #400

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gmazzo
Copy link

@gmazzo gmazzo commented Feb 19, 2024

Implements #83 by introducing a new workflow_run rule to check the existence of referenced workflows at that trigger.

This rule is a bit different than others, it will be a singleton instance for the check run (instead of one per file), to allow collecting workflow names from the whole suite.

Because of that, it can only apply when calling LintFiles (nil for other usages of the linter).
It validates that any name reference on workflow_run.workflow exists in another workflow of the same set.

on:
  workflow_run:
    types: [completed]
    workflows:
      - build
      - test # ERROR: This workflow does not exist, its name is `Tests`

image

This is my first experience working with Go, feel free to suggest any change regarding code style or best practices.

@@ -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
Copy link
Author

@gmazzo gmazzo Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need the path here to infer the name from the filename, because it's not always defined explicitly on the workflow's YAML.

I also explored adding a new VisitFile method to avoid touching too many files, but because of the stateful nature of this rule, and the concurrent approach for processing files, the current name stored as a field of the rule itself gets overridden, leading to a race condition.

So I end up adding this extra parameter here to compute the final name.

I could also add the final name to the Workflow structure, but it didn't feel right, as that stands for a pure YAML model.

@gmazzo gmazzo force-pushed the workflow_run_refs branch 3 times, most recently from a860a2c to 88e10e6 Compare February 20, 2024 10:44
@gmazzo gmazzo marked this pull request as ready for review February 20, 2024 10:56
@gmazzo
Copy link
Author

gmazzo commented Mar 6, 2024

Hi @rhysd, I see there is activity in this repo. Any chance to get this one reviewed? We are currently using a forked internal version of your tool.

Great work by the way!

@gmazzo gmazzo changed the title Added workflow_run.workflows references check feat: Add workflow_run.workflows references check Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant