management network IPv6 waterfall M1/M2#4036
Open
zstack-robot-2 wants to merge 14 commits into
Open
Conversation
added 2 commits
May 21, 2026 12:36
Allow APIAddHost managementIp to accept IPv6 literals and canonicalize them. Format KVM agent URLs with IPv6 brackets and cover add-host validation with KVM IPv6 case. Resolves: ZSTAC-79206 Change-Id: I6cdaabc8c7d7d62161565b5df6a02ecc61f787b2
Implement waterfall M1/M2 management network IPv6 support. Resolves: ZSTAC-79206 Change-Id: Icfeba77200168f8ff05139824166243a87d4b10d
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough该 PR 为管理网络引入 IPv6 支持：新增 IPv6NetworkUtils 和双栈 CIDR 判断，添加 PREFER_IPV6 全局配置，扩展 Platform 的管理 IP/CIDR 解析与持久化，更新 API 校验并在多个插件（KVM、Ceph、REST、NFS、VXLAN）中统一使用 IPv6 友好的格式化/校验，并补充集成测试覆盖。 变更IPv6 管理网络完整支持
🎯 4 (Complex) | ⏱️ ~45 minutes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
utils/src/main/java/org/zstack/utils/network/IPv6NetworkUtils.java (1)
515-517: 💤 Low value建议为
normalizeIpv6添加空值检查以保持一致性。既然
isIpv6Address和formatHostForUrl已添加 null 防护，normalizeIpv6也应处理 null 输入以保持 API 一致性。当前如果传入 null，IPv6Address.fromString(null)会抛出异常。♻️ 建议的改进
public static String normalizeIpv6(String ip) { + if (ip == null) { + return null; + } return getIpv6AddressCanonicalString(ip); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/src/main/java/org/zstack/utils/network/IPv6NetworkUtils.java` around lines 515 - 517, normalizeIpv6 currently forwards to getIpv6AddressCanonicalString without null protection, which lets IPv6Address.fromString(null) throw; add a null check at the start of normalizeIpv6 (if ip == null) return null to match the null-safe behavior of isIpv6Address and formatHostForUrl, then otherwise call getIpv6AddressCanonicalString(ip).plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java (1)
138-140: ⚡ Quick win建议为公共方法添加 Javadoc 文档。
此方法是公共 API，应当配有 Javadoc 说明其用途、参数和返回值。特别是对于处理 IPv6 地址格式化这种非平凡逻辑，文档应说明：
- 输入参数的预期格式（IPv4、IPv6 或已括号化的 IPv6）
- 返回值的格式（IPv4 原样、IPv6 添加方括号）
- 是否对已括号化的 IPv6 幂等
📝 建议添加的 Javadoc
+ /** + * Format a host address (IPv4 or IPv6) for use in URLs. + * IPv6 addresses are wrapped in square brackets; IPv4 addresses are returned unchanged. + * Already-bracketed IPv6 addresses are returned as-is (idempotent). + * + * `@param` host the host address (IPv4, IPv6, or bracketed IPv6) + * `@return` the URL-safe host format + */ public static String formatHostForUrl(String host) { return IPv6NetworkUtils.formatHostForUrl(host); }根据编码规范：接口方法与公共方法应当配有有效的 Javadoc 注释。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java` around lines 138 - 140, Add a Javadoc block to the public method formatHostForUrl explaining its purpose and contract: state that it formats a host string for use in URLs by delegating to IPv6NetworkUtils.formatHostForUrl, document the expected input formats (IPv4, IPv6, and already-bracketed IPv6), describe the return value (IPv4 returned unchanged, IPv6 returned with surrounding brackets), and note idempotence (calling formatHostForUrl on an already-bracketed IPv6 is a no-op). Include `@param` host and `@return` tags and a short example for IPv4 vs IPv6 to clarify behavior, referencing the formatHostForUrl method name and the delegation to IPv6NetworkUtils.formatHostForUrl.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java`:
- Around line 127-134: In validate(APIAddHostMsg msg) trim the incoming
management IP first and operate on the trimmed value: retrieve
msg.getManagementIp(), null-check it, call String.trim() and set it back into
msg (e.g., msg.setManagementIp(trimmed)), then call
isValidManagementEndpoint(trimmed) and later pass the trimmed value to
IPv6NetworkUtils.getIpv6AddressCanonicalString; update references in the
validate method so validation and normalization always use the trimmed
managementIp.
In `@core/src/main/java/org/zstack/core/Platform.java`:
- Around line 825-834: The current getManagementServerCidr(int) can pick an IP
from any enabled NIC (first match) instead of the IP bound to the management
interface; update the logic so when the primary current IP doesn't match the
requested ipVersion you fetch the management-interface-specific address for that
version (use the management-bound getters getManagementServerIp6() /
getManagementServerIp4() or a helper that returns the IP tied to the configured
management NIC) and then call getManagementServerCidr(ip) with that management
interface IP; ensure you do not iterate over all NIC addresses or return the
first version-matching address from the system interfaces.
In
`@plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java`:
- Around line 58-60: Trim and canonicalize the RemoteVtepIp before validation
and before any string equality checks: in
validate(APICreateVxlanPoolRemoteVtepMsg msg) (and the analogous places
referenced around the same methods), replace direct uses of
msg.getRemoteVtepIp() with a normalized value obtained by trimming whitespace
and converting to canonical form (e.g. using
InetAddress.getByName(trimmed).getHostAddress() or a
NetworkUtils.normalize/canonicalize helper), use that normalized IP for
NetworkUtils.isIpAddress validation, for duplicate detection, for error
messages, and write the normalized IP back into the message or use it
consistently so IPv6 equivalent forms no longer bypass checks or break
deletions.
---
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java`:
- Around line 138-140: Add a Javadoc block to the public method formatHostForUrl
explaining its purpose and contract: state that it formats a host string for use
in URLs by delegating to IPv6NetworkUtils.formatHostForUrl, document the
expected input formats (IPv4, IPv6, and already-bracketed IPv6), describe the
return value (IPv4 returned unchanged, IPv6 returned with surrounding brackets),
and note idempotence (calling formatHostForUrl on an already-bracketed IPv6 is a
no-op). Include `@param` host and `@return` tags and a short example for IPv4 vs
IPv6 to clarify behavior, referencing the formatHostForUrl method name and the
delegation to IPv6NetworkUtils.formatHostForUrl.
In `@utils/src/main/java/org/zstack/utils/network/IPv6NetworkUtils.java`:
- Around line 515-517: normalizeIpv6 currently forwards to
getIpv6AddressCanonicalString without null protection, which lets
IPv6Address.fromString(null) throw; add a null check at the start of
normalizeIpv6 (if ip == null) return null to match the null-safe behavior of
isIpv6Address and formatHostForUrl, then otherwise call
getIpv6AddressCanonicalString(ip).
🪄 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: bef24941-3ca9-49fa-a22c-0fc1c94ce185
📒 Files selected for processing (20)
compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.javacore/src/main/java/org/zstack/core/NetworkGlobalConfig.javacore/src/main/java/org/zstack/core/Platform.javacore/src/main/java/org/zstack/core/rest/RESTFacadeImpl.javaplugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmConstant.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/CephPrimaryStorageBase.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.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/KVMHostUtils.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/core/ManagementNetworkIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.javautils/src/main/java/org/zstack/utils/network/IPv6NetworkUtils.javautils/src/main/java/org/zstack/utils/network/NetworkUtils.javautils/src/main/java/org/zstack/utils/zsha2/ZSha2Helper.java
Comment on lines
127
to
134
| private void validate(APIAddHostMsg msg) { | ||
| if (!NetworkUtils.isIpv4Address(msg.getManagementIp()) && !NetworkUtils.isHostname(msg.getManagementIp())) { | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_HOST_10113, "managementIp[%s] is neither an IPv4 address nor a valid hostname", msg.getManagementIp())); | ||
| if (!isValidManagementEndpoint(msg.getManagementIp())) { | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_HOST_10128, INVALID_MANAGEMENT_IP_ERROR, msg.getManagementIp())); | ||
| } | ||
|
|
||
| if (IPv6NetworkUtils.isIpv6Address(msg.getManagementIp())) { | ||
| msg.setManagementIp(IPv6NetworkUtils.getIpv6AddressCanonicalString(msg.getManagementIp())); | ||
| } |
There was a problem hiding this comment.
先 trim 再做管理地址校验与规范化
Line 128/132 直接使用原始 managementIp。若用户复制粘贴带空格/换行，会误判为非法或写入非预期值。
💡 建议修改
private void validate(APIAddHostMsg msg) {
- if (!isValidManagementEndpoint(msg.getManagementIp())) {
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_HOST_10128, INVALID_MANAGEMENT_IP_ERROR, msg.getManagementIp()));
+ String managementIp = msg.getManagementIp() == null ? null : msg.getManagementIp().trim();
+ msg.setManagementIp(managementIp);
+ if (!isValidManagementEndpoint(managementIp)) {
+ throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_HOST_10128, INVALID_MANAGEMENT_IP_ERROR, managementIp));
}
- if (IPv6NetworkUtils.isIpv6Address(msg.getManagementIp())) {
- msg.setManagementIp(IPv6NetworkUtils.getIpv6AddressCanonicalString(msg.getManagementIp()));
+ if (IPv6NetworkUtils.isIpv6Address(managementIp)) {
+ msg.setManagementIp(IPv6NetworkUtils.getIpv6AddressCanonicalString(managementIp));
}
}As per coding guidelines "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java` around
lines 127 - 134, In validate(APIAddHostMsg msg) trim the incoming management IP
first and operate on the trimmed value: retrieve msg.getManagementIp(),
null-check it, call String.trim() and set it back into msg (e.g.,
msg.setManagementIp(trimmed)), then call isValidManagementEndpoint(trimmed) and
later pass the trimmed value to IPv6NetworkUtils.getIpv6AddressCanonicalString;
update references in the validate method so validation and normalization always
use the trimmed managementIp.
Comment on lines
+58
to
+60
| private void validate(APICreateVxlanPoolRemoteVtepMsg msg) { | ||
| boolean isIpv4 = NetworkUtils.isIpv4Address(msg.getRemoteVtepIp()); | ||
| if (!isIpv4) { | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10015, "%s:is not ipv4", msg.getRemoteVtepIp())); | ||
| if (!NetworkUtils.isIpAddress(msg.getRemoteVtepIp())) { | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10015, "%s is not a valid IP address", msg.getRemoteVtepIp())); |
There was a problem hiding this comment.
IPv6 未规范化会绕过重复校验并影响删除匹配
Line 66 依赖字符串精确匹配，但 Line 59/75 仅做“是否是 IP”校验。IPv6 不同等价写法会被当成不同值，导致重复数据或删除不到目标记录。
💡 建议修改
+import org.zstack.utils.network.IPv6NetworkUtils;
+
private void validate(APICreateVxlanPoolRemoteVtepMsg msg) {
- if (!NetworkUtils.isIpAddress(msg.getRemoteVtepIp())) {
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10015, "%s is not a valid IP address", msg.getRemoteVtepIp()));
+ String remoteVtepIp = msg.getRemoteVtepIp() == null ? null : msg.getRemoteVtepIp().trim();
+ if (IPv6NetworkUtils.isIpv6Address(remoteVtepIp)) {
+ remoteVtepIp = IPv6NetworkUtils.getIpv6AddressCanonicalString(remoteVtepIp);
+ }
+ msg.setRemoteVtepIp(remoteVtepIp);
+ if (!NetworkUtils.isIpAddress(remoteVtepIp)) {
+ throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10015, "%s is not a valid IP address", remoteVtepIp));
}
...
}
private void validate(APIDeleteVxlanPoolRemoteVtepMsg msg) {
- if (!NetworkUtils.isIpAddress(msg.getRemoteVtepIp())) {
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10017, "%s is not a valid IP address", msg.getRemoteVtepIp()));
+ String remoteVtepIp = msg.getRemoteVtepIp() == null ? null : msg.getRemoteVtepIp().trim();
+ if (IPv6NetworkUtils.isIpv6Address(remoteVtepIp)) {
+ remoteVtepIp = IPv6NetworkUtils.getIpv6AddressCanonicalString(remoteVtepIp);
+ }
+ msg.setRemoteVtepIp(remoteVtepIp);
+ if (!NetworkUtils.isIpAddress(remoteVtepIp)) {
+ throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10017, "%s is not a valid IP address", remoteVtepIp));
}
}As per coding guidelines "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"。
Also applies to: 63-67, 75-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java`
around lines 58 - 60, Trim and canonicalize the RemoteVtepIp before validation
and before any string equality checks: in
validate(APICreateVxlanPoolRemoteVtepMsg msg) (and the analogous places
referenced around the same methods), replace direct uses of
msg.getRemoteVtepIp() with a normalized value obtained by trimming whitespace
and converting to canonical form (e.g. using
InetAddress.getByName(trimmed).getHostAddress() or a
NetworkUtils.normalize/canonicalize helper), use that normalized IP for
NetworkUtils.isIpAddress validation, for duplicate detection, for error
messages, and write the normalized IP back into the message or use it
consistently so IPv6 equivalent forms no longer bypass checks or break
deletions.
Use new VXLAN error code constants for remote VTEP invalid IP validation so the new messages do not reuse existing ErrorCodes. Resolves: ZSTAC-79206 Change-Id: Ibd6ea314eda79a25e56f3e75d13e0101ca0c4b44
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java (2)
58-72:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win缺少 trim 和 IPv6 规范化导致重复校验失效与数据一致性问题
Line 59 仅校验 IP 格式但未规范化，Line 66 依赖字符串精确匹配。存在两个严重问题：
- 未对输入参数执行 trim 操作：违反编码规范要求"注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"
- IPv6 地址未规范化：不同等价写法（如
2001:db8::1与2001:0db8:0000:0000:0000:0000:0000:0001）会被当作不同值，导致：
- 重复校验失效，可创建重复的远端 VTEP 记录
- 数据库中存储非标准格式的 IPv6 地址
- 后续删除操作可能因格式不匹配而失败
🔧 建议的修复方案
+import org.zstack.utils.network.IPv6NetworkUtils; + private void validate(APICreateVxlanPoolRemoteVtepMsg msg) { + String remoteVtepIp = msg.getRemoteVtepIp(); + if (remoteVtepIp != null) { + remoteVtepIp = remoteVtepIp.trim(); + if (IPv6NetworkUtils.isIpv6Address(remoteVtepIp)) { + remoteVtepIp = IPv6NetworkUtils.getIpv6AddressCanonicalString(remoteVtepIp); + } + msg.setRemoteVtepIp(remoteVtepIp); + } if (!NetworkUtils.isIpAddress(msg.getRemoteVtepIp())) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10032, "%s is not a valid IP address", msg.getRemoteVtepIp())); }As per coding guidelines "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java` around lines 58 - 72, Trim and normalize the remote VTEP IP before validation and DB queries in validate(APICreateVxlanPoolRemoteVtepMsg msg): take msg.getRemoteVtepIp(), apply trim, parse it as an InetAddress and obtain a canonical textual form (e.g. InetAddress.getByName(...).getHostAddress()) to normalize IPv6, then use that normalizedIp for NetworkUtils.isIpAddress check and for the SimpleQuery condition on VtepVO_.vtepIp; also ensure the normalized value is what gets persisted (or set back on the message if mutable) so stored and comparison values are consistent.
74-79:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win缺少 trim 和 IPv6 规范化导致删除操作可能失败
Line 75 存在与创建校验相同的问题：
- 未对输入参数执行 trim 操作：违反编码规范
- IPv6 地址未规范化：如果数据库中存储的是 IPv6 地址的某种格式（如
2001:db8::1），而用户提供等价但不同的格式（如2001:0db8:0000:0000:0000:0000:0000:0001），删除操作将因字符串不匹配而无法找到目标记录🔧 建议的修复方案
private void validate(APIDeleteVxlanPoolRemoteVtepMsg msg) { + String remoteVtepIp = msg.getRemoteVtepIp(); + if (remoteVtepIp != null) { + remoteVtepIp = remoteVtepIp.trim(); + if (IPv6NetworkUtils.isIpv6Address(remoteVtepIp)) { + remoteVtepIp = IPv6NetworkUtils.getIpv6AddressCanonicalString(remoteVtepIp); + } + msg.setRemoteVtepIp(remoteVtepIp); + } if (!NetworkUtils.isIpAddress(msg.getRemoteVtepIp())) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_L2_VXLAN_VXLANNETWORKPOOL_10033, "%s is not a valid IP address", msg.getRemoteVtepIp())); } }As per coding guidelines "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java` around lines 74 - 79, In validate(APIDeleteVxlanPoolRemoteVtepMsg msg) inside VxlanPoolApiInterceptor, trim the incoming msg.getRemoteVtepIp() and replace the raw value with a canonical/normalized IP string before validation and DB lookup; specifically, perform msg.setRemoteVtepIp(msg.getRemoteVtepIp().trim()) and then normalize IPv6 using the existing NetworkUtils normalization helper (e.g., NetworkUtils.normalizeIpAddress or equivalent canonicalization method) so NetworkUtils.isIpAddress() checks the cleaned value and deletion will match stored IPv6 canonical forms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.java`:
- Around line 58-72: Trim and normalize the remote VTEP IP before validation and
DB queries in validate(APICreateVxlanPoolRemoteVtepMsg msg): take
msg.getRemoteVtepIp(), apply trim, parse it as an InetAddress and obtain a
canonical textual form (e.g. InetAddress.getByName(...).getHostAddress()) to
normalize IPv6, then use that normalizedIp for NetworkUtils.isIpAddress check
and for the SimpleQuery condition on VtepVO_.vtepIp; also ensure the normalized
value is what gets persisted (or set back on the message if mutable) so stored
and comparison values are consistent.
- Around line 74-79: In validate(APIDeleteVxlanPoolRemoteVtepMsg msg) inside
VxlanPoolApiInterceptor, trim the incoming msg.getRemoteVtepIp() and replace the
raw value with a canonical/normalized IP string before validation and DB lookup;
specifically, perform msg.setRemoteVtepIp(msg.getRemoteVtepIp().trim()) and then
normalize IPv6 using the existing NetworkUtils normalization helper (e.g.,
NetworkUtils.normalizeIpAddress or equivalent canonicalization method) so
NetworkUtils.isIpAddress() checks the cleaned value and deletion will match
stored IPv6 canonical forms.
ℹ️ 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: cff868de-edf8-4a5f-925c-9c37a2f8266d
📒 Files selected for processing (2)
plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanPoolApiInterceptor.javautils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
Select appliance VM bootstrap managementNodeIp by VR management CIDR and reject non-routable IPv6 management endpoints. Resolves: ZSTAC-79206 Change-Id: If4f857d07b043519fc2f6a7b752246f35d0ced8e
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy (1)
43-45: ⚡ Quick win方法名称与实现不符，建议重命名或补充断言
方法名
testPreferIpv6DefaultFalse()暗示会测试默认值为false，但实际只校验了配置项的 identity 字符串。建议：
- 重命名为
testPreferIpv6Identity()以准确反映其意图- 或添加
assert !Platform.isManagementServerPreferIpv6()来实际测试默认值🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy` around lines 43 - 45, The test method name testPreferIpv6DefaultFalse() doesn't match its implementation (it only asserts NetworkGlobalConfig.PREFER_IPV6.getIdentity()); either rename the method to testPreferIpv6Identity() to reflect that it checks the identity string, or add an assertion to test the actual default value by calling Platform.isManagementServerPreferIpv6() (e.g., assert !Platform.isManagementServerPreferIpv6()) so the method name and behavior align; update the method name or add the extra assertion in the testPreferIpv6DefaultFalse / testPreferIpv6Identity method accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy`:
- Around line 43-45: The test method name testPreferIpv6DefaultFalse() doesn't
match its implementation (it only asserts
NetworkGlobalConfig.PREFER_IPV6.getIdentity()); either rename the method to
testPreferIpv6Identity() to reflect that it checks the identity string, or add
an assertion to test the actual default value by calling
Platform.isManagementServerPreferIpv6() (e.g., assert
!Platform.isManagementServerPreferIpv6()) so the method name and behavior align;
update the method name or add the extra assertion in the
testPreferIpv6DefaultFalse / testPreferIpv6Identity method accordingly.
ℹ️ 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: a0ef1b22-dd4a-4c6a-921b-35beca761d91
📒 Files selected for processing (6)
core/src/main/java/org/zstack/core/NetworkGlobalConfig.javacore/src/main/java/org/zstack/core/Platform.javaplugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.javatest/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/kvm/host/KvmHostIpv6Case.groovyutils/src/main/java/org/zstack/utils/network/IPv6NetworkUtils.java
added 2 commits
May 21, 2026 19:50
Persist managementServerId across management IP changes and add IPv6-safe REST URL helpers with focused coverage. Resolves: ZSTAC-79206 Change-Id: I03d57df9dd199aba5875f1726cb5cc1cc4b1e32e
Add focused coverage for IPv4 and IPv6 management CIDR parsing from ip address command output. Resolves: ZSTAC-79206 Change-Id: Id3776bc971679c3e1b88f14b1f8a19ced6621098
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/src/main/java/org/zstack/core/Platform.java (1)
889-897:⚠️ Potential issue | 🟠 Major | ⚡ Quick win把 IPv4/IPv6 回退地址绑定到管理网卡。
Line 896 在协议族不匹配时会回退到
getManagementServerIp6()/getManagementServerIp4()，但 Line 947-984 是扫描所有已启用网卡并返回第一个匹配地址，不是当前管理 IP 所在接口的地址。多网卡场景下，getManagementServerCidr(int)、getManagementServerIp6Cidr()和getManagementServerIps()都可能拿到非管理网段的地址/网段，导致下游传播错误的 management CIDR/IP。Also applies to: 947-999
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/main/java/org/zstack/core/Platform.java` around lines 889 - 897, The fallback in getManagementServerCidr(int) uses getManagementServerIp6()/getManagementServerIp4() which scans all NICs and returns the first matching address, potentially from a different interface than the current management IP; change the fallback logic to resolve the opposite-family address on the same network interface as the current management IP (i.e. locate the NIC/interface owning getManagementServerIp(), then look up its IPv6/IPv4 address and compute CIDR from that address). Apply the same interface-bound lookup strategy to the related helpers getManagementServerIp6Cidr() and getManagementServerIps() so they always return addresses/CIDRs from the management IP’s interface rather than any enabled NIC.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/src/main/java/org/zstack/core/Platform.java`:
- Around line 512-513: 当前实现调用 loadOrCreateManagementServerId（并使用
Platform::getUuid）在静态初始化时会回写 classpath 下的
zstack.properties，从而把只读配置路径变为可写并可能覆盖用户文件；请修改该逻辑使其不再直接写回 zstack.properties：在
loadOrCreateManagementServerId 中先尝试从 classpath 读取 managementServerId，若缺失则生成新的
ID（Platform::getUuid）但将运行时状态持久化到一个专用的状态文件/目录（例如管理服务器状态文件而非
zstack.properties），并确保写操作只针对该状态文件；同时检查并修复代码中其它类似写回 classpath
配置的地方（与本处相同的逻辑调用点），保证平台启动不会修改或覆盖原始 zstack.properties。
In
`@test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy`:
- Around line 160-176: The test method testManagementServerIdPersisted reads but
does not clear the JVM system property Platform.MANAGEMENT_SERVER_ID_PROPERTY
before calling Platform.loadOrCreateManagementServerId, which can make
generatedId come from an existing environment; modify the test to clear
System.clearProperty(Platform.MANAGEMENT_SERVER_ID_PROPERTY) (or remove it)
before the first call to Platform.loadOrCreateManagementServerId and ensure the
original value (oldValue) is restored in the finally block so the test is
isolated; refer to the testManagementServerIdPersisted method and the
Platform.loadOrCreateManagementServerId / Platform.MANAGEMENT_SERVER_ID_PROPERTY
symbols when implementing the change.
---
Duplicate comments:
In `@core/src/main/java/org/zstack/core/Platform.java`:
- Around line 889-897: The fallback in getManagementServerCidr(int) uses
getManagementServerIp6()/getManagementServerIp4() which scans all NICs and
returns the first matching address, potentially from a different interface than
the current management IP; change the fallback logic to resolve the
opposite-family address on the same network interface as the current management
IP (i.e. locate the NIC/interface owning getManagementServerIp(), then look up
its IPv6/IPv4 address and compute CIDR from that address). Apply the same
interface-bound lookup strategy to the related helpers
getManagementServerIp6Cidr() and getManagementServerIps() so they always return
addresses/CIDRs from the management IP’s interface rather than any enabled NIC.
🪄 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: 6a8d4a72-d14c-4a9b-9982-2dad6478b8cb
📒 Files selected for processing (3)
core/src/main/java/org/zstack/core/Platform.javacore/src/main/java/org/zstack/core/rest/RESTFacadeImpl.javatest/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy
Comment on lines
+512
to
+513
| // get ms id should after global property setup | ||
| msId = loadOrCreateManagementServerId(globalPropertiesFile, Platform::getUuid); |
There was a problem hiding this comment.
不要在启动时回写 zstack.properties。
如果 managementServerId 缺失，这里会在静态初始化阶段把新值写回 classpath 上的 zstack.properties。这会把原本“只读加载配置”的启动路径变成“必须能修改配置文件”：只读部署、镜像内置配置或受外部配置管理托管的场景都会直接启动失败，而且还会重写用户维护的主配置文件。更稳妥的是把这个运行时状态落到专用状态文件，而不是回写主配置。
As per coding guidelines, "向后兼容原则 之前的代码产生的行为不要直接去改动，即使是 bug 也可能用户有依赖，不应直接改动" and "平台不应直接覆盖用户可能自定义修改过的东西，这个非常重要".
Also applies to: 720-745, 760-775
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/src/main/java/org/zstack/core/Platform.java` around lines 512 - 513,
当前实现调用 loadOrCreateManagementServerId（并使用 Platform::getUuid）在静态初始化时会回写 classpath
下的 zstack.properties，从而把只读配置路径变为可写并可能覆盖用户文件；请修改该逻辑使其不再直接写回 zstack.properties：在
loadOrCreateManagementServerId 中先尝试从 classpath 读取 managementServerId，若缺失则生成新的
ID（Platform::getUuid）但将运行时状态持久化到一个专用的状态文件/目录（例如管理服务器状态文件而非
zstack.properties），并确保写操作只针对该状态文件；同时检查并修复代码中其它类似写回 classpath
配置的地方（与本处相同的逻辑调用点），保证平台启动不会修改或覆盖原始 zstack.properties。
added 8 commits
May 21, 2026 20:01
Add focused management IPv6 coverage for NFS URL parsing, Ceph mon URL parsing, and VXLAN VTEP validation. Resolves: ZSTAC-79206 Change-Id: Ia2925ec043a3b2be4d1e3b921ad285a8aebc9aed
Add focused IPv6 coverage for KVM extra IP CIDR selection used by data and migration network paths. Resolves: ZSTAC-79206 Change-Id: I03ea5987d0e4ebc4fdd78dc261297baaf12d0832
Add focused ApplianceVm IPv6 bootstrap JSON tests for MN IP and CIDR fields. Resolves: ZSTAC-79206 Change-Id: Icdbe40c69af9306eb2663a12031774c1b4c64195
Avoid lambda bytecode that blocks KvmHostIpv6Case initialization before IPv6 assertions. Resolves: ZSTAC-79206 Change-Id: I2d1fdced05d1127f6414e4ae0adc491e10b23d7d
Add focused coverage for host management IPv6 canonicalization and invalid endpoint detection without starting the full KVM Spring harness. Resolves: ZSTAC-79206 Change-Id: I731c2168a22e5b52f993319f21835ca9a456a76b
Add API and DB assertions for IPv6 remote VXLAN VTEP creation and invalid remote VTEP rejection. Resolves: ZSTAC-79206 Change-Id: Ieb2368b5f4a274bcad5b192003ac051ae4179552
Avoid relying only on Configurable injection for GlobalConfig instances created during GlobalConfigFacadeImpl startup. Resolves: ZSTAC-79206 Change-Id: I3f975fc23240d88ba08dc870220277cc065f7cfb
Make startup-only objects use explicit facades instead of AspectJ injection during unit tests.\n\nMove allocator and VM flow extension lookup to runtime to avoid early plugin registry access.\n\nAllow IPv6 remote VTEP addresses through API and SDK validation and cover host/VTEP IPv6 cases.\n\nVerification:\n- mvn -Dtest=KvmHostIpv6Case test\n- mvn -Dtest=AddRemoteVxlanVtepIpCase test\n- ./runMavenProfile premium Change-Id: I0146cfab99160e1f784170d051444e4fec69ad83
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
1 participant
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.
Implement management network IPv6 features from approved waterfall PRD and Func Spec M1/M2. ZSTAC-79206 is used only for branch and Jira association. Joint branch set uses @@5; zstack-distro is standalone.
sync from gitlab !9932