actionlint
actionlint copied to clipboard
feat: Lint actions
This PR tries to address #46 to lint action metadata files (action.yml
/ action.yaml
) analog to workflow files. In addition to scheme validation this allows finding more sophisticated errors by running pyflake, shellcheck, validation expressions and other cross field references (e.g. outputs). The main advantage is obviously for people writing composite actions.
Included changes
This PR tries to cover all essential aspects of this feature and provides example implementation for most of them:
- Add new
parse*
methods to parse action metadata. - Add a new input-format parameter (both to the CLI and linteropts) to decide whether the input should be treated as workflow file or action metadata file.
- Adapt rules if needed to handle action metadata files, too.
- Add new branding rule to validate branding values.
- Adapt the playground to allow linting of actions, too.
- Add tests to correctly valid common actions and caught various issues.
Implementation decisions
We needed to take a few implementation decisions where it is not clear whether to picked the best option (once they are decided we will squash our changes and mark the PR as ready):
-
API stability: the public interface was kept unchanged unchanged (if needed function alias like
Parse
andVisit
were added). At the moment, the visitorspass
interface is extended byVisitActionPre
andVisitActionPost
, however it reusesvisitStep
. This could "crash" (nil value accessed) custom rules asVisitWorkflowPre
,VisitJobPre
might not be called beforevisitStep
: theRuleID
,RuleExpression
had this issue. -
parseAction vs ActionMetadata: The local action metadata cache (
action_metadata.go
) was left unchanged (it appears to be more light-weight and overall simpler to leave as small duplication). -
LintDir behavior: to keep the change simple,
LintDir
will only lint arbitrary.yml
,.yaml
files if the directory is.github/workflows
. This should be an issue in practice (as workflows needs to be in such directory to be interpreted by GitHub), but it required thetestdata/projects
tests to be adapted (specify-input-format=workflow
would also work). -
Default behavior: actions are linted by default (at least if
actionlint
is invoked without extra arguments) -
Action location: At the moment, any
action.yml
/action.yaml
file within the repository is considered an action metadata file (top level, or in any subdirectory at any level). While it ensures by default any such file is discovered, it might slow if it is a big repository and could cause false positives (the filename isn't that specific). Discovery could be reduced; but, new config option might be needed to handle more complex use-cases.-input-format=workflow
was added to the dog-food CI calls to ensure thetestdata
actions aren't discovered. -
CLI shortcut: this PR added
actionlint $dir
to lint only files within the specific directory, e.g.actionlint .github/actions
(it was originally added for testing purposes but it might be helpful for others, too). -
Expression values: Sadly, we haven't found anything in the GitHub documentation which expressions are supported action metadata files (probably the step files are the same, however,
inputs.X.default
,runs.envs.X
,runs.pre-if
,runs.post-if
are new. E.g. I suspect special functions likealways()
,success()
are supported forruns.pre-if
but I can't proof it. -
Branding values: the allowed branding values (
color
andicon
) are currently hardcoded. I suspect, a script to extracted them from GitHub's docs should be added?
Marking PR as ready to indicate further work depends on PR review.
What's the status on this? Will this eventually be merged or should I look for another tool to lint action files? I've tried @msw-kialo changes on my action files and it seems to work perfectly. I would like to use this in my actions workflow without having to clone and build their fork.
I am ready to make needed adjustments, rebases, restructure the PR/change etc. However, I am waiting for a first review from @rhysd before I invest more time in a potentially wrong direction.
- Branding values: the allowed branding values (color and icon) are currently hardcoded. I suspect, a script to extracted them from GitHub's docs should be added?
I'm not sure if GitHub themselves provides anything, but there is a JSON payload that has the respective values here: https://github.com/rhysd/actionlint/pull/366
Honestly, I don't think these change often enough that it warrants the added complexity in this initial PR. My 2 cents is this can be tracked as a future feature improvement, and implemented if/when it becomes an issue for folks down the road.
Oh @msw-kialo another thing I forgot to comment on - it's worth updating the README.md
as part of this change, since it's very significant, and worth highlighting this new functionality in the README. What do you think?
@ChrisCarini Thanks for testing it and providing feedback.
I am still open to make needed changes to this PR. However, since https://github.com/rhysd/actionlint/releases/tag/v1.7.0:
- From this version, actionlint starts to check action metadata file action.yml (or action.yaml). At this point, only very basic checks are implemented and contents of steps: are not checked yet.
- Checks for steps: contents are planned to be implemented. Since several differences are expected between steps: in workflow file and steps: in action metadata file (e.g. available contexts), the implementation is delayed to later version. And the current implementation of action metadata parser is ad hoc. I'm planning a large refactorying and breaking changes Go API around it are expected.
It appears that @rhysd has other architecture plans. So until I hear where this is going I won't invest more time into this. Actually, I consider just closing this PR as it is apparently not appreciated.