[ISSUE #10498] Update ACL documentation to include required 5.x properties#10499
[ISSUE #10498] Update ACL documentation to include required 5.x properties#10499SummCoder wants to merge 1 commit into
Conversation
In RocketMQ 5.x, enabling ACL requires more than just 'aclEnable=true'. The following properties are also required: - authenticationEnabled - authorizationEnabled - migrateAuthFromV1Enabled - authenticationMetadataProvider - authorizationMetadataProvider Without these, ACL either silently allows all traffic or throws 'authenticationMetadataProvider is not configured'.
RockteMQ-AI
left a comment
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
Updates ACL documentation to include required 5.x properties (admin, default.group.perm, etc.) that are mandatory in RocketMQ 5.x but were missing from the docs.
LGTM. Important documentation gap filled — users deploying ACL on 5.x need these properties. 👍
Automated review by github-manager-bot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #10499 +/- ##
=============================================
- Coverage 48.13% 48.05% -0.08%
+ Complexity 13355 13330 -25
=============================================
Files 1377 1377
Lines 100707 100707
Branches 13010 13010
=============================================
- Hits 48477 48399 -78
- Misses 46296 46365 +69
- Partials 5934 5943 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
SummCoder
changed the title
majialoong
left a comment
majialoong
left a comment
There was a problem hiding this comment.
Thanks for the patch! I left a few comments.
| ## RocketMQ 5.x 需要额外配置以下ACL属性 | ||
| authenticationEnabled=true | ||
| authorizationEnabled=true | ||
| migrateAuthFromV1Enabled=true |
There was a problem hiding this comment.
I think migrateAuthFromV1Enabled is not necessary here, since it is only required when migrating from ACL v1 to ACL v2.
| migrateAuthFromV1Enabled=true | ||
| authenticationMetadataProvider=org.apache.rocketmq.auth.authentication.provider.LocalAuthenticationMetadataProvider | ||
| authorizationMetadataProvider=org.apache.rocketmq.auth.authorization.provider.LocalAuthorizationMetadataProvider | ||
| listenPort=10911 |
There was a problem hiding this comment.
Maybe we should also add the default authentication and authorization providers here for completeness:
authenticationProvider=org.apache.rocketmq.auth.authentication.provider.DefaultAuthenticationProvider
authorizationProvider=org.apache.rocketmq.auth.authorization.provider.DefaultAuthorizationProvider
| ## RocketMQ 5.x requires the following additional ACL properties | ||
| authenticationEnabled=true | ||
| authorizationEnabled=true | ||
| migrateAuthFromV1Enabled=true |
RockteMQ-AI
left a comment
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
Updates both Chinese and English ACL documentation to include the six additional properties required for RocketMQ 5.x ACL configuration: authenticationEnabled, authorizationEnabled, migrateAuthFromV1Enabled, authenticationMetadataProvider, authorizationMetadataProvider.
Findings
- [Positive]
docs/cn/acl/user_guide.mdanddocs/en/acl/Operations_ACL.md— Both language versions are updated consistently. The new properties are added in the correct location within the broker configuration example. - [Positive] The prose is updated to reflect that these are 5.x-specific requirements, not just optional additions.
- [Info] The
migrateAuthFromV1Enabled=trueproperty is particularly important for users migrating from 4.x — it enables backward compatibility with the legacy ACL format.
Suggestions
- Consider adding a brief explanation of what each new property does, especially
migrateAuthFromV1Enabled. Users copying the config block without understanding may enable migration mode unnecessarily or forget to enable it when needed.
Verdict
LGTM. Addresses a real documentation gap that causes confusion for 5.x deployers.
Automated review by github-manager-bot
5 participants
Which Issue(s) This PR Fixes
Fixes #10498
Brief Description
The current ACL documentation (both English and Chinese) only lists
aclEnable=trueas the required broker property. In RocketMQ 5.x,this is insufficient — the new authentication/authorization framework
introduced in 5.x requires five additional properties to function.
Code evidence for each property:
Without these, users experience either:
How Did You Test This Change?
mvn -Prelease-all -DskipTests clean install -Uinvalid credentials correctly rejected with "User not found"
tools.ymlcredentialsmatching the ACL account
failure mode (silent bypass or configuration error)