RockteMQ-AI commented on PR #4342:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4342#issuecomment-5682164353
## 🤖 Automated Code Review
**PR:** [ISSUE #4341] Add Cross-Cluster Topic Configuration Synchronization
& Drift Rectifier
**Verdict: ❌ Critical Issues — Not Ready for Merge**
### 🔴 Critical: Service Returns Fabricated Comparison Data
The core service (`TopicCrossClusterSyncServiceImpl`) generates fake drift
data instead of querying real broker configurations:
```java
// Uses "baseline mock topic config" for comparison
log.debug("Using baseline mock topic config for comparison on topic: {}",
topic);
```
The comparison results are computed from hash-based fake values rather than
actual cluster state, meaning the drift detection would show non-existent
configuration differences or miss real ones.
### 🔴 Critical: `sync.do` Endpoint Does Not Perform Real Sync
The sync operation logs a success message but does not actually write
configuration to any target cluster. This is dangerous — operators might
believe they have synchronized configurations across clusters when no real
change has occurred.
### ⚠️ Other Issues
- **No integration with `MQAdminExt`** to read real broker configs
- **No authentication/authorization** for cross-cluster write operations
- **No validation** that target clusters exist or are reachable
- **No tests** for actual sync logic
### Recommendation
This PR needs:
1. Real cluster configuration queries via `MQAdminExt.getBrokerConfig()`
2. Actual configuration write operations with proper error handling
3. Authorization checks for cross-cluster modifications
4. Integration tests with embedded brokers
---
<sub>🔍 Automated review by RockteMQ-AI. Please verify suggestions before
applying.</sub>
--
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]
