unbridled-41 opened a new pull request, #4559:
URL: https://github.com/apache/rocketmq-dashboard/pull/4559
Fixes #3103.
## Problem / Evidence
Broker rows on the Cluster page's Broker tab come entirely from the global
NameServer registry (`listRegistryClusters`), while the page's config actions
(preview, update, and both config diffs) attached the page's currently selected
route instance (`selectedInstanceIdRef.current`) to every request. A NameServer
can be shared by zero, one, or many instances — the registry rows carry no
instance-owner column — so the route instance may be unrelated to the cluster
being written, and a preview/update/diff could be computed against the wrong
instance's provider and credentials (#3103, maintainer-confirmed).
## Root cause / Fix
Per the maintainer's review direction on #3151: ownership is resolved by the
backend. The frontend now sends only the cluster id and omits `instanceId` for
these three paths; the backend resolves the target authoritatively from the
cluster (`ClusterService.resolveCluster(id)`,
`BrokerConfigDiffService.compare(id, null)`,
`NameServerConfigDiffService.compare(clusterId)` — all already accept
`instanceId=null` and normalize blank values). The request-generation guards
from #3154 are unchanged. Zero or multiple endpoint matches are legitimate
states and are no longer treated as errors anywhere.
This is a clean single-commit replay of the reworked #3151 (head 2404fa6d,
maintainer-directed) onto the `master` trunk after #4379 closed the
`rocketmq-studio` branches; no merge commits.
## Priority & scoring
PRIORITY 83 = impact 35 (writes/diffs could target the wrong instance's
cluster — a cross-instance config write) + reach 12 (cluster page config
actions) + repro 18 (deterministic with a shared NameServer) + maintain 18
(maintainer-directed rework; closes the long-open #3103). FIX_CONFIDENCE 92
(backend already accepts the omitted field; the change only stops sending it).
## Tests
- New regression tests (red → green, verified on this branch by restoring
the baseline `index.tsx` while keeping the tests: `6 failed | 23 passed → 29
passed`):
- `routes registry config actions through the backend without an
instanceId` — preview/update payload carries no `instanceId`;
- `keeps registry config actions available when several instances share
the NameServer` — ambiguous endpoint is legitimate, no frontend rejection;
- `resolves config diffs server-side from the cluster id` — both diffs
call `get*ConfigDiff(clusterId)`;
- three prior assertions updated from `(..., 'instance-1')` to single-arg
calls.
- `npx vitest run src/pages/cluster/__tests__/ClusterPage.test.tsx
--testTimeout 60000` → **29 passed**.
- `npx tsc -b` clean; `npx eslint` on both changed files clean.
## Risk
Low. The backend behavior for `instanceId=null` is pre-existing and covered
(`normalizeInstanceId` → null; `resolveCluster(id)`); instances whose route
instance was previously correct are resolved the same way server-side, now from
the authoritative cluster→instance mapping instead of a client guess.
--
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]
