-
Notifications
You must be signed in to change notification settings - Fork 11
Update GitHub Actions workflow and add Dependabot config #63
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| version: 2 | ||
| updates: | ||
| - package-ecosystem: github-actions | ||
| directory: / | ||
| schedule: | ||
| interval: weekly | ||
| commit-message: | ||
| prefix: ci | ||
| labels: | ||
| - dependencies | ||
| - github-actions | ||
|
|
||
| - package-ecosystem: bundler | ||
| directory: / | ||
| schedule: | ||
| interval: weekly | ||
| commit-message: | ||
| prefix: deps | ||
| labels: | ||
| - dependencies | ||
| - ruby | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,24 +13,24 @@ on: | |
| jobs: | ||
| github-pages: | ||
| runs-on: ubuntu-latest | ||
| container: ruby:3.2.2-bookworm | ||
| container: ruby:3.4.9-bookworm | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - uses: actions/cache@v1 | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather not hit a specific commit, because that makes it a little more annoying to 'follow master' — future versions of checkout/cache could break things, and we wouldn't know until at some point we're forced to update dependencies.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see dependabot maybe manages the deps though?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes dependabot will automatically open PRs for version bumps, following this same schema of pinning to the SHA and adding a human readable comment for the corresponding version (major.minor.patch). Basically it's the same as pinning to the version number except it's more secure, and dependabot takes care of keeping them up to date. A good followup (which I can open later, if "auto-merge" is enabled in the repo settings), is a workflow that will automatically merge minor version bumps from dependabot (minor version bumps won't break anything), while for major version bumps dependabot would open the PR but it wouldn't automatically merge, we would merge it manually to ensure nothing breaks. But even better is to have an automatic build step on dependabot major bumps PRs that ensures the build is clean, and only if that build test passes would we merge (or auto-merge).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. basically I had never touched the "enable auto-merge" setting on any of my projects until recently, because I was afraid that it would automatically merge open PRs without being able to review them. But I just discovered in the past week or two that it actually just enables the possibility of auto-merge, it doesn't actually auto-merge; if enabled though, you can add another workflow that will auto-merge dependabot PRs, and you can choose if it auto-merges all dependabot PRs or only minor version bump PRs. And you can place conditions such as "does this build cleanly?"
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I'm fine with either approach; actions/foo is managed by GitHub so I'm not terribly worried about the sha changing, especially on a static site like this. |
||
| - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 | ||
| with: | ||
| path: vendor/bundle | ||
| key: ${{ runner.os }}-gems-${{ hashFiles('**/Gemfile.lock') }} | ||
| key: ${{ runner.os }}-gems-${{ hashFiles('**/Gemfile.lock', '.ruby-version') }} | ||
|
|
||
| - name: Install Ruby dependencies. | ||
| run: | | ||
| gem install bundler | ||
| bundle install --path vendor/bundle | ||
| bundle install --path vendor/bundle | ||
|
|
||
| - name: Build static site with Jekyll. | ||
| run: bundle exec jekyll build | ||
|
|
||
| - name: Deploy static site to gh-pages branch. | ||
| uses: peaceiris/actions-gh-pages@v3 | ||
| uses: peaceiris/actions-gh-pages@4f9cc6602d3f66b9c108549d475ec49e8ef4d45e # v4.0.0 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| publish_dir: ./_site | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 3.2.2 | ||
| 3.4.9 |
Uh oh!
There was an error while loading. Please reload this page.
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 don't actually want to review dependabot PRs weekly though, and we really have no reason to for a static site. I'd rather bump everything (manually, without dependabot) once a year.
Uh oh!
There was an error while loading. Please reload this page.
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.
Bump manually without dependabot, for minor version bumps? The problem with bumping it manually without dependabot opening a PR, is that it won't get bumped manually and then things start to break (as is currently the case). We all have lots of projects on our hands, and something that needs looking into once a year probably means nobody will look into it. But if dependabot opens the PR, then you at least get notified "hey there's something to look into here". Setting up a workflow to auto-merge minor version bumps means we wouldn't even have to look at it weekly: minor version bumps just get merged and you hardly notice. For GitHub actions, that won't break anything. Major version bumps can be merged manually: dependabot will check weekly but of course there won't be a weekly major version bump, it'll still only happen maybe once a year. And at least you get notified...
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.
That's why I usually like pinning to major versions, and each build will automatically have the latest release in that version (e.g.
v1,v2, etc.).Can Dependabot pin to major versions instead of to commit hashes? That would be easiest for me. You start to see failures immediately if a minor fix in the major version causes it—though for GitHub Actions, there was only one time in the 300+ repos I maintain that a minor action change caused any grief.
Uh oh!
There was an error while loading. Please reload this page.
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.
the best way to prevent a failure, is to add a build job to any PR that touches anything that determines the build (see PR #65 for that). So a dependabot PR that attempts to bump the minor version of an action, will still have to await a successful build action before it can be merged, and that's the best way of knowing whether a bumped action could break a build, you catch it before it is even merged to the codebase.
I don't feel very strongly about this and I am happy to revert to major version strings instead of SHAs, but SHAs are now recommended as the best security practice. I think ChatGPT will sum it up better than I could explain:
And then enabling an auto-merge workflow for minor version bumps, results basically in the same situation as pinning to a major version. Pinning to a major version, you don't have control over any of the minor version bumps, they are transparent. Pinning to a SHA and allowing auto-merge of minor version bumps, they're basically still transparent but with the added security that there is no poisoning, AND you can add the
test buildworkflow that guarantees it won't break anything.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 may be a way to configure dependabot to keep things up to date without too much regular interaction. I guess I'd be open to exploring that, but I still don't think we need it. If we do explore it, I'd prefer it in a separate, isolated PR.
I do want version tags for workflows, not SHAs. I understand the argument ChatGPT is making. The advice is reasonable, and in a different context SHAs might be the preferred approach. It's not my preferred approach here:
I have a more minimal PR here that gets the build working. It makes the minimal changes needed to fix the build - update actions/cache, and add a PR test workflow to validate the change. We can also update other things (ruby version, jekyll version, etc), but I'd prefer to do so in separate, smaller, isolated PRs.
Uh oh!
There was an error while loading. Please reload this page.
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.
That is true, but my understanding of dependabot is that it doesn't do any blind version bumps, but does some security checks to ensure that the bump is valid and not poisoned, considering it also takes care of opening PRs for security fixes on dependencies. I think some trust can be placed in Dependabot? Having that extra security check on the pinned SHA makes it nearly impossible to get a bad bump; whereas linking to major versions means neither we nor dependabot has any control over the version bumps.
In any case, not a big deal, one way or the other, as long as we get the build working again.
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.
#68 is merged, so the build should be working again!
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.
Yeah, main thing is getting the build working.
I mainly like not having 'noise' in the commits (e.g. keeping up with commits, Dependabot going a bit crazy there), especially for a project that's mostly in maintenance mode (at least architecturally).
And security through commit hashes instead of tags is a dubious argument, I'd seriously argue the point with ChatGPT, but then in the end it would probably tell me I'm right, because it just loves making me feel good about whatever I decide haha.
Uh oh!
There was an error while loading. Please reload this page.
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 notice that when people ask AI an opinion on an issue, and they already have a bit of an opinion themselves, AI will often tend to let them hear what they want to hear. But if instead of asking for an opinion or an argument in favor of or against something, you ask simply to summarize the situation with the latest best practices based on documentation, it does a pretty good job at that.
The main reference document here is:
https://docs.github.com/en/actions/reference/security/secure-use#using-third-party-actions
But this really does matter more when you are dealing with third party actions. The actions curated by GitHub can generally be considered secure.