dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.61k stars 981 forks source link

Dependabot ignoring group #8961

Open Drowze opened 7 months ago

Drowze commented 7 months ago

Is there an existing issue for this?

Package ecosystem

bundler

Package manager version

2.5.4

Language version

Ruby 3.2

Manifest location and content before the Dependabot update

It's a private project with a lot of private gems (private gems being hosted GitHub Packages), but here's some relevant bits:

Gemfile

source "https://rubygems.org"

ruby File.read(File.join(__dir__, ".ruby-version")).strip

source "https://rubygems.pkg.github.com/Over-haul" do
  gem "gem_a"
  gem "gem_b"
  gem "gem_c"
end

gem "rails", "~> 7.1.3"
gem "foundation-rails", "5.4.5.0"
gem "wicked_pdf", "~> 1.4.0"
gem "graphql", "~> 2.0.28"
gem "karafka"

source "https://REDACTED@gems.karafka.io" do
  gem "karafka-license", "1.abc123" # pro version
end

gem "psych", "~> 3.3.4"

group :test do
  gem "rspec-rails"
  gem "diff-lcs"
  gem "karafka-testing"
end

dependabot.yml content

untouched except for internal-library names

version: 2
registries:
  ruby-github:
    type: rubygems-server
    url: https://rubygems.pkg.github.com/Over-haul/github_api
    token: ${{ secrets.PAT }}

updates:
  - package-ecosystem: "bundler"
    directory: "/"
    insecure-external-code-execution: allow
    registries:
      - ruby-github
    schedule:
      # By default, this is on Monday
      interval: "weekly"
    commit-message:
      prefix: "[Dependabot]"
    pull-request-branch-name:
      separator: "-"
    labels:
      - dependencies
      - ruby
    groups:
      karafka:
        patterns:
          - "karafka*"
          - "waterdrop"
      internal-libraries:
        patterns:
          - "gem_a"
          - "gem_b"
      dev-dependencies:
        dependency-type: "development"
      prod-dependencies:
        dependency-type: "production"

    ignore:
      # Our own gems
      - dependency-name: "gem_c"
      # Locked dependencies
      - dependency-name: "foundation-rails"
      # Locked dependencies (patch version upgrade allowed)
      - dependency-name: "psych"
        update-types: ["version-update:semver-minor"]
      - dependency-name: "graphql"
        update-types: ["version-update:semver-minor"]
      - dependency-name: "rails"
        update-types: ["version-update:semver-minor"]
      - dependency-name: "wicked_pdf"
        update-types: ["version-update:semver-minor"]
      # Allow only minor and patch version updates
      - dependency-name: "*"
        update-types: ["version-update:semver-major"]

Updated dependency

No response

What you expected to see, versus what you actually saw

I expected Dependabot to open separate PRs for Karafka whenever the update would contain either:

I also would NOT expect Dependabot to open a PR for the dev-dependencies group updating a gem that is specifically NOT in either the test or development gem group - but it did open the PR including karafka (which is not in any gem group)

But instead, Dependabot opened the dev-dependencies group PR updating karafka, karafka-testing and even waterdrop (though that's not in the PR description as it's not explicitly defined in the Gemfile)

Native package manager behavior

No response

Images of the diff or a link to the PR, issue, or logs

PR: Screenshot 2024-02-02 at 11 15 42

Diff: Screenshot 2024-02-02 at 11 19 12

Excerpt from relevant logs (job id 780006799):

updater | 2024/01/29 16:06:02 INFO <job_780006799> Starting job processing
updater | 2024/01/29 16:06:04 INFO <job_780006799> Starting grouped update job for Over-haul/repo
updater | 2024/01/29 16:06:04 INFO <job_780006799> Found 4 group(s).
updater | 2024/01/29 16:06:04 INFO <job_780006799> Starting update group for 'dev-dependencies'
...
updater | 2024/01/29 16:07:58 INFO <job_780006799> Checking if karafka-testing 2.2.2 needs updating
updater | 2024/01/29 16:07:58 INFO <job_780006799> Ignored versions:
updater | 2024/01/29 16:07:58 INFO <job_780006799>   version-update:semver-major - from .github/dependabot.yml
updater | 2024/01/29 16:07:58 INFO <job_780006799> Latest version is 2.3.0
updater | 2024/01/29 16:09:24 INFO <job_780006799> Requirements to unlock all
updater | 2024/01/29 16:09:24 INFO <job_780006799> Requirements update strategy bump_versions
updater | 2024/01/29 16:09:24 INFO <job_780006799> Updating karafka-testing, karafka
...
updater | 2024/01/29 16:10:25 INFO <job_780006799> Creating a pull request for 'dev-dependencies'
updater | 2024/01/29 16:10:39 INFO <job_780006799> Starting update group for 'internal-libraries'
...
updater | 2024/01/29 16:10:50 INFO <job_780006799> Nothing to update for Dependency Group: 'internal-libraries'
updater | 2024/01/29 16:10:50 INFO <job_780006799> Starting update group for 'karafka'
updater | 2024/01/29 16:10:50 INFO <job_780006799> Skipping karafka as it has already been handled by a previous group
updater | 2024/01/29 16:10:51 INFO <job_780006799> Checking if karafka-license 1.abc123 needs updating
updater | 2024/01/29 16:10:51 INFO <job_780006799> Ignored versions:
updater | 2024/01/29 16:10:51 INFO <job_780006799>   version-update:semver-major - from .github/dependabot.yml
updater | 2024/01/29 16:10:52 INFO <job_780006799> Latest version is 1.abc123
updater | 2024/01/29 16:10:52 INFO <job_780006799> No update needed for karafka-license 1.abc123
updater | 2024/01/29 16:10:52 INFO <job_780006799> Skipping karafka-testing as it has already been handled by a previous group
updater | 2024/01/29 16:10:52 INFO <job_780006799> Nothing to update for Dependency Group: 'karafka'
updater | 2024/01/29 16:10:52 INFO <job_780006799> Starting update group for 'prod-dependencies'
...
updater | 2024/01/29 16:23:14 INFO <job_780006799> Skipping karafka as it has already been handled by a previous group
...
updater | 2024/01/29 16:23:22 INFO <job_780006799> Skipping karafka-license as it has already been handled by a previous group
...
updater | 2024/01/29 16:23:28 INFO <job_780006799> Creating a pull request for 'prod-dependencies'
updater | 2024/01/29 16:23:39 INFO <job_780006799> Starting update job for Over-haul/repo
updater | 2024/01/29 16:23:39 INFO <job_780006799> Checking all dependencies for version updates...
...
updater | 2024/01/29 16:23:48 INFO <job_780006799> Finished job processing
updater | 2024/01/29 16:23:48 INFO Results:
updater | +------------------------------------------------------------------------------------------------------------------------------------+
updater | |                                                Changes to Dependabot Pull Requests                                                 |
updater | +---------+--------------------------------------------------------------------------------------------------------------------------+
updater | | created | rspec-rails ( from 6.1.0 to 6.1.1 ), karafka-testing ( from 2.2.2 to 2.3.0 ), karafka ( from 2.2.14 to 2.3.0 )           |
updater | | created | aws-sdk-kinesis ( from 1.54.0 to 1.55.0 ), aws-sdk-s3 ( from 1.142.0 to 1.143.0 ), prawn-svg ( from 0.33.0 to 0.34.0 ... |
updater | +---------+--------------------------------------------------------------------------------------------------------------------------+
updater | time="2024-01-29T16:23:48Z" level=info msg="task complete" container_id=job-780006799-updater exit_code=0 job_id=780006799 step=updater

Smallest manifest that reproduces the issue

No response

jurre commented 7 months ago

At a first glance it seems like this happens because karafka-testing depends on the karafka gem:

updater | 2024/01/29 16:09:24 INFO <job_780006799> Requirements to unlock all
updater | 2024/01/29 16:09:24 INFO <job_780006799> Requirements update strategy bump_versions
updater | 2024/01/29 16:09:24 INFO <job_780006799> Updating karafka-testing, karafka

In order to upgrade karafka-testing, bundler needs to also update karafka and there is not much that we can do to prevent bundler from doing that?

Drowze commented 7 months ago

Right, but shouldn't karafka-testing be grouped inside the karafka group? Since I'm defining the karafka group with a pattern which I believe should include karafka-testing:

    groups:
      karafka:
        patterns:
          - "karafka*"
          - "waterdrop"

So I wouldn't expect that karafka-testing is included in dev-dependencies group (even though it technically is a dev dependency, it should be matched first into the karafka group. By the way I also thought that the order of the groups are defined would be respected (note that I define the karafka group before the other groups), but doesn't look to be the case as dev-dependencies was checked before the karafka group 🤔

jurre commented 7 months ago

Yeah that's a good point, we currently don't respect the order in which groups are defined and that's a bug that we can and will fix. It's been on our radar. Right now the groups are getting processed in alphabetical order which is why dev-dependencies is going first, it's been on our radar and hoping to get to it soon! All in all I agree it leads to unexpected results and a frustrating experience in your scenario.

Drowze commented 7 months ago

Got it, thank you for confirming the issue @jurre - I understand this issue is related to the processing order of the groups. Looking forward to updates on it :)

Just to be clear, I assumed the order of the groups should be respected given this excerpt from the documentation: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#example-4

Dependabot creates groups in the order they appear in your dependabot.yml file. If a dependency update could belong to more than one group, it is only assigned to the first group it matches with.

Drowze commented 7 months ago

Update: I updated our dependabot.yml as such:

    groups:
      ...
      dev-dependencies:
        dependency-type: "development"
+       exclude-patterns:
+         - "karafka-testing"

That fixed the dev-dependencies PRs... But then Dependabot tried to rebase an existing prod-dependencies PR before creating a karafka group PR - which led to Dependabot creating a prod-dependencies PR with the karafka upgrade 😅

Screenshot 2024-02-08 at 17 34 50

Sounds like a tricky issue for Dependabot to fix 🤔

Drowze commented 4 months ago

Any updates to this? It looks like this is still an issue in our end.

Nishnha commented 4 months ago

Hi we fixed the issue around ordering with groups. The PRs in your test repo look to be correct to me https://github.com/Drowze/test-dependabot/pulls

What is broken on your private repo?

Drowze commented 4 months ago

Hello,

Sorry for the delay - I wasn't available for most of the week.

In our private repo we still see the issue, at least when Dependabot is rebasing an existing PR we have in our prod-dependencies group (note: this group has been being rebased since 26 March and wasn't merged yet - not sure if the issue will stop happening once we merge this PR 🤔).