unbridled-41 opened a new pull request, #4889:
URL: https://github.com/apache/rocketmq-dashboard/pull/4889

   ## Problem
   
   On a role-backed Studio Instance (Tencent), the ACL read tools fail 
outright: `rmq.acl.list`, `rmq.acl.get`, `rmq.user.list` and `rmq.user.get` 
answer `INTERNAL_ERROR` (HTTP 500) as soon as the instance owns one role, and 
the caller gets no ACL data at all.
   
   ## Evidence
   
   
`server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/contract/acl/AclRuleItem.java:39`
 / `AclUserItem.java:33` (base `1ef5d860`) built the tool item's `id` from the 
numeric primary key only: `rule.getId() == null ? null : 
rule.getId().toString()`. Both records are `@JsonInclude(Include.NON_NULL)`, so 
a `null` id is **omitted from the payload**.
   
   The value is `null` for a role-backed instance by construction — 
`AclService.listRules`/`listUsers`/`getRule`/`getUser` dispatch to 
`TencentAclService` (`instance/acl/AclService.java:82,136,176`), whose 
projections never set an id:
   
   ```java
   // provider/tencent/TencentAclService.java:364,378
   return AclUserVO.builder().username(role.getRoleName())...build();   // no id
   return AclRuleVO.builder().principal(role.getRoleName())...build();  // no id
   ```
   
   The tool catalog requires it — `tool-catalog/tools/acl.yaml:57` and 
`user.yaml:32` declare item `required: [id, ...]` with `id: type: string` — and 
`ToolValidationFilter` validates every result through 
`ToolSchemaValidator.validateOutput`, which throws when the schema is violated. 
`ToolExecutionService.executeInternal` then maps the non-`BusinessException` to 
`ToolError.UNEXPECTED_EXECUTION_FAILURE` (`ToolError.java:91`, HTTP 500 / 
`INTERNAL_ERROR`).
   
   Reproduced by the new tests against the unfixed source (test kept, source 
reverted):
   
   ```
   java.lang.IllegalStateException: Tool output validation failed for 
rmq.acl.list: [/items/0: required property 'id' not found]
   java.lang.IllegalStateException: Tool output validation failed for 
rmq.user.get: [: required property 'id' not found]
   ```
   
   The binding is not the barrier: both tools are gated on `ACL_MANAGEMENT`, 
which `TencentInstanceProvider.capabilities()` declares 
(`provider/tencent/TencentInstanceProvider.java:152`), and an empty role list 
validates fine — so the failure appears exactly when there *is* something to 
list.
   
   The console treats the same `null` as an oversight and normalises it 
client-side (`web/src/pages/instance/acl.tsx:83,101`: `id: rule.id ?? 
rule.principal`, `id: user.id ?? user.username`), which is what the tool 
contract was missing.
   
   ## Root cause
   
   The tool DTOs assumed the numeric primary key of the Studio ACL tables, 
while the ACL service also projects rules and users from a cloud role store 
where the name is the identifier.
   
   ## Fix
   
   `AclRuleItem.from` / `AclUserItem.from` fall back to the rule principal / 
user name when there is no numeric id. That identifier is not cosmetic: 
`AclService.getRule` matches `id.equals(rule.getPrincipal())`, `getUser` 
matches `id.equals(user.getUsername())` and `deleteRule` passes the argument 
straight to the role-name-based `TencentAclService.deleteRule`, so a listed 
rule can be fetched or deleted with the id the list returned.
   
   Scope note: `rmq.acl.update` still requires a numeric id 
(`AclUpdateToolHandler.parseId` → `ACL_ID_INVALID` for a role name), so 
updating a role-backed rule through the tool layer remains unsupported. That is 
a separate limitation and is not addressed here.
   
   ## Tests
   
   - `mvn -o test 
-Dtest='org.apache.rocketmq.studio.ops.ai.tool.handler.acl.*Test,org.apache.rocketmq.studio.ops.ai.tool.handler.audit.*Test,org.apache.rocketmq.studio.instance.acl.*Test'`
 → **Tests run: 193, Failures: 0, Errors: 0 — BUILD SUCCESS** (includes the two 
new regressions: 
`AclListToolHandlerTest.executeShouldIdentifyARuleThatHasNoNumericId`, 
`UserGetToolHandlerTest.executeShouldIdentifyAUserThatHasNoNumericId`).
   - Both new tests fail on the unfixed source with the validation errors 
quoted above; the pre-existing 
`executeShouldSatisfyOutputSchemaWhenOptionalRuleFieldsAreNull` and the ACL 
service/controller suites stay green.
   
   ## Scoring
   
   PRIORITY 66 (impact 26: a documented tool family returns 500 instead of data 
on a supported provider; reach 10: role-backed instances only; reproducibility 
18: deterministic unit tests, no cloud account needed; maintenance value 12: 
restores the schema contract the tool layer enforces). FIX_CONFIDENCE 88.
   
   ## Risk
   
   Low. MySQL-backed instances are unaffected (`getId()` is set, so the 
fallback never fires and every existing assertion on numeric ids still passes). 
The fallback can only make an item that previously crashed output validation 
serializable; if a role name were ever null the schema would still reject it, 
which is the correct behaviour. No schema, controller or UI change.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to