-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add Actionlint check for actions #70
base: v3
Are you sure you want to change the base?
Conversation
adb110b
to
7c8d7bd
Compare
.github/workflows/self_check.yml
Outdated
actionlint: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears dependabot (as well as Renovate) supports doing this for various formats...
https://github.blog/changelog/2022-10-31-dependabot-now-updates-comments-in-github-actions-workflows-referencing-action-versions/
I think given the use of this repo, it's probably best to pin all the workflows? I can make a separate PR to do that for the reusable ones if this format is preferred and if someone would like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of the exact pinning because you get so many changes just to update some version. Reading through the history gets quite hard IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to very much agree with you generally. I threw it up not so much for these checks specifically, but as an example / point of discussion.
My take is that given the issue of supply chain attacks (and the blast radius of any problem, with so many repos consuming it), and also the fact that this repo basically has nothing else but this (combined with the fact that dependabot makes it easy to keep updated, and has a configurable schedule, which we could adjust to monthly or quarterly), it might be worth considering for this repo, especially in light of the very real (though admittedly not using this exact vector) exploits against Puppet's release process.
Testing-wise, long term, might also be interesting to embed a small fake module inside this repo, for the purposes of testing the job against itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, I took them out here. But, if it's decided at some point that it's wise to pin them, let me know and I can throw up a PR doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if that looks better now.
.yamllint
Outdated
|
||
rules: | ||
line-length: | ||
max: 250 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's only one line that's > 200; we could probably fold it with >
and set this a little tighter 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that the description for the working-directory
? I think we can split that and drop this whole file. I'd prefer that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Yeah, I wasn't sure how strict we wanted to be on line length since sometimes with GHA, it's unavoidable. there are quite a lot of lines in the 150-200 range still.. I can make a PR that uses yamlfix
to remove extra quotes, and fold the descriptions, but maybe it's better to do some of that separately, and then see where our max line length ends up?
It's github/workflows/beaker.yml:250
- currently 201 characters
run: bundle exec metadata2gha --domain ${{ inputs.domain }} --pidfile-workaround ${{ inputs.pidfile_workaround }} --beaker-facter "${{ inputs.beaker_facter }}" --beaker-hosts "${{ inputs.beaker_hosts }}"
I think in this case, folding with > and putting it across multiple lines would work Ok probably
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ekohl want to see what you think of it as it is now?
Check the actions in this repo with actionlint and yamllint; make the filename generic so that other checks can also be added
7c8d7bd
to
31d52f4
Compare
Check the actions in this repo with actionlint and yamllint; make the filename generic so that other checks can also be added