Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[feat][broker] Implement allowBrokerOperationAsync in PulsarAuthorizationProvider to avoid exception thrown #23663

Merged

Conversation

mattisonchao
Copy link
Member

@mattisonchao mattisonchao commented Nov 29, 2024

Motivation

#23637 introduces a method called allowTopicPolicyOperationAsync. Although we kept the compatibility, the exception thrown will cost more CPU and give GC pressure.

Modifications

  • implemented superuser validation for PulsarAuthorizationProvider

Verifying this change

  • The existing test BrokerEndpointsAuthorizationTest is already covered.

  • Make sure that the change passes the CI checks.

Documentation

  • doc-not-needed

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 29, 2024
@mattisonchao mattisonchao self-assigned this Nov 29, 2024
@mattisonchao mattisonchao reopened this Nov 29, 2024
@lhotari lhotari changed the title [feat][broker] follow up #23637 to avoid exception thrown [feat][broker] Implement allowBrokerOperationAsync in PulsarAuthorizationProvider to avoid exception thrown Nov 29, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.36%. Comparing base (bbc6224) to head (350bef6).
Report is 771 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23663      +/-   ##
============================================
+ Coverage     73.57%   74.36%   +0.78%     
- Complexity    32624    34508    +1884     
============================================
  Files          1877     1944      +67     
  Lines        139502   147328    +7826     
  Branches      15299    16257     +958     
============================================
+ Hits         102638   109558    +6920     
- Misses        28908    29304     +396     
- Partials       7956     8466     +510     
Flag Coverage Δ
inttests 27.28% <0.00%> (+2.70%) ⬆️
systests 24.37% <0.00%> (+0.04%) ⬆️
unittests 73.75% <100.00%> (+0.90%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ker/authorization/PulsarAuthorizationProvider.java 68.70% <100.00%> (+0.08%) ⬆️

... and 668 files with indirect coverage changes

@lhotari lhotari merged commit 4603722 into apache:master Nov 29, 2024
75 of 81 checks passed
@lhotari lhotari added this to the 4.1.0 milestone Nov 29, 2024
lhotari pushed a commit that referenced this pull request Nov 29, 2024
…tionProvider to avoid exception thrown (#23663)

(cherry picked from commit 4603722)
mattisonchao added a commit that referenced this pull request Dec 4, 2024
…tionProvider to avoid exception thrown (#23663)

(cherry picked from commit 4603722)
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants