Skip to content

Group Dependabot PRs and remove merge-dependabot - #3990

Merged
ppkarwasz merged 1 commit into
2.xfrom
fix/2.x/remove-dependabot
Dec 11, 2025
Merged

Group Dependabot PRs and remove merge-dependabot#3990
ppkarwasz merged 1 commit into
2.xfrom
fix/2.x/remove-dependabot

Conversation

@ppkarwasz

Copy link
Copy Markdown
Member

This change adds grouping to all dependabot configurations to limit the number of open PRs to one.

It also removes the merge-dependabot workflow, which is no longer useful in case of grouped upgrades.

This change adds grouping to all dependabot configurations to limit the
number of open PRs to one.

It also removes the `merge-dependabot` workflow, which is no longer
useful in case of grouped upgrades.

@vy vy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This effectively means dependency updates will not contain a changelog entry anymore. I'm reluctant to accept this.

I thought we agreed to "disable dependabot" (i.e., delete dependabot.yaml) and restore it once the CI workflow is fixed. Have I misunderstood you?

@github-project-automation github-project-automation Bot moved this to Changes requested in Log4j pull request tracker Dec 1, 2025
@ppkarwasz

ppkarwasz commented Dec 3, 2025

Copy link
Copy Markdown
Member Author

I prefer to keep Dependabot alive as a reminder. Having a partial Dependabot PR is better than having no PR at all.

Until we solve the blocker for Dependabot automation, we can always complete those PRs ourselves or I can share a short Python script to generate the entries.

@ppkarwasz
ppkarwasz requested a review from vy December 5, 2025 14:35
@vy

vy commented Dec 11, 2025

Copy link
Copy Markdown
Member

I prefer to keep Dependabot alive as a reminder. Having a partial Dependabot PR is better than having no PR at all.

I'm not very keen on this idea, since it will take a single click for a maintainer to approve such a PR, and effectively lose the trail of updates, which are necessary to generate changelog entry files.

Until we solve the blocker for Dependabot automation, we can always complete those PRs ourselves or I can share a short Python script to generate the entries.

Can you put that in a Groovy script that gets executed in a dedicated logging-parent profile, that is, ./mvnw -P generate-dependabot-changelog apache/logging-log4j2 12345?

@vy vy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I will implement the logging-parent script myself – you can proceed.

@github-project-automation github-project-automation Bot moved this from Changes requested to Approved in Log4j pull request tracker Dec 11, 2025
@ppkarwasz
ppkarwasz merged commit a6bc86e into 2.x Dec 11, 2025
11 checks passed
@ppkarwasz
ppkarwasz deleted the fix/2.x/remove-dependabot branch December 11, 2025 19:47
@github-project-automation github-project-automation Bot moved this from Approved to Merged in Log4j pull request tracker Dec 11, 2025
ppkarwasz added a commit that referenced this pull request Dec 11, 2025
This change restores a valid Dependabot configuration that was broken in #3990 (see the failing GHA run: [57824321555](https://github.com/apache/logging-log4j2/runs/57824321555)).

Because GitHub does not validate Dependabot configuration files before merge, the fix is based **entirely** on the documented format and examples in the [Dependabot options reference](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/dependabot-options-reference#exclude-paths-).

**Note:** Dependabot appears to interpret the `exclude-paths` key differently from `directories`; unlike `directories`, `exclude-paths` does **not** require a leading slash.
ppkarwasz added a commit that referenced this pull request Dec 11, 2025
This change restores a valid Dependabot configuration that was broken in #3990 (see the failing GHA run: [57824321555](https://github.com/apache/logging-log4j2/runs/57824321555)).

Because GitHub does not validate Dependabot configuration files before merge, the fix is based **entirely** on the documented format and examples in the [Dependabot options reference](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/dependabot-options-reference#exclude-paths-).

**Note:** Dependabot appears to interpret the `exclude-paths` key differently from `directories`; unlike `directories`, `exclude-paths` does **not** require a leading slash.
@ppkarwasz

Copy link
Copy Markdown
Member Author

Can you put that in a Groovy script that gets executed in a dedicated logging-parent profile, that is, ./mvnw -P generate-dependabot-changelog apache/logging-log4j2 12345?

I’m concerned that, for security reasons, we cannot rely on a Groovy script to generate the changelog entries.

Regardless of whether we split the workflow into two steps (which would indeed make it easier to review), changelog generation effectively consists of two distinct phases:

  1. Reading Dependabot changes (unprivileged)
    This step can safely use a Groovy script, relying on mvnw, Maven and the Maven Groovy Plugin, provided it runs as pull_request.

  2. Writing changelog entries to the repository (privileged)
    Once the PR commit is checked out, we must be careful not to execute any script or dependency defined in the repository itself. Concretely:

    • We should not run ./mvnw to determine the major version; that value can and should be passed explicitly as an argument.
    • More importantly, we must not run ./mvnw to execute a Groovy script whose dependencies or versions could have been modified in the PR.

For this reason, using a Groovy script in the privileged part of the workflow is not something I would be comfortable with.

vy added a commit to apache/logging-log4j-scala that referenced this pull request Dec 29, 2025
@vy vy added this to the 2.26.0 milestone Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants