btlqql opened a new pull request, #5322:
URL: https://github.com/apache/rocketmq-dashboard/pull/5322
### Which Issue(s) This PR Fixes
- Fixes #5240
### Brief Description
`AclService`
(server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclService.java)
routed Tencent instances to `tencentAclService` and returned *before* the audit
call: `createRule`, `updateRule`, `deleteRule`, `createUser`, `updateUser` and
`deleteUser` each had a `if (isTencentInstance(instanceId)) { return
tencentAclService...; }` early return, and `TencentAclService` records nothing
itself. Creating, changing or revoking an ACL rule or user on a Tencent
instance therefore left no audit row at all, while the same operation on an
Apache instance produced `CREATE/UPDATE/DELETE_ACL_RULE` / `ACL_USER`. The
second half of the defect is that the Apache-path audits passed `clusterId =
null`, so even the rows that were written could not be attributed to an
instance.
The fix records the same audit vocabulary on both paths and carries the
instance context:
- the Tencent branches now audit after the provider call succeeds —
`createRule`/`updateRule` re-audit the returned `AclRuleVO`,
`createUser`/`updateUser` the returned `AclUserVO`, and the two delete methods
audit the id they were given (the role name for Tencent roles);
- `auditRule`/`auditUser` take the `instanceId` and pass it as the audit
`clusterId` (the existing failure convention is preserved: when the provider
throws, the exception propagates before the audit, so nothing is recorded);
- Tencent ACL VOs have no numeric database id (`TencentAclService` keeps the
role name in `principal`/`username` and leaves `id` null), so the resource
identifier falls back to the role name instead of the literal `"null"`.
### How Did You Test This Change?
Three new tests cover the Tencent path (rule creation, role creation, role
deletion) and one covers the Apache instance context; all four fail on the
unmodified tree.
Red run on the unmodified tree:
```
$ cd server && mvn -B -ntp test -Dtest=AclServiceTest
[ERROR]
AclServiceTest.createRuleShouldAuditTencentRoleMutationsWithTheRoleName:890
[ERROR]
AclServiceTest.createUserShouldAuditTheTencentRoleNameAndTheInstanceState:903
[ERROR]
AclServiceTest.deleteUserShouldAuditTencentRoleDeletionWithTheInstanceState:914
[ERROR]
AclServiceTest.updateRuleShouldAuditTheInstanceContextForApacheInstances:931
Argument(s) are different! Wanted:
operationAuditService.record("UPDATE_ACL_RULE", "ACL_RULE", "1", "apache-1",
"principal=user1", "SUCCESS", null);
Wanted but not invoked:
operationAuditService.record("CREATE_ACL_RULE", ...)
Actually, there were zero interactions with this mock.
[ERROR] Tests run: 82, Failures: 4, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
Green run after the fix (same command, same test class):
```
$ cd server && mvn -B -ntp test -Dtest=AclServiceTest
[INFO] Tests run: 82, Failures: 0, Errors: 0, Skipped: 0, Time elapsed:
3.538 s -- in org.apache.rocketmq.studio.instance.acl.AclServiceTest
[INFO] Tests run: 82, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
`mvn` also runs checkstyle at `validate`; both runs report `You have 0
Checkstyle violations.`
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits
- [x] Tests added or updated for non-trivial changes
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable, no UI text changed
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks) — no new dependency between packages, `instance/acl` only talks to
`audit` as before
- [x] New source files carry the ASF license header — no new source file
- [x] Documentation touched where behaviour changed (README / `docs/` /
in-app help) — not applicable: no API shape change, only the audit rows
written; `docs/api-spec.md` is owned by another 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]
