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

ci: fix dependabot for audit and cont_integration workflows #1377

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

Conversation

notmandatory
Copy link
Member

@notmandatory notmandatory commented Mar 16, 2024

Description

We are getting errors on dependabot generated PRs. See:

https://github.com/bitcoindevkit/bdk/actions/runs/8310897352
https://github.com/bitcoindevkit/bdk/actions/runs/8310897348

To fix this one of the recommended solutions is to exclude push triggered workflow runs for the dependabot created PR branches, push trigger is sufficient. See: Error: 403 "Resource not accessible by integration".

Notes to the reviewers

An alternative fix is to remove the push trigger but this seemed like a smaller change and the audit workflow only uses the push trigger and I'm not sure if it will break if we switch it to use pull_request instead.

It's a little annoying having dependabot create PRs but since it only triggers on security related issues it's worth having so we don't forget to do those updates.

Changelog notice

None.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@notmandatory notmandatory self-assigned this Mar 16, 2024
@notmandatory notmandatory added this to the 1.0.0-alpha milestone Mar 16, 2024
@notmandatory notmandatory marked this pull request as ready for review March 16, 2024 23:20
@notmandatory
Copy link
Member Author

@storopoli have you run into this issue before with dependabot and any thoughts on if this is the best way to fix it?

@notmandatory notmandatory modified the milestones: 1.0.0-alpha, 1.0.0-beta Mar 18, 2024
@storopoli
Copy link
Contributor

storopoli commented Mar 18, 2024

Digging a little on this one by looking at:

We can definitely add the ignore as you are doing,
or we can try to add the necessary permissions as well.
Something like this:

jobs:
  security_audit:
    runs-on: ubuntu-20.04
    permissions:
      security_events: write

I am fine with both approaches.
I think that security audits would be nice to have in dependabot PRs,
since it is primarily updating dependencies.
I am fond of having a Cargo.lock committed and updating it through dependabot,
see Dependabot cargo.
It will be much easier than the current pinning approach.

The Nix CI will add a Cargo.lock (#1320) and the next logical step,
which I was already planning,
was to add Cargo updates to dependabot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

None yet

2 participants