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: disable non-security dependabot updates #12487

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Jan 9, 2024

As discussed in a previous Contributor Meeting and last month on Slack.

Motivation

Modifications

Set open-pull-requests-limit: 0 in dependabot.yml for all our currently specified package ecosystems

minor style modification

  • gomod section had 0 space while the rest had 2 space indentation
    • now all are 2 space consistently

Verification

GH has no way to actually test this, but this is something I previously implemented in other repos that I have maintained (example).

- most of the automated updates from dependabot cause a lot of noise and use up CI time, without adding much
  - most often are small patch updates that don't affect our usage of deps
  - some can also cause a lot of breakage when they pass CI but break something in a way that doesn't have an automated test

- instead of unnecessary automated updates, we can just manually update deps when needed (when we need a feature or patch) and set a reminder every 6 months to check in
  - IMO, I think this would actually end up as less work than monitoring all the dependabot updates

- Note that this intentionally _does not_ impact security updates. Security updates will still happen automatically
  - Per the [linked doc](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#specifying-dependencies-and-versions-to-ignore):
    > When used alone, the `ignore.versions` key affects both Dependabot updates, but the `ignore.update-types` key affects only Dependabot version updates.
    - that is why I specifically used only `ignore.update-types`

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- `gomod` section had 0 space while the rest had 2 space indentation
  - now all are 2 space consistently

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies github_actions Pull requests that update Github_actions dependencies go Pull requests that update Go dependencies area/build Build or GithubAction/CI issues javascript Pull requests that update Javascript dependencies labels Jan 9, 2024
- this also does not impact security updates and is a simpler configuration
  - per the [linked docs](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#open-pull-requests-limit):
    > This option has no impact on security updates, which have a separate, internal limit of ten open pull requests.

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added the prioritized-review For members of the Sustainability Effort label Jan 14, 2024
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

I think they are still useful though. We can catch issues earlier and disable certain dependency's upgrade when necessary.

@terrytangyuan
Copy link
Member

most of the automated updates from dependabot cause a lot of noise and use up CI time

I am not bothered by that. The builds run on weekends and we have plenty of CI time.

@agilgur5
Copy link
Contributor Author

I think they are still useful though.

There is a trade-off though; per my PR description, I don't think they are more useful than they are harmful or impeding.

We can catch issues earlier and disable certain dependency's upgrade when necessary.

Currently non-security updates from dependabot have a record of causing issues rather than catching them.
Disabling updates is very reactive -- we do so only after dependabot created a bug.

In previous discussions, I had also mentioned the alternatives of disabling all devDeps (only possible for NPM in our current usage as go.mod does not distinguish unfortunately) and disabling all major updates. @terrytangyuan what do you think about those alternatives?

Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

I am in favour of this approach as well, I think it's much safer to only update deps when it poses a security threat.

It makes updating the nix hash easier (can be done every 6 months), currently the hash is outdated because of dependabot (the hash has to be updated manually).

@agilgur5 agilgur5 requested a review from Joibel January 17, 2024 18:59
@juliev0
Copy link
Contributor

juliev0 commented Jan 17, 2024

Hey @terrytangyuan - are you okay with being the Assignee on this one?

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Sounds good

@blkperl
Copy link
Contributor

blkperl commented Apr 10, 2024

@agilgur5 do you want to try grouped updates instead? We can have dependabot update all the dependencies weekly or monthly. The reason we enabled dependabot originally is because nobody was keeping them up to date and fixing 1 year of interface changes all at once was not fun.

example:

  - package-ecosystem: "gomod"
    directory: "/"
    schedule:
      interval: "weekly"
    groups:
      dependencies:
        patterns:
          - "*"
    ignore:
      - dependency-name: "k8s.io/*"
      - dependency-name: "sigs.k8s.io/*"

https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups

@agilgur5
Copy link
Contributor Author

fixing 1 year of interface changes all at once was not fun.

Fundamentally, dependabot does not solve this: dependabot makes no interface changes what-so-ever.
That is in fact the problem that this PR solves, as dependabot would keep making PRs with major bumps with breaking changes but no interface changes that often resulted in breaking CI, or worse, releases, and requiring reverts.
Some of your PRs replaced dependabot PRs with breakage that fortunately were caught by CI. You still had to manually update those deps though, so the dependabot PRs did not give any benefit, and perhaps did the opposite as they all had to be manually replaced and closed with reasons etc.

And when it didn't make breaking changes but only patches or minor bumps, those were often not useful and just added noise, especially when there were test flakes.

The signal-to-noise ratio since this was implemented has been substantially higher, nearly every dep update is both useful (EoL version, CVEs, bugfixes, etc) and/or has interface changes in the same PR now. Before this the ratio was in the opposite direction, mostly noise and little signal.

do you want to try grouped updates instead?

Grouped updates don't change the above.

or monthly

Per the linked discussions, we also discussed the update frequency and had decided on every 6 months unless otherwise done earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues github_actions Pull requests that update Github_actions dependencies go Pull requests that update Go dependencies javascript Pull requests that update Javascript dependencies prioritized-review For members of the Sustainability Effort type/dependencies PRs and issues specific to updating dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants