Skip to content

[ISSUE #9966] Propagate policy type for ACL deletion#10428

Open
skt-shinyruo wants to merge 2 commits into
apache:developfrom
skt-shinyruo:issue-9966-delete-acl-policytype
Open

[ISSUE #9966] Propagate policy type for ACL deletion#10428
skt-shinyruo wants to merge 2 commits into
apache:developfrom
skt-shinyruo:issue-9966-delete-acl-policytype

Conversation

@skt-shinyruo

Copy link
Copy Markdown

Summary

  • add an optional policyType parameter to the ACL delete path in mqadmin, admin, client, and remoting request header
  • preserve existing behavior when policyType is not provided, while allowing mqadmin deleteAcl to target PolicyType=Default
  • add focused regression coverage in client, broker, and auth tests for deleting ACLs with explicit policy types

Root Cause

DeleteAclRequestHeader supports policyType, but the delete path from mqadmin to broker did not populate it. When the field is absent, broker-side delete handling falls back to PolicyType.CUSTOM, so deleting a DEFAULT policy entry cannot target the intended ACL rule.

Test Plan

  • mvn -o -pl tools -am -DskipTests compile
  • mvn -o -pl client -am -DfailIfNoTests=false -Dtest=MQClientAPIImplTest#testDeleteAcl+testDeleteAclWithPolicyType test
  • mvn -o -pl broker -am -DfailIfNoTests=false -Dtest=AdminBrokerProcessorTest#testDeleteAcl+testDeleteAclWithPolicyType test
  • mvn -o -pl auth -am -DfailIfNoTests=false -Dtest=AuthorizationMetadataManagerTest#deleteAcl test

Closes #9966

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 11.53846% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.96%. Comparing base (2a9560c) to head (bf03479).
⚠️ Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
...cketmq/tools/command/auth/DeleteAclSubCommand.java 0.00% 14 Missing ⚠️
...moting/protocol/header/DeleteAclRequestHeader.java 0.00% 4 Missing ⚠️
...he/rocketmq/tools/admin/DefaultMQAdminExtImpl.java 0.00% 3 Missing ⚠️
...apache/rocketmq/tools/admin/DefaultMQAdminExt.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10428      +/-   ##
=============================================
- Coverage      48.05%   47.96%   -0.09%     
+ Complexity     13303    13281      -22     
=============================================
  Files           1377     1377              
  Lines         100611   100634      +23     
  Branches       12991    12994       +3     
=============================================
- Hits           48347    48271      -76     
- Misses         46348    46419      +71     
- Partials        5916     5944      +28     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Bug title 无法删除 PolicyType为Default 的 acl

2 participants