zhaohai666 opened a new pull request, #1037:
URL: https://github.com/apache/rocketmq-dashboard/pull/1037
# [RIP-1] Fix v2.1.0 mandatory defects (#380 #381 #390 #401 #402 #403 #407)
## Background
The RocketMQ Studio dashboard carries seven mandatory defects identified
against the v2.1.0 baseline.
This PR implements concrete, unit-tested fixes for each one on top of the
`rocketmq-studio` branch.
All changes are minimal and scoped: no functional behaviour outside these
defects is touched, and the
ACL 2.0 cluster-config endpoints belonging to PR #792 are intentionally left
alone (see the note at
the end).
**Base branch:** `upstream/rocketmq-studio` at `a8d263e`. The branch was
rebased onto the latest
`rocketmq-studio` (which includes #1025 "harden RocketMQ metadata semantics,
client contracts and
admin error reporting"); one import-only merge conflict in
`RocketMQAdminClientImpl.java` was resolved
by keeping both `MQClientException` (from #1025) and `MQBrokerException`
(added by this PR).
## Commits
```
70b42a5 feat: validate ACL 2.0 policy with IP range matcher (#403 #407)
51e888e fix: stop swallowing -1 consumer lag and add proxy fallback resolver
(#380 #381)
28d1aa2 fix: accept JSON body in addProxyAddr (#390)
710bfde test: assert default relative /api base url (#401)
136afde fix: add proxy fallback policy for unsupported broker request codes
(#402)
3be973a fix: make ACL IP range validation DNS-independent (#407)
```
## Per-defect summary
| Issue | Symptom | Studio-side root cause | Fix | Files changed | Tests
added |
|-------|---------|------------------------|-----|---------------|-------------|
| #380 | Consumer group progress shows `NOT_CONSUME_YET` even after messages
are consumed | `getGroupProgress` clamped `brokerOffset - consumerOffset` with
`Math.max(0, …)`, silently turning the broker's `-1` "unknown" into `0` |
`ConsumerLagResolver` keeps the raw diff and only falls back to a proxy when
one is wired in; `-1` is surfaced as the genuine unknown state |
`rocketmq/ConsumerLagResolver.java`, `rocketmq/ProxyStatsProvider.java`,
`rocketmq/NoopProxyStatsProvider.java`,
`rocketmq/InMemoryProxyStatsProvider.java`,
`rocketmq/RocketMQMetadataProvider.java` | `ConsumerLagResolverTest.java` (5) |
| #381 | RocketMQ 5.0 gRPC consumers show `-1` lag/delay | Same clamping hid
the unknown sentinel | Proxy-backed lag resolution seam; with no proxy the
unknown `-1` is returned instead of a fabricated `0` | same as #380 | same as
#380 |
| #390 | Unable to add ProxyAddr from a JSON client | Backend only bound
`@RequestParam` (form); a JSON body was rejected | `addProxyAddr` now also
accepts an `@RequestBody Map` (form overload kept); service already rejects
blank addresses | `cluster/proxy/ProxyCompatController.java` |
`ProxyCompatControllerTest.java` (+1 JSON case) |
| #401 | docker-compose front-end API address resolves to `localhost` | n/a
— Studio already uses a relative `/api` default | Added a regression test
asserting the default is relative `/api` and never an absolute `localhost` URL
| `web/src/config.test.ts` (new) | `config.test.ts` (3) |
| #402 | Connecting to a 5.4.0 broker throws "request type 106/206 not
supported" | No handling for proxy-incompatible broker request codes | Pure,
tested `ProxyFallbackPolicy` (`isUnsupportedRequestCode`, `isProxyMode`,
`shouldFallback`) plus a guarded seam in `RocketMQAdminClientImpl` that logs
and returns a clear 501 instead of crashing |
`rocketmq/ProxyFallbackPolicy.java`, `rocketmq/RocketMQAdminClientImpl.java` |
`ProxyFallbackPolicyTest.java` (10) |
| #403 | ACL 2.0 model (`Acl2PolicyContext`) not usable | Model existed but
was not wired into any service | `AclService.validateAcl2Policy(...)` validates
`policyName`, `boundType`, non-null `rules`, and each `whiteSet` entry via the
IP matcher | `instance/acl/AclService.java` | `AclServiceTest.java` (+33 cases
incl. IP-range validation) |
| #407 | ACL IP whitelist: bare `0.0.0.0` did not wildcard all IPs;
malformed entries were not reliably rejected | Exact-string match, and
`isValidRange` used `InetAddress.getByName` which performs DNS resolution (a
non-IP string could be resolved instead of rejected) | New `IpRangeMatcher`
(`isInRange`, `isValidRange`) supporting `0.0.0.0`/`0.0.0.0/0`/`::/0`
wildcards, exact IPv4, and CIDR subnet matching; validation uses a strict
dotted-quad IPv4 literal parser (no DNS dependency) so malformed input always
returns `false` | `instance/acl/IpRangeMatcher.java` |
`IpRangeMatcherTest.java` (13) |
## Testing
Backend (offline Maven, JDK 21):
```
cd server &&
JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk-21.jdk/Contents/Home \
mvn -o -q test
-Dtest=IpRangeMatcherTest,ConsumerLagResolverTest,ProxyCompatControllerTest,ProxyFallbackPolicyTest,AclServiceTest,ProxyAddressServiceTest
```
Result: **74 tests run, 0 failures** (IpRangeMatcherTest 13,
ConsumerLagResolverTest 5,
ProxyCompatControllerTest 4, ProxyFallbackPolicyTest 10, AclServiceTest 33,
ProxyAddressServiceTest 9).
Frontend type check:
```
cd web && node_modules/.bin/tsc -b
```
Result: **exit 0**.
Frontend regression tests:
```
cd web && node_modules/.bin/vitest run src/config.test.ts
src/api/proxy.test.ts
```
Result: **8 tests passed** (config.test.ts 3, proxy.test.ts 5).
## Note on ACL 2.0 endpoints
The full ACL 2.0 functional change (#792, unmerged) owns the cluster-config
endpoints. This PR does
**not** add those endpoints. Instead it makes the existing
`Acl2PolicyContext` model operational by
adding `AclService.validateAcl2Policy(...)`, which validates policy
structure and IP ranges so the
model can be consumed by callers today without duplicating #792.
--
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]
