You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "coder999o (via GitHub)" <gi...@apache.org> on 2026/08/25 01:24:08 UTC

[PR] [Studio] feat: preview broker config updates before applying [rocketmq-dashboard]

coder999o opened a new pull request, #2552:
URL: https://github.com/apache/rocketmq-dashboard/pull/2552

   ## What is the purpose of the change
   
   Studio could update broker configuration across the discovered brokers, but operators had to submit the change before seeing the effective broker properties, target broker addresses, or field-level differences.
   
   This change adds a broker config preview path so users can inspect the effective update impact from the Studio console before applying it.
   
   ## Brief changelog
   
   - Added a Studio backend preview endpoint for cluster broker config updates.
   - Reused the backend broker-property mapping for both preview and apply paths.
   - Added a Broker config modal preview action showing target brokers, broker properties, and field-level changes.
   - Added backend controller/service tests plus frontend API/service/page interaction tests.
   
   ## Verifying this change
   
   - `mvn -q -Dtest=ClusterServiceTest,ClusterControllerTest test`
   - `mvn -q checkstyle:check -DskipTests`
   - `npm test -- --run src/api/cluster.test.ts src/services/clusterService.test.ts src/pages/cluster/__tests__/ClusterPage.test.tsx`
   - `npx eslint src/api/cluster.ts src/api/cluster.test.ts src/services/clusterService.ts src/services/clusterService.test.ts src/pages/cluster/index.tsx src/pages/cluster/__tests__/ClusterPage.test.tsx --max-warnings=0`
   - `npm run build`
   - `git diff --check`


-- 
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] [Studio] feat: preview broker config updates before applying [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #2552:
URL: https://github.com/apache/rocketmq-dashboard/pull/2552


-- 
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] [Studio] feat: preview broker config updates before applying [rocketmq-dashboard]

Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on code in PR #2552:
URL: https://github.com/apache/rocketmq-dashboard/pull/2552#discussion_r3848864051


##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterController.java:
##########
@@ -69,6 +70,12 @@ public Result<ClusterConfigUpdateResultVO> updateClusterConfig(@Valid @RequestBo
         return Result.ok(clusterService.updateClusterConfig(command));
     }
 
+    @PostMapping("/config/preview")

Review Comment:
   **[Critical]** Missing authorization check on preview endpoint.
   
   The `/clusters/config/preview` endpoint does not have any `@PreAuthorize` or similar security annotation. While this is a read-only operation, it exposes sensitive internal configuration details (broker addresses, current config values, cluster topology) to potentially unauthenticated users.
   
   **Recommendation:** Add the same authorization annotation as the `updateClusterConfig` endpoint to ensure only authorized users can preview cluster configurations.



##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterService.java:
##########
@@ -157,6 +161,26 @@ public void requireProxy(String clusterId, String addr) {
         requireProxy(cluster, addr);
     }
 
+    public ClusterConfigPreviewVO previewClusterConfig(UpdateConfigDTO command) {

Review Comment:
   **[Info]** Potential edge case in queue number validation
   
   The `requireMatchingDefaultQueueNums()` method validates that `writeQueueNums` and `readQueueNums` match when both are provided. However, if only one is set, the validation passes. Both map to the same broker property `defaultTopicQueueNums`. Consider adding a comment explaining this behavior or validating that both must be set together.



-- 
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