Skip to content

fix(acl): align ACL 2.0 binding type validation with the policy service - #2927

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-acl2-boundtype
Open

fix(acl): align ACL 2.0 binding type validation with the policy service#2927
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-acl2-boundtype

Conversation

@yyqdbngt

@yyqdbngt yyqdbngt commented Sep 1, 2026

Copy link
Copy Markdown

Summary

  • align Acl2PolicyContext.validate() with the operational AclService#validateAcl2Policy binding-type vocabulary
  • require a non-blank boundType and accept TOPIC, GROUP, *, USER, SERVICE_ACCOUNT case-insensitively
  • add regression coverage for the model validator's accepted and rejected binding types

Why

Two validators for the same ACL 2.0 policy concept disagreed: the model validator only accepted exactly-cased USER/GROUP/SERVICE_ACCOUNT and silently let a null boundType through, while the service validator (covered by AclServiceTest) requires a non-blank type and accepts the wider TOPIC/GROUP/*/USER/SERVICE_ACCOUNT set case-insensitively. A policy the service accepts ("topic", "user") failed the model's validate() with a misleading exception, and a null binding type passed the model but failed the service.

Testing

  • cd server && mvn -q -Dtest=Acl2PolicyContextTest test — 11 tests pass (new class)
  • cd server && mvn -q -Dtest=AclServiceTest test — all tests pass (regression for the aligned vocabulary)

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Clean fix that properly aligns the model validator with the service validator's binding-type vocabulary.

Observations

  • Correctness ✓ — The null/blank guard (boundType == null || boundType.trim().isEmpty()) correctly prevents the previous silent pass-through of null values.
  • Case-insensitive matching via toUpperCase(Locale.ROOT) is the right approach — locale-safe and consistent with the service layer.
  • Tests ✓ — Good parameterized coverage for both accepted and rejected types, including edge cases like whitespace-only strings.

Minor Note

  • java.util.Locale is fully qualified inline — consider adding an import for consistency with the rest of the file, but this is purely stylistic.

Automated review by github-manager-bot

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.

3 participants