[PR] fix(cluster): let the backend resolve registry config targets [rocketmq-dashboard]

["unbridled-41 (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(cluster): scope the config update to the instance that owns the registry row [rocketmq-dashboard]

["github-actions[bot] (via GitHub)" <gi...@apache.org>]

github-actions[bot] commented on PR #4559:
URL: https://github.com/apache/rocketmq-dashboard/pull/4559#issuecomment-5963534428

   This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated changes.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(cluster): let the backend resolve registry config targets [rocketmq-dashboard]

["unbridled-41 (via GitHub)" <gi...@apache.org>]

unbridled-41 commented on PR #4559:
URL: https://github.com/apache/rocketmq-dashboard/pull/4559#issuecomment-5746350238

   ## Verification evidence
   
   **CI (current head)**: this PR's head SHA `0057bd5b0c2b2aa162abbb4bec966b78419ad428` has exactly one workflow run — "CI" (pull_request), run id 35478298552, conclusion `startup_failure`, and 0 check-runs / 0 statuses on the head commit. Upstream's CI workflow fails at startup for **every** branch and PR in this repository (including maintainers'), i.e. no PR gets CI execution; local verification below substitutes for it.
   
   **Red (baseline source, new tests kept)**: restored `origin/master`'s `web/src/pages/cluster/index.tsx` while keeping the new test file, then `npx vitest run src/pages/cluster/__tests__/ClusterPage.test.tsx --testTimeout 60000` → `Test Files 1 failed (1)`, `Tests 6 failed | 23 passed (29)`. Failures are the new/updated assertions (payload still carrying `instanceId`, diff calls carrying the route instance).
   
   **Green**: same command with the fix → `Test Files 1 passed (1)`, `Tests 29 passed (29)`.
   
   **Type check / lint / build**: `npx tsc -b` exit 0; `npx eslint src/pages/cluster/index.tsx src/pages/cluster/__tests__/ClusterPage.test.tsx` exit 0; `npm run build` → `✓ built in 9.83s`.
   
   **Full web suite**: `npx vitest run --testTimeout 60000` → 1037 tests, 2 failed test files / 4 failed tests, all in files this PR does not touch (ConsumerPage ×2, MetricsExplorer, NotificationDeliveries — the load-fragile set recorded in prior rounds); running those three files in isolation → **62/62 passed**, confirming load fragility rather than a regression. Full-suite totals on this branch: 1037 tests (upstream master 1032 + 5 net new/updated regression assertions).
   
   **Diff self-check**: `git show --numstat 0057bd5b` → `web/src/pages/cluster/index.tsx +11/−3`, `web/src/pages/cluster/__tests__/ClusterPage.test.tsx +113/−9`. Two files, no unrelated changes; the −3/−9 are the previous route-instance expectations the fix replaces.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(cluster): scope the config update to the instance that owns the registry row [rocketmq-dashboard]

["github-actions[bot] (via GitHub)" <gi...@apache.org>]

github-actions[bot] closed pull request #4559: fix(cluster): scope the config update to the instance that owns the registry row
URL: https://github.com/apache/rocketmq-dashboard/pull/4559


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org