feat(ZSTAC-79206): management network IPv6 support#3864
Open
zstack-robot-1 wants to merge 8 commits intofeature-mgt-ip6from
Open
feat(ZSTAC-79206): management network IPv6 support#3864zstack-robot-1 wants to merge 8 commits intofeature-mgt-ip6from
zstack-robot-1 wants to merge 8 commits intofeature-mgt-ip6from
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Walkthrough此PR为系统引入全面的IPv6支持：新增IPv6工具、扩展管理节点IP/CIDR发现与验证以支持IPv6、调整多处组件的地址处理，并补充大量IPv6相关测试用例（管理节点、KVM、VXLAN、Ceph、NFS、控制台等）。 Changes
代码审查工作量🎯 4 (Complex) | ⏱️ ~75 分钟 诗歌
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Add IPv6 support for ZStack management network (milestone M1). New files: - IPv6Utils: buildUrl/bracketIpv6/normalizeIpv6/isValidManagementIp - NetworkGlobalConfig: management.server.prefer.ipv6 (default false) Modified: - Platform: getManagementServerIp() prefer IPv4/IPv6 via GlobalConfig; getManagementServerCidr() IPv6 CIDR; jgroupsAddr() JGroups bracket fix; volatile fields for JMM correctness - RESTFacadeImpl: callbackUrl uses IPv6Utils.buildUrl(); Pattern static final - HostApiInterceptor: accept + normalize IPv6 management IP - ApplianceVmFacadeImpl: getMnIpForVr() CIDR matching for dual-stack MN Tests (TP-001~031): IPv6UtilsCase, MnIpv6Case, KvmHostIpv6Case, ApplianceVmIpv6Case Resolves: ZSTAC-79206 Change-Id: I3fec17c211d84c82a84e6711b7d09eea
New utility methods: - IPv6Utils.buildAddr(ip, port): format Ceph monAddr with IPv6 brackets - NetworkUtils.isIpInCidr(ip, cidr): dual-stack CIDR matching F-010: KVMHost.getDataNetworkAddress() uses isIpInCidr for IPv6 storage CIDR F-011: CephBackupStorageMonBase/CephPrimaryStorageMonBase use IPv6Utils.buildAddr F-012: KVMHost.checkMigrateNetworkCidrOfHost() uses isIpInCidr for IPv6 migration F-013: VxlanPoolApiInterceptor accepts IPv6 VTEP addresses F-014a: ZSha2Helper.isMaster() grep pattern fixed for IPv6 VIP detection NFS: NfsApiParamChecker.isValidStorageCidr supports IPv6 CIDR format Resolves: ZSTAC-79206 Change-Id: Idc7022fbdfce429ca8795f3522df682b
MnIpv6StorageMigrationCase: TP-032~038 storage/migration CIDR dual-stack VxlanIpv6Case: TP-039~041 VXLAN vtep IPv6 support ZSha2Ipv6Case: TP-042~046 HA zsha2 IPv6 SSH/nginx/URL/grep Resolves: ZSTAC-79206 Change-Id: Ia4e1480db3514b9c9f9e0e24aff2310d
- KvmHostIpmiPowerExecutor: remove IPv4-only guard for IPMI (F-025) - ConsoleManagerImpl: bracketIpv6() for console proxy hostname (F-026) - KVMAgentCommands/KVMHost: vncListenAddress='::' for IPv6 host (F-027) Resolves: ZSTAC-79206 Change-Id: I3f8a901c7d2e4b6f5c1a8d3e9f2b7c0d1e4a5f6
Add MnIpv6M3Case and MnIpv6M4Case covering: - TP-062~069,076,077: IPMI IPv6, Console Proxy bracket, BM DPU callback, COLO qemu URL - TP-083~089: ZWatch InfluxDB/Prometheus/Grafana URL, License HTTPS URL, Keycloak container name, SSO CAS login URL Resolves: ZSTAC-79206 Change-Id: If19fdab6ef4f6a7f8d9a16fb97c1ee29365197b2
Resolves: ZSTAC-79206 Add TP number prefix to 59 Groovy test methods across 9 test case files so method names align with TP-001~096 traceability matrix in Func Spec. Change-Id: I9aee46759327162b1a0ed3da9277c9be695ae0d8
MatheMatrix
force-pushed
the
sync/shixin.ruan/shixin.ruan-ZSTAC-79206@@4
branch
from
bcfe36f to
c2a648d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
header/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.java-4-4 (1)
4-4:⚠️ Potential issue | 🟡 Minor请为接口方法补充 Javadoc。
Line [4] 的扩展点方法缺少注释，插件实现方很难准确理解参数语义与调用时机。
✍️ 建议修复
public interface AfterUpdateVmNicMacExtensionPoint { + /** + * Invoked after a VM NIC MAC address is updated. + * + * `@param` nic VM NIC inventory after update + * `@param` oldMac previous MAC address + * `@param` newMac new MAC address + */ void afterUpdateVmNicMac(VmNicInventory nic, String oldMac, String newMac); }As per coding guidelines:
**/*.java中“接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释”。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.java` at line 4, 在 AfterUpdateVmNicMacExtensionPoint 接口的方法 afterUpdateVmNicMac(VmNicInventory nic, String oldMac, String newMac) 上补充完整的 Javadoc：描述该回调的调用时机（例如何时在 VM NIC MAC 更新后触发）、每个参数的语义（nic 为更新后的 VmNicInventory、oldMac 为更新前的 MAC、newMac 为更新后的 MAC）以及实现方应承担的行为/约束（比如是否允许抛出异常或仅做无阻塞的处理、是否可以修改 nic 等）；保持方法签名不加多余修饰符，确保注释清晰示例化调用场景并使用标准的 `@param/`@throws/@since 标记。rest/src/main/resources/scripts/RestDocumentationGenerator.groovy-853-861 (1)
853-861:⚠️ Potential issue | 🟡 Minor必填字段校验可被“仅空白字符”绕过
Line [853]-Line [861] 目前用
isEmpty()判断，" "这类值会被当作已填写，导致校验漏过。建议改为判空白（trim 后为空也视为缺失）。建议修改
- if (markDown.desc_CN.isEmpty()) missingFields.add("desc_CN") - if (markDown.name_CN.isEmpty()) missingFields.add("name_CN") - if (markDown.valueRangeRemark.isEmpty()) missingFields.add("valueRangeRemark") - if (markDown.defaultValueRemark.isEmpty()) missingFields.add("defaultValueRemark") - if (markDown.resourcesGranularitiesRemark.isEmpty()) missingFields.add("resourcesGranularitiesRemark") - if (markDown.additionalRemark.isEmpty()) missingFields.add("additionalRemark") - if (markDown.backgroundInformation.isEmpty()) missingFields.add("backgroundInformation") - if (markDown.isUIExposed.isEmpty()) missingFields.add("isUIExposed") - if (markDown.isCLIExposed.isEmpty()) missingFields.add("isCLIExposed") + if (markDown.desc_CN == null || markDown.desc_CN.trim().isEmpty()) missingFields.add("desc_CN") + if (markDown.name_CN == null || markDown.name_CN.trim().isEmpty()) missingFields.add("name_CN") + if (markDown.valueRangeRemark == null || markDown.valueRangeRemark.trim().isEmpty()) missingFields.add("valueRangeRemark") + if (markDown.defaultValueRemark == null || markDown.defaultValueRemark.trim().isEmpty()) missingFields.add("defaultValueRemark") + if (markDown.resourcesGranularitiesRemark == null || markDown.resourcesGranularitiesRemark.trim().isEmpty()) missingFields.add("resourcesGranularitiesRemark") + if (markDown.additionalRemark == null || markDown.additionalRemark.trim().isEmpty()) missingFields.add("additionalRemark") + if (markDown.backgroundInformation == null || markDown.backgroundInformation.trim().isEmpty()) missingFields.add("backgroundInformation") + if (markDown.isUIExposed == null || markDown.isUIExposed.trim().isEmpty()) missingFields.add("isUIExposed") + if (markDown.isCLIExposed == null || markDown.isCLIExposed.trim().isEmpty()) missingFields.add("isCLIExposed")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around lines 853 - 861, The current required-field checks use isEmpty() on strings like markDown.desc_CN, markDown.name_CN, markDown.valueRangeRemark, etc., which treats strings of only whitespace as non-empty; change each check to treat null or whitespace-only as missing by replacing isEmpty() with a null-safe trim check (e.g., check that markDown.<field> is null or markDown.<field>.trim().isEmpty()) so fields containing only whitespace are added to missingFields; update all the listed symbols (desc_CN, name_CN, valueRangeRemark, defaultValueRemark, resourcesGranularitiesRemark, additionalRemark, backgroundInformation, isUIExposed, isCLIExposed) accordingly.utils/src/main/java/org/zstack/utils/network/NetworkUtils.java-561-563 (1)
561-563:⚠️ Potential issue | 🟡 Minor注释语言不符合仓库规范。
Line 562-563 的注释为中文，需改为英文注释。
As per coding guidelines:
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/src/main/java/org/zstack/utils/network/NetworkUtils.java` around lines 561 - 563, Replace the Chinese comment above the IP/CIDR check with an English-only comment; in the NetworkUtils class (the comment immediately preceding the method that checks whether an IP (IPv4 or IPv6) belongs to a specified CIDR), change the Chinese text to a clear English sentence such as: "Determines whether an IP (IPv4 or IPv6) belongs to the specified CIDR." Ensure there are no Chinese characters left in that comment.docs/modules/network/pages/networkResource/ZStackL2NetworkType.adoc-451-453 (1)
451-453:⚠️ Potential issue | 🟡 Minor
srcPath规则前后矛盾，需统一口径Line 451-453 写“OVN_DPDK 或 ZNS 会设置
srcPath”，但 Line 480 又写 “L2Geneve + ZNS 不设置”。两处描述冲突，容易导致实现/排障误判。🛠 建议修订示例
-当 VSwitchType 为 OVN_DPDK 或 ZNS 时，`completeNicInformation()` 会设置 -`srcPath = "/var/run/openvswitch/" + nic.getInternalName()`，用于 DPDK vhost-user socket 连接。 +当网卡类型为 `dpdkvhostuserclient` 且走 NoVlan/Vlan backend 时，`completeNicInformation()` 会设置 +`srcPath = "/var/run/openvswitch/" + nic.getInternalName()`，用于 DPDK vhost-user socket 连接。| L2GeneveNetwork | ZNS -| 否（仅 OVN_DPDK 设置） +| 否（当前 Geneve backend 不设置） | KVMRealizeL2GeneveNetworkBackendAlso applies to: 478-481
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/modules/network/pages/networkResource/ZStackL2NetworkType.adoc` around lines 451 - 453, The documentation contains contradictory rules about when completeNicInformation() sets srcPath: lines say VSwitchType OVN_DPDK or ZNS set srcPath = "/var/run/openvswitch/" + nic.getInternalName(), but later state L2Geneve + ZNS does not set it; reconcile and make a single authoritative rule. Update the text to consistently describe the srcPath behavior (e.g., "completeNicInformation() sets srcPath for OVN_DPDK and ZNS vSwitch types to '/var/run/openvswitch/' + nic.getInternalName(), except when L2Geneve explicitly overrides this behavior" or the opposite if the intent is that ZNS with L2Geneve does not set srcPath), and ensure all mentions of VSwitchType, OVN_DPDK, ZNS, L2Geneve, completeNicInformation(), and srcPath reflect that same rule.test/src/test/groovy/org/zstack/test/integration/utils/IPv6UtilsCase.groovy-7-92 (1)
7-92:⚠️ Potential issue | 🟡 Minor请将测试注释统一为英文。
Line 7-92 的注释说明为中文，建议改为英文，避免与仓库统一规范冲突。
As per coding guidelines
**/*.*:代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/utils/IPv6UtilsCase.groovy` around lines 7 - 92, The test file IPv6UtilsCase contains Chinese comments/documentation (class header and all test method javadoc/comments such as for testTP008_buildUrlIpv4, testTP009_buildUrlIpv6, testTP010_bracketIpv6Idempotent, testTP011_normalizeIpv6, testTP012_isValidMnIpLinkLocal, testTP013_isValidMnIpInvalid, and the TP-014 description); replace all Chinese text with clear, idiomatic English descriptions and assertions messages (preserve existing identifiers and intent) so comments and assertion messages follow the repository guideline of using English only.test/src/test/groovy/org/zstack/test/integration/network/vxlan/VxlanIpv6Case.groovy-12-104 (1)
12-104:⚠️ Potential issue | 🟡 Minor请将该测试文件中的中文注释与说明文案统一改为英文。
Line 12-104 出现了多处中文注释/说明（包括块注释与行内注释）。这会违反仓库的统一语言规范，建议统一替换为准确英文表述。
As per coding guidelines
**/*.*:代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/network/vxlan/VxlanIpv6Case.groovy` around lines 12 - 104, The file VxlanIpv6Case contains Chinese comments and messages throughout (class VxlanIpv6Case, methods testTP039_vxlanAcceptsIpv6VtepIp, testTP040_vtepVoColumnLength, checkVtepIpColumnLength, testTP041_invalidVtepIpRejected); replace all Chinese block and inline comments and any Chinese text in assertion messages or logger.info calls with clear, correct English equivalents while preserving the original meaning (e.g., describe tests TP-039/040/041, explain why IPv6/IPv4 checks are used, and note JPA `@Column` length expectation), keep identifiers and logic unchanged, and run tests to ensure no string-sensitive behavior is affected.utils/src/main/java/org/zstack/utils/network/IPv6Utils.java-16-118 (1)
16-118:⚠️ Potential issue | 🟡 Minor请将该工具类中的中文注释/Javadoc 统一改为英文。
Line 16-118 的注释与示例说明包含中文，建议统一改成英文以符合仓库规范并保持跨团队可读性。
As per coding guidelines
**/*.*:代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java` around lines 16 - 118, Replace all Chinese Javadoc/comments in the IPv6Utils class with clear English equivalents while preserving examples and semantics; update the Javadoc for the class and methods buildUrl, buildHttpsUrl, bracketIpv6, normalizeIpv6, buildAddr (both overloads) so comments, examples, parameter descriptions and the `@throws` text are all in correct, idiomatic English and mention behavior like adding brackets for IPv6, idempotency, zone ID stripping, RFC 5952 normalization, and rejection conditions for validate methods; keep existing example lines and formatting but translate their text and explanation to English and ensure no Chinese remains in any comment or error message.test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy-87-91 (1)
87-91:⚠️ Potential issue | 🟡 Minor
contains(":")不能证明返回值是合法 IPv6。任何带冒号的脏字符串都会让这个断言通过，TP-025 的覆盖会被高估。这里最好直接使用现有的 IPv4/IPv6 校验 helper。
Based on learnings
在 ZStack 的 NetworkUtils 类中，应使用 IPv6NetworkUtils 中的 isValidIpv4 和 isValidIpv6 方法来验证 IP 地址格式.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy` around lines 87 - 91, The current test uses selectedIp.contains(":") to detect IPv6 which is unsafe; change the validation in the ApplianceVmIpv6Case test (where callGetMnIpForVr(mnCidr) provides selectedIp) to use the proper validators: replace NetworkUtils.isIpv4Address(selectedIp) || selectedIp.contains(":") with a call to IPv6NetworkUtils.isValidIpv4(selectedIp) || IPv6NetworkUtils.isValidIpv6(selectedIp) (or the equivalent isValidIpv4/isValidIpv6 helpers available in the codebase) so the assertion only passes for syntactically valid IPv4 or IPv6 addresses.test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy-110-118 (1)
110-118:⚠️ Potential issue | 🟡 MinorTP-027 对执行机的网卡布局有环境依赖。
10.99.88.0/24并不是保证不存在的网段；如果 CI 节点刚好有这个网段，getMnIpForVr()就可能返回匹配地址而不是 fallback，导致用例偶发失败。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy` around lines 110 - 118, The test testTP027_unmatchedCidrFallback relies on an environment-specific CIDR ("10.99.88.0/24") that may exist on some CI nodes; change the unmatchedCidr usage in testTP027_unmatchedCidrFallback (and where callGetMnIpForVr is invoked) to use a guaranteed non-routable/test-only CIDR (e.g. a TEST-NET block such as "192.0.2.0/24") or, even better, programmatically generate/find a CIDR that does not match any local interface before calling callGetMnIpForVr; update the unmatchedCidr assignment and any assertions accordingly so the test deterministically falls back to Platform.getManagementServerIp().test/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovy-14-34 (1)
14-34:⚠️ Potential issue | 🟡 Minor请把新增测试说明改成英文。
这个类级注释是中文，和仓库约定不一致；同文件里后续新增的方法注释也建议一并统一为英文。
As per coding guidelines:
**/*.*:代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovy` around lines 14 - 34, Translate the class-level Groovy comment in MnIpv6Case (and any other Chinese comments in the same file, including method-level comments added later in this file) into clear, idiomatic English; ensure all descriptions for the test cases (TP-001...TP-031) are converted, contain no Chinese characters, and follow repository coding guidelines (no Chinese in code, error messages, or comments), preserving the original meaning and references like MnIpv6Case, sanitizeCallbackUrl, getManagementServerIp, getManagementServerCidr, and jgroupsAddr so reviewers can map tests to behavior.test/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleManagerDispatchTest.java-377-398 (1)
377-398:⚠️ Potential issue | 🟡 Minor这个用例没有实际覆盖“注册顺序”。
这里创建了
seq，但failedMigrate路径并没有把它写入任何可断言的顺序字段；现有断言只能证明 A/B/C 都被调用过一次，无法发现顺序颠倒或并发执行。建议在MockNicLifecycle.failedMigrate()里也记录顺序，再断言a < b < c。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleManagerDispatchTest.java` around lines 377 - 398, The test multiImpl_invocation_order_follows_registration_order doesn’t assert invocation order because MockNicLifecycle.failedMigrate() doesn’t record the seq; update MockNicLifecycle.failedMigrate() to capture the invocation sequence by incrementing/reading the provided AtomicInteger seq and storing that value in a per-instance field or list (e.g., failedMigrateOrder or failedMigrateSeq), then in the test after mgr.failedToMigrateVm(...) add assertions that a.failedMigrateSeq < b.failedMigrateSeq < c.failedMigrateSeq (in addition to the existing size checks) to verify registration order was preserved.test/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleManagerDispatchTest.java-136-151 (1)
136-151:⚠️ Potential issue | 🟡 Minor这个用例没有真正验证“只传递匹配的 NIC”。
当前断言只检查了
preMigrate被调用一次且目标 host 是h-dst，并没有验证传给扩展的 NIC 集合是否只包含l3-a的那两张卡。即使 manager 把 3 张 NIC 全量传下去，这个测试也会通过。建议让MockNicLifecycle记录preMigrate的 NIC 参数，并在这里断言只收到n1/n3。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleManagerDispatchTest.java` around lines 136 - 151, The test currently only checks that MockNicLifecycle.preMigrate was called once with target "h-dst" but doesn't assert which NICs were passed; update MockNicLifecycle to record the NIC collection passed into its preMigrate method (e.g., add a field like preMigrateNics or extend preMigrateCalls to store a Pair/struct of target and nic list) and then in preMigrate_only_passes_matching_nics_to_setup assert that the recorded NIC list equals the two matching NICs (n1 and n3) — locate the MockNicLifecycle class and its preMigrate method and the test method preMigrate_only_passes_matching_nics_to_setup to add the recording and the assertion.test/src/test/java/org/zstack/test/compute/vmnic/MockNicLifecycle.java-124-132 (1)
124-132:⚠️ Potential issue | 🟡 Minor
reconcileDelayed分支现在无法被测试代码“稍后完成”。这里在延迟模式下只是跳过了
completion.done()，但没有把completion保存出来；一旦某个用例真要验证“延迟后成功”，这个 stub 只能一直挂起，做不到注释里说的“test should drive completion explicitly”。建议把 pending completion 缓存到字段里，并提供显式的finishReconcile()辅助方法。可参考的修改方向
+ private volatile NoErrorCompletion pendingReconcileCompletion; + `@Override` public void reconcileOnHost(String hostUuid, List<VmNicInventory> expectedNics, NoErrorCompletion completion) { reconcileCalls.add(hostUuid); if (!reconcileDelayed) { completion.done(); + return; } - // if delayed, test should drive completion explicitly + pendingReconcileCompletion = completion; } + + public void finishReconcile() { + NoErrorCompletion c = pendingReconcileCompletion; + pendingReconcileCompletion = null; + if (c != null) { + c.done(); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/java/org/zstack/test/compute/vmnic/MockNicLifecycle.java` around lines 124 - 132, The reconcileOnHost method currently ignores the completion when reconcileDelayed is true, causing tests to hang; change it to save the incoming NoErrorCompletion into a new field (e.g., pendingReconcileCompletion) and only call completion.done() immediately when reconcileDelayed is false, and add a public helper method finishReconcile() that invokes pendingReconcileCompletion.done() (and clears the field) so tests can drive the delayed completion explicitly; update the class-level fields (reconcileDelayed, reconcileCalls) accordingly and handle the case where finishReconcile is called with no pending completion (no-op).
🧹 Nitpick comments (27)
network/src/main/java/org/zstack/network/l3/L3NetworkManagerImpl.java (1)
197-197: 建议添加空值检查。
dbf.findByUuid(msg.getL3NetworkUuid(), L3NetworkVO.class)在极端情况下（如并发删除）可能返回null，导致L3NetworkInventory.valueOf(null)抛出 NPE。♻️ 建议的防御性检查
+ L3NetworkVO l3Vo = dbf.findByUuid(msg.getL3NetworkUuid(), L3NetworkVO.class); + if (l3Vo == null) { + trigger.fail(err(SysErrors.RESOURCE_NOT_FOUND, + "L3Network[uuid:%s] not found", msg.getL3NetworkUuid())); + return; + } - L3NetworkInventory l3Inv = L3NetworkInventory.valueOf(dbf.findByUuid(msg.getL3NetworkUuid(), L3NetworkVO.class)); + L3NetworkInventory l3Inv = L3NetworkInventory.valueOf(l3Vo);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@network/src/main/java/org/zstack/network/l3/L3NetworkManagerImpl.java` at line 197, The call to L3NetworkInventory.valueOf(dbf.findByUuid(msg.getL3NetworkUuid(), L3NetworkVO.class)) can NPE if dbf.findByUuid(...) returns null; add a defensive null-check after calling dbf.findByUuid(msg.getL3NetworkUuid(), L3NetworkVO.class) (use the returned L3NetworkVO reference) and handle the missing resource by returning/failing with a clear error (e.g., throw a CloudRuntimeException/ApiMessageInterceptionException or populate an error reply) instead of passing null into L3NetworkInventory.valueOf; update the code around L3NetworkInventory.valueOf and dbf.findByUuid to first test for null and include msg.getL3NetworkUuid in the error message.core/src/main/java/org/zstack/core/rest/webhook/WebhookCallbackClient.java (2)
5-5: 建议：避免直接依赖实现类ThreadFacadeImpl当前代码导入了
ThreadFacadeImpl仅为使用其内部类TimeoutTaskReceipt。这违反了面向接口编程原则，增加了与具体实现的耦合。建议将
TimeoutTaskReceipt提取为独立的顶层接口或类，或通过ThreadFacade接口暴露，以保持代码的松耦合。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/rest/webhook/WebhookCallbackClient.java` at line 5, The import of the concrete class ThreadFacadeImpl in WebhookCallbackClient to access TimeoutTaskReceipt couples this class to an implementation; replace that by depending on an abstraction: add or expose a top-level TimeoutTaskReceipt type via the ThreadFacade interface (or create a standalone interface/class TimeoutTaskReceipt), update references in WebhookCallbackClient to use ThreadFacade.TimeoutTaskReceipt or the new TimeoutTaskReceipt type, and remove the import of ThreadFacadeImpl so WebhookCallbackClient only depends on the interface ThreadFacade (or the new top-level type).
81-87: 建议添加幂等性保护
start()方法当前没有防止重复调用的保护。如果被多次调用，会重复注册 HTTP 回调处理器，可能导致不可预期的行为。♻️ 建议的修改
+ private volatile boolean started = false; + public void start() { + if (started) { + return; + } + started = true; this.callbackUrl = restf.getSendCommandUrl(); restf.registerSyncHttpCallHandler( protocol.getCallbackPath(), protocol.getCallbackClass(), this::onCallback); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/rest/webhook/WebhookCallbackClient.java` around lines 81 - 87, The start() method registers an HTTP handler unconditionally, so repeated calls double-register the handler; add an idempotency guard (e.g., a private AtomicBoolean started or a volatile boolean with synchronized start) and check-and-set at the beginning of start() to return immediately if already started, then proceed to set callbackUrl and call restf.registerSyncHttpCallHandler(protocol.getCallbackPath(), protocol.getCallbackClass(), this::onCallback) only once; ensure the check is thread-safe to prevent race conditions.header/src/main/java/org/zstack/header/network/l3/AfterUpdateIpRangeExtensionPoint.java (1)
3-4: 请为扩展点方法补充 Javadoc 契约说明。Line [3-4] 当前缺少方法级文档，建议明确调用阶段、参数语义（old/new 是否为持久化前后快照）和异常处理约定，避免插件实现出现歧义。
As per coding guidelines, “接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/network/l3/AfterUpdateIpRangeExtensionPoint.java` around lines 3 - 4, 为 AfterUpdateIpRangeExtensionPoint 接口和其方法 afterUpdateIpRange 添加规范性 Javadoc：说明该扩展点何时被调用（例如在 IP 范围更新完成并持久化后触发或在事务 commit 之后/之前），明确参数含义（oldIpRange 为更新前的持久化快照，newIpRange 为更新后的持久化快照，均为 IpRangeInventory 实例且不可变假设），约定实现的异常处理（不应抛出受检异常；若抛出运行时异常，平台将记录并可能回滚/忽略），并补充并发/事务上下文说明（是否在同一线程、是否在数据库事务内执行）。同时移除接口方法上的多余修饰符（不要在接口方法上使用 public 修饰符），保证 Javadoc 覆盖 AfterUpdateIpRangeExtensionPoint 和 afterUpdateIpRange 的行为契约与参数语义。rest/src/main/resources/scripts/RestDocumentationGenerator.groovy (1)
827-831: 避免在一致性检查中原地修改资源列表Line [827] 直接对
md.globalConfig.resources.sort()会修改md对象状态。建议在副本上排序，减少后续逻辑的隐式副作用。建议修改
- List<String> oldClasses = md.globalConfig.resources.sort() + List<String> oldClasses = new ArrayList<>(md.globalConfig.resources ?: []).sort()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around lines 827 - 831, The code calls md.globalConfig.resources.sort() which mutates md's state; change it to sort a copy instead so md isn't modified in place—for example, create a new list from md.globalConfig.resources (e.g., via toList()/collect() or new ArrayList(md.globalConfig.resources)) and sort that into oldClasses, then compare oldClasses to newClasses; ensure you only read from md and never call sort() directly on md.globalConfig.resources.header/src/main/java/org/zstack/header/vm/AfterReleaseVmNicExtensionPoint.java (1)
10-11: 建议补充方法级契约说明（Completion调用约束）。建议在方法上明确“实现方必须且只能回调一次 completion（success/fail）”，避免扩展实现导致流程悬挂。
✍️ 建议修改
public interface AfterReleaseVmNicExtensionPoint { + /** + * Implementations must invoke {`@code` completion.success()} or + * {`@code` completion.fail(...)} exactly once. + */ void afterReleaseVmNic(VmNicInventory nic, Completion completion); }As per coding guidelines:
接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/AfterReleaseVmNicExtensionPoint.java` around lines 10 - 11, 为接口 AfterReleaseVmNicExtensionPoint 和其方法 afterReleaseVmNic(VmNicInventory nic, Completion completion) 添加 Javadoc，明确规定实现者必须且只能回调一次 completion（要么 completion.success(...) 要么 completion.fail(...)）且不得多次或遗漏回调；同时确保接口方法没有冗余的修饰符（不要在方法上显式声明 public 等）。在注释中给出简短示例语义（“调用完成后不应再访问流程” 或类似约束）以避免实现导致流程悬挂。header/src/main/java/org/zstack/header/vm/BeforeAllocateVmNicExtensionPoint.java (1)
12-13: 建议在方法签名处补充Completion回调语义。当前接口描述已很清晰，但建议把“必须且仅一次回调 completion”的约束直接落在方法 Javadoc 上，降低扩展点误用成本。
✍️ 建议修改
public interface BeforeAllocateVmNicExtensionPoint { + /** + * Implementations must invoke {`@code` completion.success()} or + * {`@code` completion.fail(...)} exactly once. + */ void beforeAllocateVmNic(VmNicInventory nic, VmInstanceSpec spec, Completion completion); }As per coding guidelines:
接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/BeforeAllocateVmNicExtensionPoint.java` around lines 12 - 13, Add a Javadoc to the BeforeAllocateVmNicExtensionPoint.beforeAllocateVmNic method that documents the Completion callback semantics: state that implementors must invoke the provided Completion exactly once (either success or failure), thread/context expectations (if any), and what happens if the callback is not invoked; also remove any redundant access modifier from the interface method signature so it uses the implicit public visibility for interface methods; reference the BeforeAllocateVmNicExtensionPoint interface and the beforeAllocateVmNic(VmNicInventory nic, VmInstanceSpec spec, Completion completion) method when making these changes.utils/src/main/java/org/zstack/utils/network/IPv6Utils.java (1)
34-40: 建议规范化buildHttpsUrl的path前缀，避免生成不规范 URL。Line 39 直接
base + path，当调用方传入"api"时会得到https://host:443api。建议统一补齐前导/。可参考修改
public static String buildHttpsUrl(String ip, int port, String path) { String base = "https://" + bracketIpv6(ip) + ":" + port; if (path == null || path.isEmpty()) { return base; } - return base + path; + String normalizedPath = path.startsWith("/") ? path : "/" + path; + return base + normalizedPath; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java` around lines 34 - 40, The buildHttpsUrl method concatenates base and path without ensuring a leading slash, producing malformed URLs like "https://host:443api"; update buildHttpsUrl to normalize the path by checking if path is non-null/non-empty and, if it does not start with '/', prepend a '/' before concatenation (use the existing bracketIpv6 logic for the host portion), so return base + normalizedPath; leave behavior for null/empty path unchanged.compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java (1)
125-132: 建议先对managementIp做trim()再执行校验与规范化。Line 125-132 目前直接使用原始输入；若用户粘贴包含前后空白，体验会变差且不利于数据规范化落库。
As per coding guidelines
**/*Interceptor.java:注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等.可参考修改
- String ip = msg.getManagementIp(); + String ip = msg.getManagementIp() == null ? null : msg.getManagementIp().trim(); if (!NetworkUtils.isHostname(ip) && !IPv6Utils.isValidManagementIp(ip)) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_HOST_10113, "managementIp[%s] is not a valid IPv4/IPv6 address or hostname", ip)); } // Normalize IPv6 to RFC 5952 compressed form; IPv4/hostname returned as-is msg.setManagementIp(IPv6Utils.normalizeIpv6(ip));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java` around lines 125 - 132, Trim the managementIp string before validation and normalization: obtain the value from msg.getManagementIp(), call trim() (and handle null/empty appropriately), then run NetworkUtils.isHostname/IPv6Utils.isValidManagementIp checks and finally set the normalized value via IPv6Utils.normalizeIpv6 on the trimmed input; update the logic in HostApiInterceptor (the block handling managementIp validation/normalization) so stored/validated values use the trimmed string.plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java (4)
530-531: 行内注释应使用英文♻️ 建议修改
- // F-009: 根据 VR 管理网 CIDR 选取与其同子网的 MN IP，支持多 IP/双栈主机 + // F-009: Select MN IP matching VR management CIDR, supports multi-IP/dual-stack hosts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java` around lines 530 - 531, The inline comment above the declaration of vrMgmtCidr in ApplianceVmFacadeImpl is written in Chinese; replace it with an English comment describing the same behavior (e.g., "Select MN IP(s) in the same subnet as the VR management network CIDR; support multi-IP/dual-stack hosts") so the codebase uses English inline comments consistently and the meaning is preserved.
405-407: 内部注释应使用英文♻️ 建议修改
} catch (Exception ignore) { - // CIDR 类型与 IP 类型不匹配时忽略 + // ignore when CIDR type doesn't match IP type (e.g., IPv4 vs IPv6) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java` around lines 405 - 407, Replace the Chinese inline comment in the catch block inside ApplianceVmFacadeImpl (the catch (Exception ignore) { // CIDR 类型与 IP 类型不匹配时忽略 }) with an English comment, e.g. "// Ignore when CIDR and IP types do not match", keeping the behavior unchanged and leaving the exception suppression as-is; update only the comment text in the catch block to use English.
545-548: 日志消息应使用英文♻️ 建议修改
} catch (Exception ignored) { - logger.warn(String.format("failed to parse IPv6 netmask [%s] for VR nic IP [%s], using default prefix /64", + logger.warn(String.format("failed to parse IPv6 netmask [%s] for VR NIC IP [%s], using default prefix /64", mgmtNic.getNetmask(), mgmtNic.getIp())); }注意：这条日志消息本身是英文的，但变量名
mgmtNic和字段引用是正确的。仅将 "nic" 改为 "NIC" 以保持一致性（可选）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java` around lines 545 - 548, The log message in ApplianceVmFacadeImpl's catch block uses "nic" lowercase; update the logger.warn call that formats mgmtNic.getNetmask() and mgmtNic.getIp() so the message reads "NIC" (uppercase) for consistency and remains English, e.g., change "VR nic IP" to "VR NIC IP" in the logger.warn format string that surrounds mgmtNic.getNetmask() and mgmtNic.getIp().
365-371: 方法注释应使用英文根据编码规范，代码中不应包含中文，包括注释。请将方法文档注释改为英文。
♻️ 建议修改
- /** - * F-009: 从本机网卡中选取与 VR 管理网 CIDR 同子网的 MN IP。 - * 支持 IPv4/IPv6 双栈；若无匹配则回退到 Platform.getManagementServerIp()。 - * - * `@param` vrManagementCidr VR 管理网的网络 CIDR，例如 "192.168.1.0/24" 或 "2001:db8::/64" - * `@return` 选定的 MN IP（裸地址，无括号） - */ + /** + * F-009: Select a local MN IP that matches the VR management network CIDR. + * Supports IPv4/IPv6 dual-stack; falls back to Platform.getManagementServerIp() if no match. + * + * `@param` vrManagementCidr VR management network CIDR, e.g., "192.168.1.0/24" or "2001:db8::/64" + * `@return` selected MN IP (bare address, no brackets) + */As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java` around lines 365 - 371, Update the JavaDoc comment in ApplianceVmFacadeImpl for the method that takes parameter vrManagementCidr and returns a management node IP so that the entire comment is written in English; replace the Chinese description and annotations (including the summary, IPv4/IPv6 explanation, fallback behavior and `@param/`@return text) with a clear English JavaDoc explaining: purpose (select MN IP from local interfaces matching VR management CIDR), dual-stack support and fallback to Platform.getManagementServerIp(), examples of vrManagementCidr formats, and that the return is a plain IP address. Ensure the JavaDoc tags reference vrManagementCidr and the return value exactly as in the current signature.core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java (2)
68-69: IPv6 URL 正则表达式的边界情况正则表达式
BARE_IPV6_URL_PATTERN用于检测裸 IPv6 地址。当前实现配合isIpv6Address()二次校验是安全的，但有几点值得注意：
- IPv6 地址长度范围
{2,37}可能略窄（完整 IPv6 如2001:0db8:0000:0000:0000:0000:0000:0001为 39 字符）- 双冒号压缩形式（如
::1）仅 3 字符由于有
IPv6NetworkUtils.isIpv6Address()作为二次校验，当前实现是安全的，这只是一个可选的改进建议。♻️ 可选：放宽正则范围
- private static final java.util.regex.Pattern BARE_IPV6_URL_PATTERN = java.util.regex.Pattern.compile( - "^(https?)://([0-9a-fA-F][0-9a-fA-F:]{2,37}[0-9a-fA-F]):(\\d+)(/.*)?"); + private static final java.util.regex.Pattern BARE_IPV6_URL_PATTERN = java.util.regex.Pattern.compile( + "^(https?)://([0-9a-fA-F:]{2,39}):(\\d+)(/.*)?");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java` around lines 68 - 69, BARE_IPV6_URL_PATTERN currently limits the IPv6 host part with [0-9a-fA-F:]{2,37} which can exclude valid full-length addresses and compressed forms like ::1; update the pattern in RESTFacadeImpl (BARE_IPV6_URL_PATTERN) to broaden the allowed host length (for example use {2,39} or simply + instead of {2,37}) so legitimate IPv6 literals pass this initial regex and continue to be validated by IPv6NetworkUtils.isIpv6Address(); keep the rest of the pattern (scheme, port, optional path) unchanged.
984-989: 方法注释应使用英文根据编码规范，代码中不应包含中文。
♻️ 建议修改
- /** - * 检测并修复裸 IPv6（无方括号）的 callbackUrl。 - * 正常路径下 URL 应由 {`@link` IPv6Utils#buildUrl} 生成，此方法作为兜底防御层。 - * <p> - * 示例：http://2001:db8::1:8080/path → http://[2001:db8::1]:8080/path - */ + /** + * Detect and fix bare IPv6 (unbracketed) callbackUrl. + * Normally URLs should be generated by {`@link` IPv6Utils#buildUrl}; this method serves as a fallback defense layer. + * <p> + * Example: http://2001:db8::1:8080/path → http://[2001:db8::1]:8080/path + */As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java` around lines 984 - 989, Replace the Chinese Javadoc block above the method that detects and fixes bare IPv6 callback URLs in RESTFacadeImpl (referencing IPv6Utils#buildUrl and the callbackUrl handling) with an English Javadoc: describe the purpose ("Detect and fix bare IPv6 (no brackets) callbackUrl as a defensive fallback when URLs should be generated by IPv6Utils.buildUrl"), include the same example in English ("e.g. http://2001:db8::1:8080/path → http://[2001:db8::1]:8080/path"), and keep any `@see` or links intact; ensure no Chinese text remains in the comment or example.compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java (2)
40-43:vmMgr字段已注入但未使用。与
VmDetachNicFlow相同，VmInstanceManager vmMgr被注入但未使用，应移除。♻️ 建议修改
`@Autowired` - protected VmInstanceManager vmMgr; - `@Autowired` protected PluginRegistry pluginRgty;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java` around lines 40 - 43, 字段 vmMgr (类型 VmInstanceManager) 在类 VmReturnReleaseNicFlow 中已被 `@Autowired` 注入但未使用；请从 VmReturnReleaseNicFlow 中移除该字段及其 `@Autowired` 注解（同时清理可能因此不再使用的导入），保持 PluginRegistry pluginRgty 等其他注入不变；参考 VmDetachNicFlow 中相同的清理做法以确保没有残留未使用符号或警告。
120-146:callReleaseSdnNics与VmDetachNicFlow中的实现重复。此方法与
VmDetachNicFlow.callReleaseSdnNics()代码几乎相同。建议考虑提取为共享工具类（如VmNicHelper或SdnNicReleaseHelper）以减少重复。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java` around lines 120 - 146, Extract the duplicated logic in VmReturnReleaseNicFlow.callReleaseSdnNics and VmDetachNicFlow.callReleaseSdnNics into a single helper (e.g., VmNicHelper.releaseSdnNics or SdnNicReleaseHelper.releaseSdnNics) that accepts the List<VmNicInventory>, a Completion callback, and any dependencies needed (pluginRgty and logger or have the helper obtain pluginRgty). Move the While/WhileDoneCompletion orchestration and the AfterAllocateSdnNicExtensionPoint invocation into that helper, then replace both original implementations to call the new helper method so both use the same shared code path.compute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.java (1)
44-47:vmMgr字段已注入但未使用。
VmInstanceManager vmMgr被注入但在此类中从未使用，应移除以避免死代码。♻️ 建议修改
`@Autowired` - private VmInstanceManager vmMgr; - `@Autowired` private PluginRegistry pluginRgty;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.java` around lines 44 - 47, The VmInstanceManager field vmMgr in class VmDetachNicFlow is injected but never used; remove the unused field declaration (`@Autowired` private VmInstanceManager vmMgr;) from VmDetachNicFlow and clean any unused imports related to VmInstanceManager to eliminate dead code while leaving PluginRegistry pluginRgty intact; if vmMgr was intended for future use, replace with a TODO comment or add usage where appropriate, otherwise delete the field declaration.test/src/test/groovy/org/zstack/test/integration/network/ipv6/MnIpv6StorageMigrationCase.groovy (1)
7-135: 请将该文件里的新增注释统一改成英文。这次新增的类注释、方法注释和行内注释都用了中文。
As per coding guidelines
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/network/ipv6/MnIpv6StorageMigrationCase.groovy` around lines 7 - 135, The file MnIpv6StorageMigrationCase contains Chinese comments in the class JavaDoc, all test method comments and inline comments (e.g., the class comment and method comments for testTP032_nfsCidrIpv6NotRejected, testTP033_isIpInCidrIpv6Match, testTP034_isIpInCidrNoMatchFallback, testTP035_buildAddrIpv6BracketFormat, testTP036_cephMonAddrIpv6Format, testTP038_checkMigrateNetworkCidrFallback and any inline logger messages) — replace all Chinese text with clear, correct English equivalents while preserving intent (e.g., describe test goals and assertions), update any Chinese punctuation to ASCII, and ensure comments and messages use consistent English wording and spelling.test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M4Case.groovy (1)
6-259: 请将该文件里的新增注释统一改成英文。类注释、方法注释以及多处背景说明都使用了中文，和仓库统一约定不一致。
As per coding guidelines
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M4Case.groovy` around lines 6 - 259, Translate all Chinese comments and Javadoc in class MnIpv6M4Case and its test methods (e.g., testTP083_influxDbUrlIpv6Bracket, testTP084_prometheusWriteUrlIpv6, testTP085_grafanaDataSourceUrlIpv6, testTP086_licenseHttpUrlIpv6, testTP087_keycloakContainerNameSanitize, testTP088_ssoCasLoginUrlIpv6) into clear, idiomatic English; update class-level block comment, each method description and background notes, and any inline comment lines while leaving code, assertions, identifiers, and logger messages unchanged unless they are comments; ensure grammar and spelling are correct and that the English text conveys the same meaning as the original Chinese.test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy (1)
10-152: 请将该文件里的新增注释统一改成英文。这个新测试文件里的类注释、方法注释和行内注释都用了中文。
As per coding guidelines
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy` around lines 10 - 152, The file contains Chinese comments that must be converted to English; update all comments in the ApplianceVmIpv6Case class (including the class Javadoc-like header and the method comments for initReflection, callGetMnIpForVr, testTP025_mnCidrMatchReturnsMnIp, testTP026_nullCidrFallback, testTP027_unmatchedCidrFallback, testTP028_returnedIpNoBrackets, testTP029_invalidCidrFallback and any inline comments) to clear, idiomatic English with correct spelling and punctuation, preserving the original intent and test descriptions (TP-025..TP-029) and keeping references to Platform.getManagementServerIp/Cidr, ApplianceVmFacadeImpl, and getMnIpForVr intact. Ensure no Chinese characters remain in comments or log messages.test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M3Case.groovy (1)
8-218: 请将该文件里的新增注释统一改成英文。当前新增的类注释、方法注释和行内注释都包含中文。
As per coding guidelines
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M3Case.groovy` around lines 8 - 218, The file MnIpv6M3Case contains Chinese comments and docstrings throughout (class header and all test methods like testTP062_ipmiIpv6AcceptedInInterceptor, testTP065_ipmiAddressFullLengthIpv6, testTP066_ipmiInvalidAddressRejected, testTP067_consoleBracketIpv6, testTP069_consoleVncTokenUrl, testTP064_consoleDualStackVip, testTP076_bmDpuCallbackIpBracket, testTP077_coloQemuUrlIpv6Bracket); update all Chinese text to clear, correct English (class-level description, Javadoc-style method comments and inline comments/log messages) keeping the original meaning and test identifiers (TP-062...TP-077) intact and without introducing typos. Ensure log/assert messages are also converted to English so no Chinese remains in strings.test/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovy (1)
6-133: 请将该文件里的新增注释统一改成英文。当前类注释、方法注释和多处行内注释都包含中文，不符合仓库约定。
As per coding guidelines
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovy` around lines 6 - 133, The file contains Chinese comments and message strings that violate the repository guideline; update all Chinese text to clear English in the class Javadoc for ZSha2Ipv6Case, all method comments (testTP043_isMasterGrepPatternMatchesIpv6, testTP042_isMasterGrepLogicWithVip, testTP044_bracketIpv6ForSshScp, testTP045_nginxUpstreamIpv6Format, testTP046_iamUrlIpv6Brackets) and all inline comments, assertion messages and logger.info calls to English equivalents, preserving the original meaning (e.g. "TP-042/043..." descriptions, expected/actual messages), and ensure no Chinese characters remain in any string literal or comment in this class.test/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovy (1)
14-172: 请将该文件里的新增注释统一改成英文。类注释、方法注释和多处行内注释都用了中文，和仓库约定不一致。
As per coding guidelines
代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovy` around lines 14 - 172, Replace all Chinese comments and Javadoc in the KvmHostIpv6Case class with English equivalents while preserving meaning; update the class-level comment and the Javadoc for methods testTP015_managementIpColumnLength, testTP016_addHostWithIpv6Passes, testTP017_fullIpv6NormalizedBeforeConnect, testTP018_linkLocalIpv6Rejected, testTP019_invalidIpRejected, and testTP020_fullIpv6FitsInColumn, and convert any Chinese inline comments inside those methods to English (e.g., the explanatory lines before assertions and the comment about normalization and expected behaviors). Keep identifiers and assertions unchanged, and ensure logger messages that are comments remain consistent in English where they are explanatory (do not alter assertion messages unless they contain Chinese).test/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleStartStopCase.groovy (1)
23-29:VmNicLifecycleExtension.xml重复引入。与
VmNicLifecycleCoexistenceCase相同，KvmTest.springSpec已包含该扩展，此处spring { include(...) }冗余。♻️ 建议的修改
`@Override` void setup() { useSpring(KvmTest.springSpec) - spring { - include("VmNicLifecycleExtension.xml") - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleStartStopCase.groovy` around lines 23 - 29, The setup() method in VmNicLifecycleStartStopCase.groovy redundantly includes VmNicLifecycleExtension.xml even though KvmTest.springSpec already provides that extension (same as in VmNicLifecycleCoexistenceCase); remove the spring { include("VmNicLifecycleExtension.xml") } block from the setup() method so the test relies on KvmTest.springSpec for the extension instead of re-including it.test/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleCoexistenceCase.groovy (1)
29-34:VmNicLifecycleExtension.xml重复引入。
KvmTest.springSpec已在第 27 行包含了VmNicLifecycleExtension.xml，此处spring { include(...) }可能导致重复加载。建议移除冗余的spring块，或确认框架能正确处理重复引入。♻️ 建议的修改
`@Override` void setup() { useSpring(KvmTest.springSpec) - spring { - include("VmNicLifecycleExtension.xml") - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleCoexistenceCase.groovy` around lines 29 - 34, The setup() method is including VmNicLifecycleExtension.xml a second time via the spring { include("VmNicLifecycleExtension.xml") } call while KvmTest.springSpec already contains that XML; remove the redundant spring include from setup() (or alternatively remove the duplicate from KvmTest.springSpec) so VmNicLifecycleExtension.xml is only loaded once—locate the setup() method in VmNicLifecycleCoexistenceCase.groovy and delete the spring { include("VmNicLifecycleExtension.xml") } block (or confirm the framework deduplicates and adjust accordingly).test/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleMigrateCase.groovy (1)
27-33:VmNicLifecycleExtension.xml重复引入。与其他生命周期测试用例相同，此处的
spring { include(...) }冗余。♻️ 建议的修改
`@Override` void setup() { useSpring(KvmTest.springSpec) - spring { - include("VmNicLifecycleExtension.xml") - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleMigrateCase.groovy` around lines 27 - 33, The test setup redundantly includes "VmNicLifecycleExtension.xml" via the spring { include("VmNicLifecycleExtension.xml") } call inside the setup() method; remove that include so the setup() only calls useSpring(KvmTest.springSpec) and the existing global/other test wiring supplies the extension, i.e., delete the include(...) line (or the entire spring { ... } block) from setup() in VmNicLifecycleMigrateCase.groovy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 7b4a8a50-b474-4dec-a5c1-491bd6cac1ec
⛔ Files ignored due to path filters (12)
build/pom.xmlis excluded by!**/*.xmlconf/globalConfig/vmNicLifecycle.xmlis excluded by!**/*.xmlconf/springConfigXml/Kvm.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xmlconf/springConfigXml/sdnController.xmlis excluded by!**/*.xmlsdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/NicTO.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/network/zns/CreateL2GeneveNetworkAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/network/zns/L2GeneveNetworkInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/network/zns/ZnsControllerInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/network/zns/ZnsTransportZoneInventory.javais excluded by!sdk/**test/src/test/resources/springConfigXml/VmNicLifecycleExtension.xmlis excluded by!**/*.xml
📒 Files selected for processing (88)
.gitconfig/hooks/commit-msgcompute/src/main/java/org/zstack/compute/host/HostApiInterceptor.javacompute/src/main/java/org/zstack/compute/vm/VmAllocateNicFlow.javacompute/src/main/java/org/zstack/compute/vm/VmAllocateSdnNicFlow.javacompute/src/main/java/org/zstack/compute/vm/VmCascadeExtension.javacompute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceExtensionPointEmitter.javacompute/src/main/java/org/zstack/compute/vm/VmNicLifecycleGlobalConfig.javacompute/src/main/java/org/zstack/compute/vm/VmNicLifecycleManager.javacompute/src/main/java/org/zstack/compute/vm/VmNicManagerImpl.javacompute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.javaconf/db/upgrade/V5.5.18__schema.sqlconsole/src/main/java/org/zstack/console/ConsoleManagerImpl.javacore/src/main/java/org/zstack/core/Platform.javacore/src/main/java/org/zstack/core/config/NetworkGlobalConfig.javacore/src/main/java/org/zstack/core/rest/RESTFacadeImpl.javacore/src/main/java/org/zstack/core/rest/webhook/WebhookCallbackClient.javacore/src/main/java/org/zstack/core/rest/webhook/WebhookProtocol.javadocs/modules/network/nav.adocdocs/modules/network/pages/networkResource/ZStackL2NetworkType.adocdocs/modules/network/pages/networkResource/ZnsIntegration.adocdocs/modules/network/pages/networkResource/networkResource.adocheader/src/main/java/org/zstack/header/network/l2/L2NetworkConstant.javaheader/src/main/java/org/zstack/header/network/l3/AfterSetL3NetworkMtuExtensionPoint.javaheader/src/main/java/org/zstack/header/network/l3/AfterUpdateIpRangeExtensionPoint.javaheader/src/main/java/org/zstack/header/network/l3/L3NetworkInventory.javaheader/src/main/java/org/zstack/header/network/l3/L3NetworkType.javaheader/src/main/java/org/zstack/header/network/l3/L3NetworkVO.javaheader/src/main/java/org/zstack/header/vm/AfterAllocateSdnNicExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/AfterAllocateVmNicIpExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/AfterReleaseVmNicExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/BeforeAllocateVmNicExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/VmInstanceNicFactory.javaheader/src/main/java/org/zstack/header/vm/VmNicLifecycleExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/VmOvsNicConstant.javanetwork/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.javanetwork/src/main/java/org/zstack/network/l2/L2NetworkExtensionPointEmitter.javanetwork/src/main/java/org/zstack/network/l2/L2NetworkHostHelper.javanetwork/src/main/java/org/zstack/network/l2/L2NoVlanNetwork.javanetwork/src/main/java/org/zstack/network/l3/L3BasicNetwork.javanetwork/src/main/java/org/zstack/network/l3/L3NetworkManagerImpl.javaplugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageMonBase.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2NoVlanNetworkBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2VlanNetworkBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/KvmHostIpmiPowerExecutor.javaplugin/kvm/src/main/java/org/zstack/kvm/VmNicLifecycleKvmBridge.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsApiParamChecker.javaplugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.javaplugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerFactory.javaplugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.javaplugin/sdnController/src/main/java/org/zstack/sdnController/h3cVcfc/H3cVcfcSdnControllerFactory.javaplugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/SugonSdnControllerFactory.javaplugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanNetworkCheckerImpl.javaplugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.javarest/src/main/resources/scripts/RestDocumentationGenerator.groovytest/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/core/MnIpv6M3Case.groovytest/src/test/groovy/org/zstack/test/integration/core/MnIpv6M4Case.groovytest/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovytest/src/test/groovy/org/zstack/test/integration/kvm/KvmTest.groovytest/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleCoexistenceCase.groovytest/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleMigrateCase.groovytest/src/test/groovy/org/zstack/test/integration/kvm/vmnic/VmNicLifecycleStartStopCase.groovytest/src/test/groovy/org/zstack/test/integration/network/ipv6/MnIpv6StorageMigrationCase.groovytest/src/test/groovy/org/zstack/test/integration/network/vxlan/VxlanIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/utils/IPv6UtilsCase.groovytest/src/test/java/org/zstack/test/compute/vmnic/MockNicLifecycle.javatest/src/test/java/org/zstack/test/compute/vmnic/TestVmNicLifecycleExtension.javatest/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleExtensionPointDefaultTest.javatest/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleKvmBridgeTest.javatest/src/test/java/org/zstack/test/compute/vmnic/VmNicLifecycleManagerDispatchTest.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/SdnControllerSpec.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.javautils/src/main/java/org/zstack/utils/network/IPv6Utils.javautils/src/main/java/org/zstack/utils/network/NetworkUtils.javautils/src/main/java/org/zstack/utils/zsha2/ZSha2Helper.java
💤 Files with no reviewable changes (5)
- plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerFactory.java
- plugin/kvm/src/main/java/org/zstack/kvm/KvmHostIpmiPowerExecutor.java
- plugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/SugonSdnControllerFactory.java
- plugin/sdnController/src/main/java/org/zstack/sdnController/h3cVcfc/H3cVcfcSdnControllerFactory.java
- header/src/main/java/org/zstack/header/vm/VmOvsNicConstant.java
Comment on lines
895
to
901
| // F-002: 支持 IPv6 — 枚举所有网卡，按 PREFER_IPV6 配置决定优先级 | ||
| boolean preferIpv6 = false; | ||
| try { | ||
| preferIpv6 = NetworkGlobalConfig.PREFER_IPV6.value(Boolean.class); | ||
| } catch (Exception ignored) { | ||
| // GlobalConfig 可能在静态初始化阶段还未就绪，安全降级为 false | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
注释应使用英文。
此处注释 "F-002: 支持 IPv6 — 枚举所有网卡，按 PREFER_IPV6 配置决定优先级" 和 "GlobalConfig 可能在静态初始化阶段还未就绪，安全降级为 false" 需替换为英文。
♻️ 建议修改
- // F-002: 支持 IPv6 — 枚举所有网卡，按 PREFER_IPV6 配置决定优先级
+ // Enumerate all network interfaces and select preferred IP family based on PREFER_IPV6 config
boolean preferIpv6 = false;
try {
preferIpv6 = NetworkGlobalConfig.PREFER_IPV6.value(Boolean.class);
} catch (Exception ignored) {
- // GlobalConfig 可能在静态初始化阶段还未就绪，安全降级为 false
+ // GlobalConfig may not be ready during static initialization; safely default to false
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/main/java/org/zstack/core/Platform.java` around lines 895 - 901,
Replace the Chinese comments in the Platform class where the preferIpv6 boolean
is determined: change "F-002: 支持 IPv6 — 枚举所有网卡，按 PREFER_IPV6 配置决定优先级" to an
English comment like "F-002: IPv6 support — enumerate all network interfaces and
decide preference based on PREFER_IPV6" and change "GlobalConfig
可能在静态初始化阶段还未就绪，安全降级为 false" to an English comment like "GlobalConfig may not be
ready during static initialization; safely degrade to false", keeping the
surrounding logic that reads
NetworkGlobalConfig.PREFER_IPV6.value(Boolean.class) and the try/catch around
preferIpv6 intact.
Comment on lines
+1014
to
+1020
| vo = dbf.updateAndRefresh(vo); | ||
| } | ||
|
|
||
| IpRangeInventory newIpr = IpRangeInventory.valueOf(vo); | ||
| for (AfterUpdateIpRangeExtensionPoint ext : pluginRgty.getExtensionList(AfterUpdateIpRangeExtensionPoint.class)) { | ||
| ext.afterUpdateIpRange(oldIpr, newIpr); | ||
| } |
There was a problem hiding this comment.
避免扩展点异常把“已更新成功”变成 API 失败。
Line [1014] 已可能完成持久化更新；若 Line [1019] 某个扩展抛异常，会导致接口回错且中断后续扩展执行，产生状态与返回结果不一致。
建议修复（对每个扩展做异常隔离）
- for (AfterUpdateIpRangeExtensionPoint ext : pluginRgty.getExtensionList(AfterUpdateIpRangeExtensionPoint.class)) {
- ext.afterUpdateIpRange(oldIpr, newIpr);
- }
+ for (AfterUpdateIpRangeExtensionPoint ext : pluginRgty.getExtensionList(AfterUpdateIpRangeExtensionPoint.class)) {
+ try {
+ ext.afterUpdateIpRange(oldIpr, newIpr);
+ } catch (Exception e) {
+ logger.warn(String.format(
+ "AfterUpdateIpRangeExtensionPoint[%s] failed for ipRange[uuid:%s]",
+ ext.getClass().getName(), newIpr.getUuid()
+ ), e);
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java` around lines
1014 - 1020, 在 L3BasicNetwork 的 IP 段更新逻辑中，vo 已经通过 dbf.updateAndRefresh(vo)
持久化并构造了 newIpr，但对
pluginRgty.getExtensionList(AfterUpdateIpRangeExtensionPoint.class) 的迭代中若某个
ext.afterUpdateIpRange(oldIpr, newIpr) 抛异常会将“已更新成功”变成 API 失败；请对每个扩展调用做异常隔离：在遍历
pluginRgty.getExtensionList(...) 时对每个 ext 调用 afterUpdateIpRange 包一层 try/catch，捕获
Throwable，使用合适的日志（包含扩展类名和异常信息）记录错误并继续处理下一个扩展，确保已成功的持久化不因扩展异常回滚或导致接口返回失败（保持原有方法签名和返回行为）。
Comment on lines
+273
to
+279
| try { | ||
| tagMgr.createTagsFromAPICreateMessage(msg, vo.getUuid(), SdnControllerVO.class.getSimpleName()); | ||
| event.setInventory(SdnControllerInventory.valueOf(dbf.findByUuid(vo.getUuid(), SdnControllerVO.class))); | ||
| } catch (Exception e) { | ||
| logger.warn(String.format("failed to load SdnControllerVO[uuid:%s] after init: %s", | ||
| vo.getUuid(), e.getMessage()), e); | ||
| } |
There was a problem hiding this comment.
不要在创建成功路径里吞掉 inventory/tag 初始化异常。
一旦进入这个 catch，事件仍会按成功发布，但 event 可能没有 inventory。这会把“创建成功但返回体不完整”的状态静默化，调用方很难感知到后置初始化已经失败。
Based on learnings: 在 ZStack 中，资源创建成功路径（如 handle(APIAddSdnControllerMsg) 中）对 tagMgr.createTagsFromAPICreateMessage(...) 和 dbf.findByUuid(...) 等后续操作，不需要用 try/catch 来防御"资源不存在"异常。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.java`
around lines 273 - 279, The code currently swallows exceptions from
tagMgr.createTagsFromAPICreateMessage(...) and
SdnControllerInventory.valueOf(dbf.findByUuid(...)) causing a success event to
be published without a populated inventory; remove the try/catch around those
calls (in the create flow that handles APIAddSdnControllerMsg) so any exception
from tagMgr.createTagsFromAPICreateMessage, dbf.findByUuid, or
SdnControllerInventory.valueOf will propagate and cause the create to fail
visibly, or alternatively rethrow/log and set the event as a failure with an
error; ensure event.setInventory(...) is executed only on successful retrieval
so callers never receive a success response with a missing inventory.
Comment on lines
+74
to
+84
| /** | ||
| * TP-064: 完整展开的 IPv6 地址长度为 39 字符，NetworkUtils 应能正确识别。 | ||
| */ | ||
| void testTP065_ipmiAddressFullLengthIpv6() { | ||
| String fullIpv6 = "2001:0db8:0000:0000:0000:0000:0000:0001" | ||
|
|
||
| assert fullIpv6.length() == 39 : "TP-064: full IPv6 address should be 39 chars, got: ${fullIpv6.length()}" | ||
|
|
||
| boolean isV6 = IPv6NetworkUtils.isIpv6Address(fullIpv6) | ||
| assert isV6 : "TP-064: 39-char full IPv6 '$fullIpv6' should be recognized as valid IPv6" | ||
| logger.info("TP-064: full 39-char IPv6 '$fullIpv6' → isIpv6=$isV6 (accepted by interceptor)") |
There was a problem hiding this comment.
TP-064 没有验证 ipmiAddress 字段或数据库长度。
这里只检查了 39 字符字符串本身和 IPv6 解析结果，没有反射列定义，也没有持久化回读，因此无法证明字段/DB 真的能存下完整地址。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M3Case.groovy`
around lines 74 - 84, The test method testTP065_ipmiAddressFullLengthIpv6 only
validates the string and IPv6 parser but never assigns or persists ipmiAddress,
so extend the test to set the ipmiAddress field on the relevant entity,
save/persist it and reload from the database, then assert the reloaded entity's
ipmiAddress equals fullIpv6 and that its length is 39; use the existing fullIpv6
variable and IPv6NetworkUtils.isIpv6Address for pre-checks, and reference the
ipmiAddress field name and testTP065_ipmiAddressFullLengthIpv6 to locate where
to add the persistence and read-back assertions.
Comment on lines
+44
to
+82
| /** | ||
| * TP-042: ZSha2Helper.isMaster() 使用 " %s/" 模式（新 pattern）匹配 IPv6 VIP。 | ||
| * 模拟 `ip addr show` 输出，验证 " VIP/" 字符串匹配正确。 | ||
| */ | ||
| void testTP043_isMasterGrepPatternMatchesIpv6() { | ||
| String output = " inet6 2001:db8::100/64 scope global dynamic" | ||
| String vip = "2001:db8::100" | ||
|
|
||
| // TP-042: 新 pattern " VIP/" 应匹配 IPv6（冒号前后无空格，但 inet6 行含 " VIP/"） | ||
| String pattern = " ${vip}/" | ||
| assert output.contains(pattern) : | ||
| "TP-042: pattern ' IP/' should match IPv6 in 'ip addr show' output. pattern='$pattern'" | ||
|
|
||
| // 验证旧 pattern [^0-9]IP[^0-9] 也能匹配（: 是非数字字符） | ||
| String oldPatternBefore = output.substring(output.indexOf(vip) - 1, output.indexOf(vip)) | ||
| String oldPatternAfter = output.substring(output.indexOf(vip) + vip.length(), output.indexOf(vip) + vip.length() + 1) | ||
| assert !oldPatternBefore.matches("[0-9]") : | ||
| "TP-042: character before VIP in output should be non-digit (was: '$oldPatternBefore')" | ||
| assert !oldPatternAfter.matches("[0-9]") : | ||
| "TP-042: character after VIP in output should be non-digit (was: '$oldPatternAfter')" | ||
| logger.info("TP-042: pattern ' $vip/' matches IPv6 in ip addr output correctly") | ||
| } | ||
|
|
||
| /** | ||
| * TP-043: ZSha2Helper.isMaster() grep 逻辑正确——ip addr 输出包含 VIP 时判定为 master。 | ||
| * 复用 TP-042 的模拟逻辑，验证存在 VIP 时 contains 返回 true，不存在时返回 false。 | ||
| */ | ||
| void testTP042_isMasterGrepLogicWithVip() { | ||
| String vip = "2001:db8::100" | ||
| String outputWithVip = " inet6 2001:db8::100/64 scope global dynamic" | ||
| String outputWithoutVip = " inet6 fd00::1/64 scope global dynamic" | ||
|
|
||
| // TP-043: 含 VIP 的输出 → isMaster 应为 true | ||
| assert outputWithVip.contains(" ${vip}/") : | ||
| "TP-043: output containing VIP should be identified as master" | ||
| // 不含 VIP 的输出 → isMaster 应为 false | ||
| assert !outputWithoutVip.contains(" ${vip}/") : | ||
| "TP-043: output without VIP should not be identified as master" | ||
| logger.info("TP-043: isMaster grep logic for IPv6 VIP verified") |
There was a problem hiding this comment.
TP-042/043 这里没有真正测试到 ZSha2Helper.isMaster()。
当前只是对模拟输出做 contains(" ${vip}/") 断言，相当于把预期逻辑手写了一遍；如果 isMaster() 自身的命令拼接、转义或匹配条件回归，这两个用例仍然会通过。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovy`
around lines 44 - 82, Tests currently assert string.contains directly instead of
exercising ZSha2Helper.isMaster(), so they won't catch regressions in command
construction or matching; update the two test methods to call
ZSha2Helper.isMaster(vip) (or the instance method used in production) and assert
its return value, mocking/stubbing the underlying command executor that
ZSha2Helper uses to return outputWithVip and outputWithoutVip respectively;
specifically, in testTP043_isMasterGrepPatternMatchesIpv6 and
testTP042_isMasterGrepLogicWithVip, replace direct contains checks with calls to
ZSha2Helper.isMaster(...) and ensure the executor/runner class used by
ZSha2Helper is stubbed to return the simulated "inet6 ..." outputs so the method
behavior (not string literals) is validated.
Comment on lines
+157
to
+171
| /** | ||
| * TP-020: 39 字符全展开 IPv6 不被 DB 截断。 | ||
| * 与 TP-015 合并验证 @Column length >= 39。 | ||
| * 全展开 IPv6 最长为 "2001:0db8:0000:0000:0000:0000:0000:0001" = 39 字符。 | ||
| */ | ||
| void testTP020_fullIpv6FitsInColumn() { | ||
| String fullIpv6 = "2001:0db8:0000:0000:0000:0000:0000:0001" | ||
| assert fullIpv6.length() == 39 : "Precondition: full-expanded IPv6 should be 39 chars" | ||
|
|
||
| Field field = HostAO.class.getDeclaredField("managementIp") | ||
| field.setAccessible(true) | ||
| Column col = field.getAnnotation(Column.class) | ||
| assert col.length() >= fullIpv6.length() : | ||
| "TP-020: managementIp column length ${col.length()} is insufficient for 39-char full-expanded IPv6" | ||
| logger.info("TP-020: column length ${col.length()} >= 39, no truncation for full-expanded IPv6") |
There was a problem hiding this comment.
TP-020 目前没有真正覆盖“数据库不截断”的场景。
这里仍然只是读取 @Column.length，没有做一次真实的持久化/回读。如果实体注解和实际 schema 漂移，这个用例也会误报通过。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovy`
around lines 157 - 171, The test testTP020_fullIpv6FitsInColumn only inspects
the `@Column` length on HostAO.managementIp and must instead perform a real
persist-and-read to detect schema drift; update the test to create a Host (or
HostAO entity) with managementIp = "2001:0db8:0000:0000:0000:0000:0000:0001",
persist it using the same persistence path your suite uses (e.g.,
DatabaseFacade/hibernate session or existing test API that saves a Host),
flush/commit, reload the entity by ID, and assert the retrieved managementIp
equals the original 39-char value (and optionally assert length); then
delete/cleanup the test row. Ensure you reference HostAO.managementIp and the
test method name testTP020_fullIpv6FitsInColumn when making the change.
Comment on lines
+47
to
+59
| /** | ||
| * TP-032: NFS 存储 CIDR = IPv6 CIDR，验证 IPv6 CIDR 格式可被工具方法正确识别， | ||
| * 不会因 INVALID_ARGUMENT_ERROR 逻辑被拒绝。 | ||
| * 直接验证 CIDR 内的 IP 可通过 isIpInCidr 匹配。 | ||
| */ | ||
| void testTP032_nfsCidrIpv6NotRejected() { | ||
| String ipv6Cidr = "2001:db8::/64" | ||
| String ipv6InCidr = "2001:db8::1" | ||
|
|
||
| // TP-032: IPv6 CIDR 应能被正确解析，CIDR 内的 IP 匹配成功（不报错） | ||
| boolean result = NetworkUtils.isIpInCidr(ipv6InCidr, ipv6Cidr) | ||
| assert result : "TP-032: IPv6 address $ipv6InCidr should be in CIDR $ipv6Cidr (NFS IPv6 CIDR should not be rejected)" | ||
| logger.info("TP-032: IPv6 CIDR '$ipv6Cidr' recognized correctly, isIpInCidr='$result'") |
There was a problem hiding this comment.
TP-032/038 只覆盖了通用 helper，没有覆盖对应业务入口。
当前只是验证 NetworkUtils.isIpInCidr()；如果 NFS 创建或迁移网络检查在调用 helper 前后还有额外校验、分支或错误码处理，这个文件仍然会全部通过。
Also applies to: 123-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test/src/test/groovy/org/zstack/test/integration/network/ipv6/MnIpv6StorageMigrationCase.groovy`
around lines 47 - 59, The test currently only exercises the helper
NetworkUtils.isIpInCidr (in testTP032_nfsCidrIpv6NotRejected) but doesn't cover
the actual NFS storage create/migrate business paths that may add extra
validation; update the test to call the real NFS create and/or migration
entrypoints used by the product (the API/manager methods that validate CIDR
during NFS primary storage creation/migration) with ipv6Cidr = "2001:db8::/64"
and ipv6InCidr = "2001:db8::1", assert those operations succeed (no
INVALID_ARGUMENT_ERROR) and/or verify error codes/exception types are not
thrown; reference the production methods that perform the validation so the test
exercises the full business logic rather than only NetworkUtils.isIpInCidr.
Comment on lines
+564
to
+567
| public static boolean isIpInCidr(String ip, String cidr) { | ||
| if (ip == null || cidr == null) return false; | ||
| if (isIpv4Address(ip)) return isIpv4InCidr(ip, cidr); | ||
| if (IPv6NetworkUtils.isIpv6Address(ip)) return IPv6NetworkUtils.isIpv6InCidrRange(ip, cidr); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Apache Commons Net 的 SubnetUtils 是否仅支持 IPv4 CIDR？如果传入 IPv6 CIDR（例如 2001:db8::/64），构造 SubnetUtils 会不会抛 IllegalArgumentException？
💡 Result:
Yes, Apache Commons Net's SubnetUtils only supports IPv4 CIDR notation. It does not support IPv6. Passing an IPv6 CIDR like "2001:db8::/64" to the SubnetUtils constructor will throw an IllegalArgumentException because the CIDR pattern matcher expects the IPv4 format n.n.n.n/m (dotted decimal with 1-3 digits per octet and mask 0-32), which IPv6 does not match. IPv6 support is provided separately via SubnetUtils6 (since 3.13.0).
Citations:
- 1: https://issues.apache.org/jira/browse/NET-405
- 2: https://commons.apache.org/net/apidocs/org/apache/commons/net/util/SubnetUtils6.html
- 3: https://commons.apache.org/proper/commons-net/apidocs/org/apache/commons/net/util/SubnetUtils.html
- 4: https://commons.apache.org/proper/commons-net/jacoco/org.apache.commons.net.util/SubnetUtils.java.html
- 5: https://commons.apache.org/proper/commons-net/apidocs/src-html/org/apache/commons/net/util/SubnetUtils.html
🏁 Script executed:
# Read the full method including Javadoc to verify the Chinese comment claim
sed -n '555,575p' utils/src/main/java/org/zstack/utils/network/NetworkUtils.javaRepository: MatheMatrix/zstack
Length of output: 908
🏁 Script executed:
# Search for isCidr method in NetworkUtils
rg "isCidr\(" utils/src/main/java/org/zstack/utils/network/NetworkUtils.java -A 3Repository: MatheMatrix/zstack
Length of output: 2449
🏁 Script executed:
# Verify if isIpv4Address and isIpv6Address are proper checks
rg "public static boolean is(Ipv[46])Address" utils/src/main/java/org/zstack/utils/network/NetworkUtils.java -A 8Repository: MatheMatrix/zstack
Length of output: 388
🏁 Script executed:
# Check for IPv6Constants usage in the file
rg "IPv6Constants\." utils/src/main/java/org/zstack/utils/network/NetworkUtils.javaRepository: MatheMatrix/zstack
Length of output: 240
isIpInCidr 在混合协议族输入时会抛出运行时异常。
当 ip 为 IPv4 而 cidr 为 IPv6（例如 ip="192.168.1.1"，cidr="2001:db8::/64"）时，第 564 行条件进入 isIpv4InCidr(ip, cidr)，该方法内部使用 SubnetUtils 解析 CIDR。由于 SubnetUtils 仅支持 IPv4 CIDR 格式，传入 IPv6 CIDR 时直接抛出 IllegalArgumentException，而非返回 false。这将布尔判定工具方法转变为异常源，破坏上层参数校验的异常处理契约。
同时，第 559-561 行的 Javadoc 使用了中文，违反代码语言规范。
💡 建议修复
public static boolean isIpInCidr(String ip, String cidr) {
- if (ip == null || cidr == null) return false;
- if (isIpv4Address(ip)) return isIpv4InCidr(ip, cidr);
- if (IPv6NetworkUtils.isIpv6Address(ip)) return IPv6NetworkUtils.isIpv6InCidrRange(ip, cidr);
+ if (ip == null || cidr == null) {
+ return false;
+ }
+ if (isIpv4Address(ip)) {
+ return isCidr(cidr, IPv6Constants.IPv4) && isIpv4InCidr(ip, cidr);
+ }
+ if (IPv6NetworkUtils.isIpv6Address(ip)) {
+ return isCidr(cidr, IPv6Constants.IPv6) && IPv6NetworkUtils.isIpv6InCidrRange(ip, cidr);
+ }
return false;
}并将 Javadoc 改为英文：
/**
* Determines whether the given IP (IPv4 or IPv6) belongs to the specified CIDR.
*/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/src/main/java/org/zstack/utils/network/NetworkUtils.java` around lines
564 - 567, The isIpInCidr method can throw IllegalArgumentException when given
mixed-family inputs because isIpv4InCidr(ip, cidr) delegates to SubnetUtils
which only accepts IPv4 CIDRs; update isIpInCidr to first detect both ip and
cidr families (use isIpv4Address and IPv6NetworkUtils.isIpv6Address on the CIDR
string or add a helper to detect CIDR family) and only call isIpv4InCidr when
both are IPv4 and IPv6NetworkUtils.isIpv6InCidrRange when both are IPv6; if
families differ return false instead of calling the wrong parser. Also replace
the Chinese Javadoc above isIpInCidr (lines around the method) with an English
comment such as "Determines whether the given IP (IPv4 or IPv6) belongs to the
specified CIDR." and ensure no call sites rely on exceptions from mixed inputs.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
core/src/main/java/org/zstack/core/Platform.java (3)
896-902:⚠️ Potential issue | 🟠 Major注释应使用英文。
Lines 896-901 包含中文注释，需替换为英文。
♻️ 建议修改
- // F-002: 支持 IPv6 — 枚举所有网卡，按 PREFER_IPV6 配置决定优先级 + // Enumerate all network interfaces and select preferred IP family based on PREFER_IPV6 config boolean preferIpv6 = false; try { preferIpv6 = NetworkGlobalConfig.PREFER_IPV6.value(Boolean.class); } catch (Exception ignored) { - // GlobalConfig 可能在静态初始化阶段还未就绪，安全降级为 false + // GlobalConfig may not be ready during static initialization; safely default to false }As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/Platform.java` around lines 896 - 902, Replace the Chinese comment block above the preferIpv6 initialization with an English comment; specifically update the comment that currently reads in Chinese around the boolean preferIpv6 declaration in Platform (the block referencing enumerating network interfaces and PREFER_IPV6) to an English equivalent (e.g., "Support IPv6 — enumerate all network interfaces and choose priority based on PREFER_IPV6 configuration"), and ensure any accompanying Chinese comment in the try/catch (about GlobalConfig not being ready) is also translated to English mentioning safe fallback to false; keep references to NetworkGlobalConfig.PREFER_IPV6 and the preferIpv6 variable unchanged.
401-410:⚠️ Potential issue | 🟠 MajorJavadoc 注释应使用英文。
根据之前的审查反馈，此处的中文注释应已被替换为英文，但仍然存在中文内容。请确保所有注释都使用英文。
♻️ 建议修改
- /** - * F-010: JGroups initial_hosts IPv6 括号修复。 - * IPv6 地址在 JGroups 中须使用 [addr][port] 格式，IPv4 使用 addr[port] 格式。 - */ + /** + * Formats an IP address and port for JGroups initial_hosts configuration. + * IPv6 addresses require [addr][port] format, while IPv4 uses addr[port]. + */ private static String jgroupsAddr(String ip, String port) {As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/Platform.java` around lines 401 - 410, Replace the Chinese Javadoc above method jgroupsAddr with an English description; update the comment to explain that jgroupsAddr(String ip, String port) returns IPv6 addresses in the "[addr][port]" form and IPv4 as "addr[port]", and keep any reference to IPv6NetworkUtils.isIpv6Address intact so the intent and behavior of the method remain clear.
828-873:⚠️ Potential issue | 🟠 MajorgetManagementServerCidrIpv6 中的注释应使用英文，异常处理可更精确。
- 此方法中仍有中文注释（lines 829-830, 838, 858, 864），需替换为英文。
- Lines 863 和 869 的
catch (Exception)捕获范围较广，建议捕获更具体的异常类型如UnknownHostException、NumberFormatException。♻️ 建议修改
- /** - * F-003: 当管理 IP 为 IPv6 时，通过 'ip -6 addr show' 解析对应的网络 CIDR。 - */ + /** + * Resolves management server CIDR when the management IP is IPv6 + * by parsing 'ip -6 addr show' output. + */ private static String getManagementServerCidrIpv6(String mnIp) { try { Linux.ShellResult result = Linux.shell("ip -6 addr show"); if (result.getExitCode() != 0) { logger.warn(String.format("failed to run 'ip -6 addr show', exit code: %d", result.getExitCode())); return null; } - // 示例行: " inet6 2001:db8::1/64 scope global" + // Example line: " inet6 2001:db8::1/64 scope global" for (String line : result.getStdout().split("\n")) { // ... try { // ... - // 计算网络地址前缀，例如 2001:db8::/64 + // Compute network address prefix, e.g. 2001:db8::/64 // ... - } catch (Exception ignore) { - // 当前行解析失败，继续下一行 + } catch (UnknownHostException | NumberFormatException e) { + // Current line parsing failed, continue to next + logger.trace(String.format("failed to parse inet6 line: %s", line), e); } }As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/main/java/org/zstack/core/Platform.java` around lines 828 - 873, The method getManagementServerCidrIpv6 contains Chinese comments and overly broad exception catches; replace all Chinese comments in this method with clear English comments, and narrow the two catch blocks that currently use catch (Exception) to catch specific exceptions (e.g. UnknownHostException, NumberFormatException and any IPv6 parsing exception thrown by IPv6Network.fromString) while preserving the existing ignore/continuation logic for per-line errors and logging for the outer failure; update the logger.warn calls to remain English-only and keep use of Linux.shell, InetAddress.getByName, IPv6Network.fromString and parsing logic unchanged except for the refined exception types.
🧹 Nitpick comments (1)
utils/src/main/java/org/zstack/utils/network/IPv6Utils.java (1)
51-63: bracketIpv6 幂等性检查可考虑增强。当前实现仅检查
startsWith("[")，但对于畸形输入如[2001:db8::1（缺少闭合括号）会直接返回而不做修正。虽然这种输入在实际场景中不太可能出现，但可以考虑更严格的检查。此外，根据上下文代码片段，
IPv6NetworkUtils.isIpv6Address()可以接受带 zone ID 的地址（如fe80::1%eth0），但normalizeIpv6会剥离 zone ID。如果先对带 zone ID 的地址调用bracketIpv6，再调用normalizeIpv6，可能导致不一致的规范化形式。♻️ 可选增强
public static String bracketIpv6(String ip) { if (ip == null) { return ip; } - // 已经有方括号，直接返回（幂等） - if (ip.startsWith("[")) { + // Already bracketed, return as-is (idempotent) + if (ip.startsWith("[") && ip.endsWith("]")) { return ip; } if (IPv6NetworkUtils.isIpv6Address(ip)) { return "[" + ip + "]"; } return ip; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java` around lines 51 - 63, bracketIpv6 currently only checks startsWith("["), so malformed inputs like "[2001:db8::1" are not fixed and zone IDs can cause inconsistent normalization; update bracketIpv6 to (1) return null for null input as now, (2) detect a correctly bracketed address by both startsWith("[") and endsWith("]") and return it unchanged, (3) if only one bracket is present, add the missing bracket (prepend "[" if endsWith("]") only, append "]" if startsWith("[") only), (4) for validity checks call IPv6NetworkUtils.isIpv6Address on the unbracketed address (strip existing brackets first) but preserve any zone ID (e.g. %eth0) when returning the bracketed result, and (5) ensure compatibility with normalizeIpv6 by treating zone IDs consistently (use normalizeIpv6 on the stripped address for validation if desired, but reattach the original zone ID when constructing the final bracketed string); refer to bracketIpv6, IPv6NetworkUtils.isIpv6Address, and normalizeIpv6 when making changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java`:
- Around line 365-419: Update the comments in the getMnIpForVr method to use
English: replace the Javadoc block describing behavior (currently in Chinese)
with an accurate English description of the method, parameters, and return
value, and change the inline Chinese comment that explains ignoring CIDR/IP type
mismatches to an English comment (e.g., "ignore when CIDR and IP types do not
match"). Keep semantics unchanged and preserve references to IPv6NetworkUtils,
NetworkUtils, Platform.getManagementServerIp(), and the exception handling.
- Around line 529-557: Replace the Chinese inline comments in the
prepareBootstrapInformation code block (the comment starting with "F-009: 根据 VR
管理网 CIDR 选取与其同子网的 MN IP，支持多 IP/双栈主机" and any other Chinese comments around the
IPv6/IPv4 CIDR handling) with clear English comments describing the same
behavior: selecting the MN IP in the same subnet as the VR management CIDR and
supporting multiple IPs/dual-stack hosts; also change the warning message in the
IPv6 netmask parse catch to English (use the same variables shown:
mgmtNic.getNetmask(), mgmtNic.getIp(), vrMgmtCidr, getMnIpForVr, and
BootstrapParams.managementNodeIp) so all comments and logs in this block are in
English.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsApiParamChecker.java`:
- Around line 83-84: NfsApiParamChecker is extracting ipAddr using
url.split(":")[0] which breaks IPv6 (bracketed or not) and causes false
ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010 errors when calling
NetworkUtils.isIpInCidr; fix by parsing the NFS URL host properly (either update
checkUrl() or the place that computes ipAddr) to handle IPv6: if the URL starts
with '[', take the substring between '[' and ']' as the host; otherwise locate
the host portion before the path by finding the first '/' and then split
host/port by the last ':' only if a port is present; then pass that normalized
host (no brackets) to NetworkUtils.isIpInCidr so
isIpInCidr/argerr/ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010 behave correctly.
In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M4Case.groovy`:
- Around line 6-17: The file MnIpv6M4Case.groovy contains Chinese Javadoc and
inline comments (class-level Javadoc block and comments for methods like setup,
environment, clean and all test methods); replace all Chinese text with clear
English equivalents (e.g., translate the class Javadoc describing TP-083..TP-089
into English, and translate method and test comments) and ensure no Chinese
remains in log/error messages or annotations; update the Javadoc for the class
and each test/method (setup, environment, clean, and each test case) to concise
English descriptions that preserve the original intent (mentioning IPv6, bracket
notation, and the TP-* IDs) so code passes the coding guideline requiring no
Chinese.
In `@test/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovy`:
- Around line 6-17: The file ZSha2Ipv6Case.groovy contains Chinese Javadoc and
method comments; replace all Chinese comments with clear, idiomatic English in
the class-level Javadoc and every method comment (the ones documenting the
TP-042..TP-046 tests) so they follow project guidelines; specifically update the
class Javadoc for ZSha2Ipv6Case and the per-test method comments that describe
behavior for ZSha2Helper.isMaster(), bracketIpv6(), nginx upstream rendering
(IPv6 server line), and IAM URL IPv6 bracket handling, ensuring no Chinese
remains and using correct spelling/grammar.
In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java`:
- Around line 75-91: The comments in IPv6Utils.normalizeIpv6 are written in
Chinese; replace them with clear English comments without changing logic: change
"IPv4 原样返回" to "Return IPv4 as-is", change "去除 zone ID（e.g. fe80::1%eth0）" to
"Strip zone ID (e.g. fe80::1%eth0)", and change any other Chinese inline comment
(e.g. above input null/empty check) to an equivalent English phrase; keep the
method name, exception messages, and code behavior unchanged.
- Around line 15-21: Replace all Chinese comments in the IPv6Utils class with
clear English equivalents: update the class/method Javadoc and inline comments
for buildUrl, buildHttpsUrlWithPath, the helper that adds brackets for IPv6
(e.g., bracketIpv6IfNeeded), normalizeIpv6, stripZoneId, formatIpPort (both
overloads), and the IP management validation method (e.g., isValidManagementIp)
so each comment describes the same behavior in English (e.g., "Construct HTTP
URL; automatically add brackets for IPv6 addresses", "Construct HTTPS URL with
path", "Add brackets for IPv6 addresses if needed", "Normalize IPv6 address by
...", "Remove zone ID from IPv6 address", "Format ip:port", "Overload: format
ip:port with String port", "Validate whether IP can be used as management IP").
Keep punctuation and wording clear and concise, preserving existing method
descriptions and examples in the Javadoc.
---
Duplicate comments:
In `@core/src/main/java/org/zstack/core/Platform.java`:
- Around line 896-902: Replace the Chinese comment block above the preferIpv6
initialization with an English comment; specifically update the comment that
currently reads in Chinese around the boolean preferIpv6 declaration in Platform
(the block referencing enumerating network interfaces and PREFER_IPV6) to an
English equivalent (e.g., "Support IPv6 — enumerate all network interfaces and
choose priority based on PREFER_IPV6 configuration"), and ensure any
accompanying Chinese comment in the try/catch (about GlobalConfig not being
ready) is also translated to English mentioning safe fallback to false; keep
references to NetworkGlobalConfig.PREFER_IPV6 and the preferIpv6 variable
unchanged.
- Around line 401-410: Replace the Chinese Javadoc above method jgroupsAddr with
an English description; update the comment to explain that jgroupsAddr(String
ip, String port) returns IPv6 addresses in the "[addr][port]" form and IPv4 as
"addr[port]", and keep any reference to IPv6NetworkUtils.isIpv6Address intact so
the intent and behavior of the method remain clear.
- Around line 828-873: The method getManagementServerCidrIpv6 contains Chinese
comments and overly broad exception catches; replace all Chinese comments in
this method with clear English comments, and narrow the two catch blocks that
currently use catch (Exception) to catch specific exceptions (e.g.
UnknownHostException, NumberFormatException and any IPv6 parsing exception
thrown by IPv6Network.fromString) while preserving the existing
ignore/continuation logic for per-line errors and logging for the outer failure;
update the logger.warn calls to remain English-only and keep use of Linux.shell,
InetAddress.getByName, IPv6Network.fromString and parsing logic unchanged except
for the refined exception types.
---
Nitpick comments:
In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java`:
- Around line 51-63: bracketIpv6 currently only checks startsWith("["), so
malformed inputs like "[2001:db8::1" are not fixed and zone IDs can cause
inconsistent normalization; update bracketIpv6 to (1) return null for null input
as now, (2) detect a correctly bracketed address by both startsWith("[") and
endsWith("]") and return it unchanged, (3) if only one bracket is present, add
the missing bracket (prepend "[" if endsWith("]") only, append "]" if
startsWith("[") only), (4) for validity checks call
IPv6NetworkUtils.isIpv6Address on the unbracketed address (strip existing
brackets first) but preserve any zone ID (e.g. %eth0) when returning the
bracketed result, and (5) ensure compatibility with normalizeIpv6 by treating
zone IDs consistently (use normalizeIpv6 on the stripped address for validation
if desired, but reattach the original zone ID when constructing the final
bracketed string); refer to bracketIpv6, IPv6NetworkUtils.isIpv6Address, and
normalizeIpv6 when making changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: e6348892-7454-4b9c-b3cb-f5d719d1ca44
📒 Files selected for processing (25)
compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.javaconsole/src/main/java/org/zstack/console/ConsoleManagerImpl.javacore/src/main/java/org/zstack/core/Platform.javacore/src/main/java/org/zstack/core/config/NetworkGlobalConfig.javacore/src/main/java/org/zstack/core/rest/RESTFacadeImpl.javaplugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageMonBase.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KvmHostIpmiPowerExecutor.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsApiParamChecker.javaplugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.javatest/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/core/MnIpv6M3Case.groovytest/src/test/groovy/org/zstack/test/integration/core/MnIpv6M4Case.groovytest/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovytest/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/network/ipv6/MnIpv6StorageMigrationCase.groovytest/src/test/groovy/org/zstack/test/integration/network/vxlan/VxlanIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/utils/IPv6UtilsCase.groovyutils/src/main/java/org/zstack/utils/network/IPv6Utils.javautils/src/main/java/org/zstack/utils/network/NetworkUtils.javautils/src/main/java/org/zstack/utils/zsha2/ZSha2Helper.java
💤 Files with no reviewable changes (1)
- plugin/kvm/src/main/java/org/zstack/kvm/KvmHostIpmiPowerExecutor.java
✅ Files skipped from review due to trivial changes (2)
- plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
- test/src/test/groovy/org/zstack/test/integration/utils/IPv6UtilsCase.groovy
🚧 Files skipped from review as they are similar to previous changes (13)
- console/src/main/java/org/zstack/console/ConsoleManagerImpl.java
- core/src/main/java/org/zstack/core/config/NetworkGlobalConfig.java
- compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java
- utils/src/main/java/org/zstack/utils/network/NetworkUtils.java
- test/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovy
- plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java
- test/src/test/groovy/org/zstack/test/integration/appliancevm/ApplianceVmIpv6Case.groovy
- test/src/test/groovy/org/zstack/test/integration/network/vxlan/VxlanIpv6Case.groovy
- test/src/test/groovy/org/zstack/test/integration/network/ipv6/MnIpv6StorageMigrationCase.groovy
- core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java
- test/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovy
- test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M3Case.groovy
- plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
Comment on lines
+365
to
+419
| /** | ||
| * F-009: 从本机网卡中选取与 VR 管理网 CIDR 同子网的 MN IP。 | ||
| * 支持 IPv4/IPv6 双栈；若无匹配则回退到 Platform.getManagementServerIp()。 | ||
| * | ||
| * @param vrManagementCidr VR 管理网的网络 CIDR，例如 "192.168.1.0/24" 或 "2001:db8::/64" | ||
| * @return 选定的 MN IP（裸地址，无括号） | ||
| */ | ||
| private String getMnIpForVr(String vrManagementCidr) { | ||
| if (vrManagementCidr == null || vrManagementCidr.isEmpty()) { | ||
| logger.warn("VR management CIDR is null/empty, fallback to getManagementServerIp()"); | ||
| return Platform.getManagementServerIp(); | ||
| } | ||
| try { | ||
| Enumeration<NetworkInterface> ifaces = NetworkInterface.getNetworkInterfaces(); | ||
| if (ifaces != null) { | ||
| while (ifaces.hasMoreElements()) { | ||
| NetworkInterface iface = ifaces.nextElement(); | ||
| if (iface.isLoopback() || !iface.isUp()) { | ||
| continue; | ||
| } | ||
| Enumeration<InetAddress> addrs = iface.getInetAddresses(); | ||
| while (addrs.hasMoreElements()) { | ||
| InetAddress addr = addrs.nextElement(); | ||
| if (addr.isLoopbackAddress() || addr.isLinkLocalAddress()) { | ||
| continue; | ||
| } | ||
| String ipStr = addr.getHostAddress(); | ||
| try { | ||
| boolean inCidr; | ||
| if (IPv6NetworkUtils.isIpv6Address(ipStr)) { | ||
| inCidr = IPv6NetworkUtils.isIpv6InCidrRange(ipStr, vrManagementCidr); | ||
| } else { | ||
| inCidr = NetworkUtils.isIpv4InCidr(ipStr, vrManagementCidr); | ||
| } | ||
| if (inCidr) { | ||
| logger.debug(String.format( | ||
| "selected MN IP [%s] matching VR management CIDR [%s]", | ||
| ipStr, vrManagementCidr)); | ||
| return ipStr; | ||
| } | ||
| } catch (Exception ignore) { | ||
| // CIDR 类型与 IP 类型不匹配时忽略 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (SocketException e) { | ||
| logger.warn(String.format("failed to enumerate network interfaces: %s", e.getMessage())); | ||
| } | ||
| String fallback = Platform.getManagementServerIp(); | ||
| logger.warn(String.format( | ||
| "no MN IP matched VR management CIDR [%s], fallback to [%s]", | ||
| vrManagementCidr, fallback)); | ||
| return fallback; | ||
| } |
There was a problem hiding this comment.
方法注释和内部注释应使用英文。
getMnIpForVr 方法的 Javadoc（lines 366-371）和内部注释（line 405-406）包含中文，需替换为英文。
♻️ 建议修改
- /**
- * F-009: 从本机网卡中选取与 VR 管理网 CIDR 同子网的 MN IP。
- * 支持 IPv4/IPv6 双栈；若无匹配则回退到 Platform.getManagementServerIp()。
- *
- * `@param` vrManagementCidr VR 管理网的网络 CIDR，例如 "192.168.1.0/24" 或 "2001:db8::/64"
- * `@return` 选定的 MN IP（裸地址，无括号）
- */
+ /**
+ * Selects a local MN IP that belongs to the same subnet as the VR management CIDR.
+ * Supports IPv4/IPv6 dual-stack; falls back to Platform.getManagementServerIp() if no match.
+ *
+ * `@param` vrManagementCidr VR management network CIDR, e.g. "192.168.1.0/24" or "2001:db8::/64"
+ * `@return` selected MN IP (bare address, no brackets)
+ */
private String getMnIpForVr(String vrManagementCidr) {
// ...
} catch (Exception ignore) {
- // CIDR 类型与 IP 类型不匹配时忽略
+ // Ignore when CIDR type doesn't match IP type
}As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java`
around lines 365 - 419, Update the comments in the getMnIpForVr method to use
English: replace the Javadoc block describing behavior (currently in Chinese)
with an accurate English description of the method, parameters, and return
value, and change the inline Chinese comment that explains ignoring CIDR/IP type
mismatches to an English comment (e.g., "ignore when CIDR and IP types do not
match"). Keep semantics unchanged and preserve references to IPv6NetworkUtils,
NetworkUtils, Platform.getManagementServerIp(), and the exception handling.
Comment on lines
+529
to
+557
|
|
||
| // F-009: 根据 VR 管理网 CIDR 选取与其同子网的 MN IP，支持多 IP/双栈主机 | ||
| String vrMgmtCidr; | ||
| if (IPv6NetworkUtils.isIpv6Address(mgmtNic.getIp())) { | ||
| int prefixLen = 64; | ||
| try { | ||
| String netmask = mgmtNic.getNetmask(); | ||
| if (netmask != null) { | ||
| try { | ||
| prefixLen = Integer.parseInt(netmask.trim()); | ||
| } catch (NumberFormatException ignore) { | ||
| if (IPv6NetworkUtils.isIpv6Address(netmask)) { | ||
| prefixLen = IPv6NetworkMask.fromAddress(IPv6Address.fromString(netmask)).asPrefixLength(); | ||
| } | ||
| } | ||
| } | ||
| } catch (Exception ignored) { | ||
| logger.warn(String.format("failed to parse IPv6 netmask [%s] for VR nic IP [%s], using default prefix /64", | ||
| mgmtNic.getNetmask(), mgmtNic.getIp())); | ||
| } | ||
| String networkAddr = IPv6Network.fromAddressAndMask( | ||
| IPv6Address.fromString(mgmtNic.getIp()), | ||
| IPv6NetworkMask.fromPrefixLength(prefixLen)).getFirst().toString(); | ||
| vrMgmtCidr = networkAddr + "/" + prefixLen; | ||
| } else { | ||
| vrMgmtCidr = NetworkUtils.getCidrFromIpMask(mgmtNic.getIp(), mgmtNic.getNetmask()); | ||
| } | ||
| String mnIp = getMnIpForVr(vrMgmtCidr); | ||
| ret.put(BootstrapParams.managementNodeIp.toString(), mnIp); |
There was a problem hiding this comment.
prepareBootstrapInformation 中的注释应使用英文。
Lines 530 和 546-547 包含中文注释，需替换为英文。
♻️ 建议修改
- // F-009: 根据 VR 管理网 CIDR 选取与其同子网的 MN IP，支持多 IP/双栈主机
+ // Select MN IP matching VR management CIDR, supports multi-IP/dual-stack hosts
String vrMgmtCidr;
if (IPv6NetworkUtils.isIpv6Address(mgmtNic.getIp())) {
// ...
} catch (Exception ignored) {
- logger.warn(String.format("failed to parse IPv6 netmask [%s] for VR nic IP [%s], using default prefix /64",
+ logger.warn(String.format("failed to parse IPv6 netmask [%s] for VR NIC IP [%s], using default prefix /64",
mgmtNic.getNetmask(), mgmtNic.getIp()));
}As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java`
around lines 529 - 557, Replace the Chinese inline comments in the
prepareBootstrapInformation code block (the comment starting with "F-009: 根据 VR
管理网 CIDR 选取与其同子网的 MN IP，支持多 IP/双栈主机" and any other Chinese comments around the
IPv6/IPv4 CIDR handling) with clear English comments describing the same
behavior: selecting the MN IP in the same subnet as the VR management CIDR and
supporting multiple IPs/dual-stack hosts; also change the warning message in the
IPv6 netmask parse catch to English (use the same variables shown:
mgmtNic.getNetmask(), mgmtNic.getIp(), vrMgmtCidr, getMnIpForVr, and
BootstrapParams.managementNodeIp) so all comments and logs in this block are in
English.
Comment on lines
+83
to
84
| if (!NetworkUtils.isIpInCidr(ipAddr, cidr)) { | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010, "IP address[%s] is not in CIDR[%s]", ipAddr, cidr)); |
There was a problem hiding this comment.
IPv6 场景下这里仍可能误判“IP 不在 CIDR 内”。
Line 83 改成 isIpInCidr 是正确方向，但 ipAddr 上游仍来自 checkUrl() 的 url.split(":")[0]。当 URL 是 IPv6 NFS 地址（例如 [2001:db8::10]:/data 或 2001:db8::10:/data）时，ipAddr 会被截断，导致这里稳定抛出 ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010，IPv6 网关 CIDR 校验无法通过。
建议修复（最小改动）
- String[] results = url.split(":");
- if (results.length == 2 && (
- results[1].startsWith("/dev") || results[1].startsWith("/proc") || results[1].startsWith("/sys"))) {
+ int pathSeparatorIndex = url.lastIndexOf(":/");
+ String ipOrHost = pathSeparatorIndex > 0 ? url.substring(0, pathSeparatorIndex) : url;
+ String exportPath = pathSeparatorIndex > 0 ? url.substring(pathSeparatorIndex + 1) : "";
+ if (exportPath.startsWith("/dev") || exportPath.startsWith("/proc") || exportPath.startsWith("/sys")) {
throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_STORAGE_PRIMARY_NFS_10007, " the url contains an invalid folder[/dev or /proc or /sys]"));
}
-
- validateUrl(systemTags, results[0]);
+ if (ipOrHost.startsWith("[") && ipOrHost.endsWith("]")) {
+ ipOrHost = ipOrHost.substring(1, ipOrHost.length() - 1);
+ }
+ validateUrl(systemTags, ipOrHost);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!NetworkUtils.isIpInCidr(ipAddr, cidr)) { | |
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010, "IP address[%s] is not in CIDR[%s]", ipAddr, cidr)); | |
| int pathSeparatorIndex = url.lastIndexOf(":/"); | |
| String ipOrHost = pathSeparatorIndex > 0 ? url.substring(0, pathSeparatorIndex) : url; | |
| String exportPath = pathSeparatorIndex > 0 ? url.substring(pathSeparatorIndex + 1) : ""; | |
| if (exportPath.startsWith("/dev") || exportPath.startsWith("/proc") || exportPath.startsWith("/sys")) { | |
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_STORAGE_PRIMARY_NFS_10007, " the url contains an invalid folder[/dev or /proc or /sys]")); | |
| } | |
| if (ipOrHost.startsWith("[") && ipOrHost.endsWith("]")) { | |
| ipOrHost = ipOrHost.substring(1, ipOrHost.length() - 1); | |
| } | |
| validateUrl(systemTags, ipOrHost); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsApiParamChecker.java`
around lines 83 - 84, NfsApiParamChecker is extracting ipAddr using
url.split(":")[0] which breaks IPv6 (bracketed or not) and causes false
ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010 errors when calling
NetworkUtils.isIpInCidr; fix by parsing the NFS URL host properly (either update
checkUrl() or the place that computes ipAddr) to handle IPv6: if the URL starts
with '[', take the substring between '[' and ']' as the host; otherwise locate
the host portion before the path by finding the first '/' and then split
host/port by the last ':' only if a port is present; then pass that normalized
host (no brackets) to NetworkUtils.isIpInCidr so
isIpInCidr/argerr/ORG_ZSTACK_STORAGE_PRIMARY_NFS_10010 behave correctly.
Comment on lines
+6
to
+17
| /** | ||
| * TP-083~089: 管理节点 IPv6 M4 Premium 支持测试 | ||
| * | ||
| * 全部为纯单元测试，无需 Spring 上下文。 | ||
| * 覆盖以下测试点： | ||
| * TP-083 - ZWatch InfluxDB URL 含 IPv6 方括号（buildUrl vs String.format 对比） | ||
| * TP-084 - Prometheus remote_write URL 含 IPv6 方括号（路径拼接正确） | ||
| * TP-085 - Grafana 数据源 URL 含 IPv6 方括号（buildUrl vs String.format 对比） | ||
| * TP-087 - License HTTP URL 含 IPv6 方括号（bracketIpv6 + buildHttpsUrl） | ||
| * TP-088 - Keycloak 容器名 IPv6 地址 sanitize（冒号替换为短横线） | ||
| * TP-089 - SSO CAS URL 含 IPv6 方括号（bracketIpv6 用于 HTTPS URL 拼接） | ||
| */ |
There was a problem hiding this comment.
测试类的注释应使用英文。
根据编码规范，代码中不应包含中文。此测试文件包含大量中文注释，包括：
- 类级别 Javadoc (lines 7-17)
- setup/environment/clean 方法注释 (lines 22, 27, 32)
- 所有测试方法的 Javadoc 和内部注释
♻️ 建议修改示例
/**
- * TP-083~089: 管理节点 IPv6 M4 Premium 支持测试
- *
- * 全部为纯单元测试，无需 Spring 上下文。
- * 覆盖以下测试点：
- * TP-083 - ZWatch InfluxDB URL 含 IPv6 方括号（buildUrl vs String.format 对比）
- * ...
+ * TP-083~089: Management node IPv6 M4 Premium support tests
+ *
+ * All are pure unit tests, no Spring context required.
+ * Coverage:
+ * TP-083 - ZWatch InfluxDB URL with IPv6 brackets (buildUrl vs String.format comparison)
+ * ...
*/As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6M4Case.groovy`
around lines 6 - 17, The file MnIpv6M4Case.groovy contains Chinese Javadoc and
inline comments (class-level Javadoc block and comments for methods like setup,
environment, clean and all test methods); replace all Chinese text with clear
English equivalents (e.g., translate the class Javadoc describing TP-083..TP-089
into English, and translate method and test comments) and ensure no Chinese
remains in log/error messages or annotations; update the Javadoc for the class
and each test/method (setup, environment, clean, and each test case) to concise
English descriptions that preserve the original intent (mentioning IPv6, bracket
notation, and the TP-* IDs) so code passes the coding guideline requiring no
Chinese.
Comment on lines
+6
to
+17
| /** | ||
| * TP-042~046: ZSha2 高可用 IPv6 支持测试 | ||
| * | ||
| * 全部为纯单元测试，无需 Spring 上下文。 | ||
| * | ||
| * 覆盖： | ||
| * TP-042 - ZSha2Helper.isMaster() grep pattern " ip/" 正确匹配 IPv6 VIP | ||
| * TP-043 - ZSha2Helper.isMaster() grep 逻辑正确（含 VIP 的 ip addr 输出返回 true） | ||
| * TP-044 - Zsha2 SSH/SCP 命令含 [IPv6] 括号：bracketIpv6() 工具正确 | ||
| * TP-045 - nginx upstream 渲染：MN IP = IPv6 → "server [ipv6]:port;" | ||
| * TP-046 - IAM URL 含 IPv6 括号 | ||
| */ |
There was a problem hiding this comment.
测试类的注释应使用英文。
根据编码规范，代码中不应包含中文。此测试文件包含大量中文注释，需要替换为英文：
- 类级别 Javadoc (lines 7-17)
- 方法注释 (lines 22, 27, 32, 44-47, 52, 67-70, 85-87, 101-103, 117-119)
♻️ 建议修改示例
/**
- * TP-042~046: ZSha2 高可用 IPv6 支持测试
- *
- * 全部为纯单元测试，无需 Spring 上下文。
- *
- * 覆盖：
- * TP-042 - ZSha2Helper.isMaster() grep pattern " ip/" 正确匹配 IPv6 VIP
- * TP-043 - ZSha2Helper.isMaster() grep 逻辑正确（含 VIP 的 ip addr 输出返回 true）
- * TP-044 - Zsha2 SSH/SCP 命令含 [IPv6] 括号：bracketIpv6() 工具正确
- * TP-045 - nginx upstream 渲染：MN IP = IPv6 → "server [ipv6]:port;"
- * TP-046 - IAM URL 含 IPv6 括号
+ * TP-042~046: ZSha2 HA IPv6 support tests
+ *
+ * All are pure unit tests, no Spring context required.
+ *
+ * Coverage:
+ * TP-042 - ZSha2Helper.isMaster() grep pattern " ip/" correctly matches IPv6 VIP
+ * TP-043 - ZSha2Helper.isMaster() grep logic correct (output with VIP returns true)
+ * TP-044 - Zsha2 SSH/SCP commands contain [IPv6] brackets: bracketIpv6() utility correct
+ * TP-045 - nginx upstream rendering: MN IP = IPv6 → "server [ipv6]:port;"
+ * TP-046 - IAM URL contains IPv6 brackets
*/As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/src/test/groovy/org/zstack/test/integration/core/ZSha2Ipv6Case.groovy`
around lines 6 - 17, The file ZSha2Ipv6Case.groovy contains Chinese Javadoc and
method comments; replace all Chinese comments with clear, idiomatic English in
the class-level Javadoc and every method comment (the ones documenting the
TP-042..TP-046 tests) so they follow project guidelines; specifically update the
class Javadoc for ZSha2Ipv6Case and the per-test method comments that describe
behavior for ZSha2Helper.isMaster(), bracketIpv6(), nginx upstream rendering
(IPv6 server line), and IAM URL IPv6 bracket handling, ensuring no Chinese
remains and using correct spelling/grammar.
Comment on lines
+15
to
+21
| /** | ||
| * 构造 HTTP URL。自动为 IPv6 地址加方括号。 | ||
| * <pre> | ||
| * buildUrl("192.168.1.1", 8080) → "http://192.168.1.1:8080" | ||
| * buildUrl("2001:db8::1", 8080) → "http://[2001:db8::1]:8080" | ||
| * buildUrl("host.example", 8080) → "http://host.example:8080" | ||
| * </pre> |
There was a problem hiding this comment.
Javadoc 和注释应使用英文。
根据编码规范，代码中不应包含中文（包括注释和错误信息）。此文件中存在多处中文注释，需要替换为英文：
- Line 16-17:
构造 HTTP URL。自动为 IPv6 地址加方括号。 - Line 27-28:
构造 HTTPS URL，带路径。 - Line 42-44:
为 IPv6 地址加方括号... - Line 65-67:
规范化 IPv6 地址... - Line 83:
去除 zone ID... - Line 93-94:
格式化 ip:port... - Line 104-105:
格式化 ip:port（String port 重载）... - Line 111-112:
校验 IP 是否可用作管理 IP...
♻️ 建议修改示例
/**
- * 构造 HTTP URL。自动为 IPv6 地址加方括号。
+ * Builds an HTTP URL. Automatically brackets IPv6 addresses.
* <pre>
* buildUrl("192.168.1.1", 8080) → "http://192.168.1.1:8080"
* buildUrl("2001:db8::1", 8080) → "http://[2001:db8::1]:8080"
* buildUrl("host.example", 8080) → "http://host.example:8080"
* </pre>
*/As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java` around lines 15
- 21, Replace all Chinese comments in the IPv6Utils class with clear English
equivalents: update the class/method Javadoc and inline comments for buildUrl,
buildHttpsUrlWithPath, the helper that adds brackets for IPv6 (e.g.,
bracketIpv6IfNeeded), normalizeIpv6, stripZoneId, formatIpPort (both overloads),
and the IP management validation method (e.g., isValidManagementIp) so each
comment describes the same behavior in English (e.g., "Construct HTTP URL;
automatically add brackets for IPv6 addresses", "Construct HTTPS URL with path",
"Add brackets for IPv6 addresses if needed", "Normalize IPv6 address by ...",
"Remove zone ID from IPv6 address", "Format ip:port", "Overload: format ip:port
with String port", "Validate whether IP can be used as management IP"). Keep
punctuation and wording clear and concise, preserving existing method
descriptions and examples in the Javadoc.
Comment on lines
+75
to
+91
| public static String normalizeIpv6(String ip) { | ||
| if (ip == null || ip.isEmpty()) { | ||
| throw new IllegalArgumentException("invalid IPv6 address: " + ip); | ||
| } | ||
| // IPv4 原样返回 | ||
| if (!ip.contains(":")) { | ||
| return ip; | ||
| } | ||
| // 去除 zone ID（e.g. fe80::1%eth0） | ||
| String stripped = ip.contains("%") ? ip.split("%")[0] : ip; | ||
| try { | ||
| InetAddress addr = InetAddress.getByName(stripped); | ||
| return addr.getHostAddress(); | ||
| } catch (UnknownHostException e) { | ||
| throw new IllegalArgumentException("invalid IPv6 address: " + ip, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
normalizeIpv6 内的中文注释需替换为英文。
Line 79, 83, 89 包含中文注释，需要替换为英文。
♻️ 建议修改
public static String normalizeIpv6(String ip) {
if (ip == null || ip.isEmpty()) {
throw new IllegalArgumentException("invalid IPv6 address: " + ip);
}
- // IPv4 原样返回
+ // Return IPv4 as-is
if (!ip.contains(":")) {
return ip;
}
- // 去除 zone ID（e.g. fe80::1%eth0）
+ // Strip zone ID (e.g. fe80::1%eth0)
String stripped = ip.contains("%") ? ip.split("%")[0] : ip;
try {
InetAddress addr = InetAddress.getByName(stripped);
return addr.getHostAddress();
} catch (UnknownHostException e) {
throw new IllegalArgumentException("invalid IPv6 address: " + ip, e);
}
}As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/src/main/java/org/zstack/utils/network/IPv6Utils.java` around lines 75
- 91, The comments in IPv6Utils.normalizeIpv6 are written in Chinese; replace
them with clear English comments without changing logic: change "IPv4 原样返回" to
"Return IPv4 as-is", change "去除 zone ID（e.g. fe80::1%eth0）" to "Strip zone ID
(e.g. fe80::1%eth0)", and change any other Chinese inline comment (e.g. above
input null/empty check) to an equivalent English phrase; keep the method name,
exception messages, and code behavior unchanged.
- Platform.java: translate Chinese comments to English; narrow catch (Exception) to UnknownHostException|NumberFormatException and IOException - SdnControllerManagerImpl: batch-load L3/L2NetworkVO before loop to fix N+1 - MnIpv6Case: TP-004 asserts IPv4; TP-005 actually sets PREFER_IPV6=true; TP-022 asserts bracket format; TP-023/024 remove silent logger.warn pass Resolves: ZSTAC-79206 Change-Id: I
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.java`:
- Around line 691-699: 在构建 l3VoMap 和 l2VoMap 时直接调用 dbf.listByPrimaryKeys 会在传入的
UUID 集合为空时退化为全表查询；在使用 dbf.listByPrimaryKeys(new ArrayList<>(l3Uuids),
L3NetworkVO.class) 前先判断 l3Uuids 是否为空（或 size==0），若为空则直接初始化为空的
Map<String,L3NetworkVO>；同理在调用 dbf.listByPrimaryKeys(new ArrayList<>(l2Uuids),
L2NetworkVO.class) 前判断 l2Uuids 并在为空时直接返回空的
Map<String,L2NetworkVO>，避免全表扫描和内存激增；参考定位符：l3Uuids、l3VoMap、l2Uuids、l2VoMap、dbf.listByPrimaryKeys。
In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovy`:
- Around line 14-34: The file contains Chinese comments and Javadoc that must be
converted to English to meet repository language standards; update all Chinese
text in the MnIpv6Case.groovy top block and all in-file comments/Javadocs
(locations noted around the class header and other comment blocks at the
previously flagged locations) into clear, idiomatic English while preserving the
original meaning (test descriptions like TP-001..TP-031, explanations of what
each test covers, and inline comments). Search for the class/filename
MnIpv6Case.groovy and replace each Chinese comment/Javadoc (including the header
block and the scattered comments near tests and helper methods) with equivalent
English text, keeping test identifiers and technical terms unchanged and
ensuring no spelling errors. Ensure formatting and line breaks in Javadoc remain
valid and run a quick grep/inspection to confirm no remaining non-ASCII/Chinese
characters in comments.
- Around line 121-135: The test toggles NetworkGlobalConfig.PREFER_IPV6 but may
still hit a previously populated static cache, so ensure the
Platform.getManagementServerIp() path for PREFER_IPV6=true is actually exercised
by saving the current cached value of Platform.managementServerIp, clearing that
cache before calling Platform.getManagementServerIp(), then restoring the
original cached value in the finally block; specifically reference
clearing/restoring the static cache (managementServerIp) around the call to
Platform.getManagementServerIp() so the method recomputes instead of returning a
stale value.
- Around line 171-175: Remove the fragile network.contains(".") check and make
the CIDR family decision explicit: use NetworkUtils.isIpv4Address(network) to
handle IPv4 (assert prefix in 0-32), use NetworkUtils.isIpv6Address(network) to
handle IPv6 (assert prefix in 0-128), and add an else branch that fails
(throws/asserts) for unknown/invalid address family; update the block around
NetworkUtils.isIpv4Address(network) and the prefix assertions accordingly in
MnIpv6Case.groovy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 9ac55a55-c245-4e3d-af53-a2f5477e2cea
📒 Files selected for processing (3)
core/src/main/java/org/zstack/core/Platform.javaplugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.javatest/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovy
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/org/zstack/core/Platform.java
Comment on lines
+171
to
+175
| if (NetworkUtils.isIpv4Address(network) || network.contains(".")) { | ||
| assert prefix >= 0 && prefix <= 32 : "TP-007: IPv4 prefix should be 0-32, got: $prefix" | ||
| } else { | ||
| assert prefix >= 0 && prefix <= 128 : "TP-007: IPv6 prefix should be 0-128, got: $prefix" | ||
| } |
There was a problem hiding this comment.
CIDR 地址家族判断建议去掉 network.contains(".")。
Line 171 用字符串包含点号推断 IPv4 容易误判，建议改为“显式 IPv4 / 显式 IPv6 / 否则失败”三分支，避免前缀范围断言落入错误分支。
建议修复（示例）
- if (NetworkUtils.isIpv4Address(network) || network.contains(".")) {
+ if (NetworkUtils.isIpv4Address(network)) {
assert prefix >= 0 && prefix <= 32 : "TP-007: IPv4 prefix should be 0-32, got: $prefix"
- } else {
+ } else if (IPv6NetworkUtils.isIpv6Address(network)) {
assert prefix >= 0 && prefix <= 128 : "TP-007: IPv6 prefix should be 0-128, got: $prefix"
+ } else {
+ assert false : "TP-007: CIDR network part should be valid IPv4/IPv6, got: $network"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/src/test/groovy/org/zstack/test/integration/core/MnIpv6Case.groovy`
around lines 171 - 175, Remove the fragile network.contains(".") check and make
the CIDR family decision explicit: use NetworkUtils.isIpv4Address(network) to
handle IPv4 (assert prefix in 0-32), use NetworkUtils.isIpv6Address(network) to
handle IPv6 (assert prefix in 0-128), and add an else branch that fails
(throws/asserts) for unknown/invalid address family; update the block around
NetworkUtils.isIpv4Address(network) and the prefix assertions accordingly in
MnIpv6Case.groovy.
…sets Resolves: ZSTAC-79206 Change-Id: I1d78094a568645d941f9c8a30b5582f42a86a209
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves ZSTAC-79206
sync from gitlab !9739