Skip to content

[fix][sec] Remove dependency on out-dated commons-configuration 1.x #24562

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.

Sign up for GitHub

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

Jump to bottom
Merged
lhotari merged 2 commits into from
Jul 29, 2025
Merged

[fix][sec] Remove dependency on out-dated commons-configuration 1.x #24562

lhotari merged 2 commits into from
Jul 29, 2025

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Jul 25, 2025

Motivation

Pulsar currently has a dependency on commons-configuration:commons-configuration:1.10 which contains a low severity vulnerability CVE-2025-46392.

BookKeeper used commons-configuration until recently. After upgrading to BookKeeper 4.17.2, there's no longer a need to depend on commons-configuration in Pulsar.

Modifications

  • remove dependency and references to commons-configuration in Pulsar code base
  • add commons-logging to dependencyManagement since it's resolved version changed after commons-configuration was removed.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 4.1.0 milestone Jul 25, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 25, 2025
@lhotari lhotari requested review from dao-jun, freeznet and merlimat July 25, 2025 12:37
david-streamlio
david-streamlio approved these changes Jul 25, 2025
View reviewed changes
Copy link
Contributor

@david-streamlio david-streamlio left a comment

Choose a reason for hiding this comment

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

LGTM

Apurva007
Apurva007 approved these changes Jul 25, 2025
View reviewed changes
Copy link
Contributor

@Apurva007 Apurva007 left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari lhotari force-pushed the lh-remove-commons-configuration-1.x branch from 87173ba to 7cffcf0 Compare July 29, 2025 11:09
@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.34%. Comparing base (bbc6224) to head (7cffcf0).
⚠️ Report is 1250 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24562      +/-   ##
============================================
+ Coverage     73.57%   74.34%   +0.77%     
- Complexity    32624    32681      +57     
============================================
  Files          1877     1880       +3     
  Lines        139502   146414    +6912     
  Branches      15299    16784    +1485     
============================================
+ Hits         102638   108851    +6213     
- Misses        28908    28936      +28     
- Partials       7956     8627     +671
Flag Coverage Δ
inttests 26.70% <ø> (+2.12%) ⬆️
systests 23.35% <ø> (-0.97%) ⬇️
unittests 73.84% <ø> (+1.00%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1113 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lhotari lhotari merged commit 2e77444 into apache:master Jul 29, 2025
95 of 97 checks passed
lhotari added a commit that referenced this pull request Jul 29, 2025
@lhotari
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (#…
0510a9f 
…24562)

(cherry picked from commit 2e77444)
lhotari added a commit that referenced this pull request Jul 29, 2025
@lhotari
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (#…
eca0fae 
…24562)

(cherry picked from commit 2e77444)
@lhotari
Copy link
Member Author

lhotari commented Jul 29, 2025

BookKeeper 4.16.7 used in branch-3.0 continues to depend on commons-configuration so this PR cannot be cherry-picked to branch-3.0.

priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 29, 2025
@lhotari @priyanshu-ctds
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (a…
3aa3a8c 
…pache#24562)

(cherry picked from commit 2e77444)
(cherry picked from commit 0510a9f)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 29, 2025
@lhotari @srinath-ctds
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (a…
eb57fe8 
…pache#24562)

(cherry picked from commit 2e77444)
(cherry picked from commit 0510a9f)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 30, 2025
@lhotari @srinath-ctds
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (a…
4eb0d98 
…pache#24562)

(cherry picked from commit 2e77444)
(cherry picked from commit 0510a9f)
@snicoll
Copy link

snicoll commented Aug 7, 2025

Is it intended that upgrading to commons-configuration 2.x "leaks" commons-logging to the classpath? It did not before so it comes as a surprise in a patch release.

@snicoll snicoll mentioned this pull request Aug 7, 2025
Closed
@lhotari
Copy link
Member Author

lhotari commented Aug 7, 2025

Is it intended that upgrading to commons-configuration 2.x "leaks" commons-logging to the classpath? It did not before so it comes as a surprise in a patch release.

Thanks for the feedback @snicoll. Yes, this is a regression. It seems that it should be excluded in pulsar-common in the similar way as commons-configuration 1.x was excluded.

@lhotari lhotari mentioned this pull request Aug 7, 2025
Closed
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request Aug 14, 2025
@lhotari @poorbarcode
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (a…
b17a786 
…pache#24562)
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
@lhotari @KannarFr
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (a…
5824c98 
…pache#24562)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
@lhotari @walkinggo
[fix][sec] Remove dependency on out-dated commons-configuration 1.x (a…
acb9052 
…pache#24562)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nodece nodece nodece approved these changes

@dao-jun dao-jun dao-jun approved these changes

@david-streamlio david-streamlio david-streamlio approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

@BewareMyPower BewareMyPower Awaiting requested review from BewareMyPower

@merlimat merlimat Awaiting requested review from merlimat

@freeznet freeznet Awaiting requested review from freeznet

+1 more reviewer

@Apurva007 Apurva007 Apurva007 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cherry-picked/branch-3.3 cherry-picked/branch-4.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.3.8 release/4.0.6

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

7 participants

@lhotari @codecov-commenter @snicoll @Apurva007 @nodece @dao-jun @david-streamlio