<fix>[network]: set nic ip out of l3 cidr scope#3301
Open
zstack-robot-2 wants to merge 4 commits into5.5.6from
Open
<fix>[network]: set nic ip out of l3 cidr scope#3301zstack-robot-2 wants to merge 4 commits into5.5.6from
zstack-robot-2 wants to merge 4 commits into5.5.6from
Conversation
APIImpact DBImpact GlobalConfigImpact Resolves: ZSTAC-81969 Change-Id: I736e77646266696e646271766d7170627378796b
Resolves: ZSTAC-81969 Change-Id: I6f6c7a72647373667577787a7869686c656b6472
Walkthrough该变更新增 VM 网卡静态 DNS 的系统标签支持与持久化，允许（可配置）在 L3 网络 IP 范围外创建静态 IP，并在数据库、DHCP、验证拦截器与相关 API/消息中加入相应字段与校验、迁移与回退逻辑（含 UsedIp.prefixLen 列与外键行为修改）。 Changes
Sequence Diagram(s)sequenceDiagram
participant API as API请求
participant VmBase as VmInstanceBase
participant StaticIp as StaticIpOperator
participant DB as 数据库
participant NetSvc as NetworkService
API->>VmBase: APISetVmStaticIpMsg(ip, dnsAddresses)
VmBase->>StaticIp: 内部 SetVmStaticIpMsg(..., dnsAddresses)
VmBase->>StaticIp: setStaticDns(vmUuid, l3Uuid, dnsAddresses)
StaticIp->>StaticIp: 验证 DNS 格式(IPv4/IPv6)
StaticIp->>DB: 创建/更新 STATIC_DNS 系统标签
StaticIp-->>VmBase: 返回成功
VmBase->>DB: 保存/提交 IP 变更
VmBase-->>API: 返回完成
Note over NetSvc,DB: DNS 查询优先级：NIC 系统标签 > L3 DNS
NetSvc->>DB: 查询 VM NIC STATIC_DNS 标签
alt NIC 有 STATIC_DNS
DB-->>NetSvc: 返回 NIC 级 DNS 列表
else
NetSvc->>DB: 查询 L3 网络 DNS
DB-->>NetSvc: 返回 L3 DNS 列表
end
sequenceDiagram
participant API as API请求
participant Interceptor as 验证拦截器(EIP/LB/PF)
participant DB as 数据库
participant Service as 目标服务
API->>Interceptor: 创建/绑定 请求(包含 usedIpUuid)
Interceptor->>DB: 查询 UsedIpVO(by usedIpUuid)
Interceptor->>Interceptor: 检查 usedIpVO.ipRangeUuid 是否为 null
alt ipRangeUuid == null
Interceptor-->>API: 抛出 ApiMessageInterceptionException（拒绝绑定）
else
Interceptor->>Service: 继续后续处理
Service-->>API: 返回成功
end
预估代码审查工作量🎯 4 (复杂) | ⏱️ ~60 分钟 诗句
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java (1)
298-311:⚠️ Potential issue | 🔴 Critical方法返回类型声明与实现不匹配，导致编译错误
方法签名声明返回
Map<String, NicIpAddressInfo>，但实现创建并返回的是Map<Integer, String>。经检查，所有调用此方法的地方（共5处）都期望接收Map<Integer, String>：
TfPortService.java:54FlatDhcpBackend.java:1753VmAllocateNicIpFlow.java:113VmInstanceBase.java:6640VmAllocateNicForStartingVmFlow.java:88方法体使用
Integer类型的键（IPv6Constants.IPv4、IPv6Constants.IPv6）存储 IP 字符串，与NicIpAddressInfo无关。此错误会导致代码无法编译。修复方案： 将方法签名改为
public Map<Integer, String> getNicStaticIpMap(List<String> nicStaticIpList)，与实现和所有调用方保持一致。
🤖 Fix all issues with AI agents
In `@compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java`:
- Around line 284-296: In getStaticDnsByVmUuidAndL3Uuid, make the l3Uuid
comparison null-safe and return an empty list instead of null: guard against
tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN) being null by using
l3Uuid.equals(uuid) or an explicit null check before comparing, and change the
method to return Collections.emptyList() (or new ArrayList<>()) when no DNS
entries are found to match the behavior of getStaticIpbyVmUuid; update
references to L3NetworkSystemTags.STATIC_DNS_TOKEN handling accordingly.
In
`@network/src/main/java/org/zstack/network/service/NetworkServiceManagerImpl.java`:
- Around line 491-499: The loop in NetworkServiceManagerImpl that compares
uuid.equals(l3NetworkUuid) risks NPE if
tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN) returns null; change
the check to use l3NetworkUuid.equals(uuid) or explicitly null-check uuid before
comparing, and ensure you still retrieve and validate the STATIC_DNS_TOKEN
string (dnsStr) before splitting to avoid downstream errors (refer to the
variables uuid, l3NetworkUuid and keys
L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN /
L3NetworkSystemTags.STATIC_DNS_TOKEN).
In
`@plugin/eip/src/main/java/org/zstack/network/service/eip/EipApiInterceptor.java`:
- Around line 205-212: In EipApiInterceptor update the error message string used
when throwing ApiMessageInterceptionException for a UsedIpVO with null
ipRangeUuid: locate the block that calls dbf.findByUuid(msg.getUsedIpUuid(),
UsedIpVO.class) and currently throws
ApiMessageInterceptionException(argerr("cannot bindBind EIP to IP address[%s]
...", usedIpVO.getIp())); and correct the typo to "cannot bind EIP to IP
address[%s] which is outside L3 network CIDR range" (retain the argerr usage and
the inserted usedIpVO.getIp() parameter).
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java`:
- Around line 645-657: Replace the per-NIC DB lookups and single-usedIp check
with batch queries and full-usedIp iteration: fetch all VmNicVOs for
msg.getVmNicUuids() in one dbf query, collect all UsedIpVO UUIDs from each
VmNicVO.getUsedIps() (or from getUsedIpUuids if available), batch-load those
UsedIpVOs in one dbf.findByUuids call, then for each VmNicVO iterate its
associated UsedIpVOs and throw ApiMessageInterceptionException using the
ORG_ZSTACK_NETWORK_SERVICE_LB_XXXXX error constant when any UsedIpVO has
ipRangeUuid == null; update the code in LoadBalancerApiInterceptor where the
loop over msg.getVmNicUuids() currently lives to use these batched collections
and iteration to avoid looped queries and to handle dual-stack IPs.
In
`@plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingApiInterceptor.java`:
- Around line 152-160: The exception message in PortForwardingApiInterceptor
when checking nicVO/usedIpVO mistakenly contains "bindBind" and should be
reworded for clarity; update the ApiMessageInterceptionException thrown via
argerr to a clear single-voice message (e.g., "cannot bind port forwarding rule
to IP address [%s] because it is outside the L3 network CIDR range") referencing
usedIpVO.getIp(), keeping the same exception type and placement so nicVO,
usedIpVO, ApiMessageInterceptionException and argerr remain unchanged except for
the corrected message text.
In
`@plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupManagerImpl.java`:
- Around line 169-178: The query in getVmIpsBySecurityGroup unconditionally
excludes rows with ip.ipRangeUuid == null which drops NICs on L3s with
enableIPAM=false or IPs allowed out-of-range; change the WHERE so the
ipRangeUuid check only applies when the L3 has IPAM enabled and disallows
out-of-range addresses. Concretely, join UsedIpVO (alias ip) to L3NetworkVO and
replace the final clause "and ip.ipRangeUuid is not null" with a predicate like
"(l3.enableIpam = false OR (l3.enableIpam = true AND ip.ipRangeUuid is not
null))" (adjust field names to the actual enableIPAM/allowOver fields) so
getVmIpsBySecurityGroup returns IPs for L3s without IPAM and still filters only
when IPAM is enabled and range-enforced.
🧹 Nitpick comments (3)
conf/db/upgrade/V5.5.7__schema.sql (2)
5-8: 表名和列名建议使用反引号包裹根据编码规范，为避免 MySQL 8.0 / GreatSQL 保留关键字冲突，所有表名和列名应使用反引号包裹。
♻️ 建议修复
-UPDATE UsedIpVO u -INNER JOIN IpRangeVO r ON u.ipRangeUuid = r.uuid -SET u.prefixLen = r.prefixLen -WHERE u.ipVersion = 6 AND u.ipRangeUuid IS NOT NULL AND u.prefixLen IS NULL; +UPDATE `UsedIpVO` u +INNER JOIN `IpRangeVO` r ON u.`ipRangeUuid` = r.`uuid` +SET u.`prefixLen` = r.`prefixLen` +WHERE u.`ipVersion` = 6 AND u.`ipRangeUuid` IS NOT NULL AND u.`prefixLen` IS NULL;As per coding guidelines: "所有表名和列名必须使用反引号包裹（例如：WHERE `system` = 1），以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误"
14-35: 存储过程逻辑正确，但建议增强幂等性存储过程正确检查了约束是否存在后再进行删除和重建。但如果约束已经是
SET NULL类型，重复执行会导致不必要的 DDL 操作。考虑在检查约束存在的同时，也检查当前的删除规则是否已经是
SET NULL，以实现完全幂等。整体迁移方案合理：将
ipRangeUuid的外键从CASCADE改为SET NULL，支持 UsedIpVO 在 IpRange 删除后仍能保留（用于 CIDR 范围外的 IP）。network/src/main/java/org/zstack/network/l3/CHANGES-VmNicIpOutsideCidr.md (1)
1-259: 文档应使用英文编写根据编码规范，所有文件内容（包括文档和注释）都应当使用英文编写。此 CHANGES 文件完全使用中文，违反了
**/*.*规则。建议将整个文档翻译为英文，包括：
- 标题和章节名
- 功能描述
- 代码注释
- 表格内容
As per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"
Comment on lines
+284
to
+296
| public List<String> getStaticDnsByVmUuidAndL3Uuid(String vmUuid, String l3Uuid) { | ||
| List<Map<String, String>> tokenList = L3NetworkSystemTags.STATIC_DNS.getTokensOfTagsByResourceUuid(vmUuid); | ||
| for (Map<String, String> tokens : tokenList) { | ||
| String uuid = tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN); | ||
| if (uuid.equals(l3Uuid)) { | ||
| String dnsStr = tokens.get(L3NetworkSystemTags.STATIC_DNS_TOKEN); | ||
| if (dnsStr != null && !dnsStr.isEmpty()) { | ||
| return Arrays.asList(dnsStr.split(",")); | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
潜在空指针异常
Line 288: uuid.equals(l3Uuid) 中 uuid 可能为 null（如果 token 中没有该 key），会导致 NPE。建议使用 l3Uuid.equals(uuid) 或进行 null 检查。
另外，Line 295 返回 null 而非空列表，与其他类似方法（如 getStaticIpbyVmUuid）返回空集合的模式不一致。
🛡️ 修复建议
public List<String> getStaticDnsByVmUuidAndL3Uuid(String vmUuid, String l3Uuid) {
List<Map<String, String>> tokenList = L3NetworkSystemTags.STATIC_DNS.getTokensOfTagsByResourceUuid(vmUuid);
for (Map<String, String> tokens : tokenList) {
String uuid = tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN);
- if (uuid.equals(l3Uuid)) {
+ if (l3Uuid.equals(uuid)) {
String dnsStr = tokens.get(L3NetworkSystemTags.STATIC_DNS_TOKEN);
if (dnsStr != null && !dnsStr.isEmpty()) {
return Arrays.asList(dnsStr.split(","));
}
}
}
- return null;
+ return new ArrayList<>();
}🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java` around
lines 284 - 296, In getStaticDnsByVmUuidAndL3Uuid, make the l3Uuid comparison
null-safe and return an empty list instead of null: guard against
tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN) being null by using
l3Uuid.equals(uuid) or an explicit null check before comparing, and change the
method to return Collections.emptyList() (or new ArrayList<>()) when no DNS
entries are found to match the behavior of getStaticIpbyVmUuid; update
references to L3NetworkSystemTags.STATIC_DNS_TOKEN handling accordingly.
Comment on lines
+491
to
+499
| for (Map<String, String> tokens : tokenList) { | ||
| String uuid = tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN); | ||
| if (uuid.equals(l3NetworkUuid)) { | ||
| String dnsStr = tokens.get(L3NetworkSystemTags.STATIC_DNS_TOKEN); | ||
| if (dnsStr != null && !dnsStr.isEmpty()) { | ||
| return Arrays.asList(dnsStr.split(",")); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
潜在空指针异常风险
当 tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN) 返回 null 时，第 493 行的 uuid.equals(l3NetworkUuid) 会抛出 NullPointerException。
建议使用 l3NetworkUuid.equals(uuid) 或添加空值检查：
🛠️ 建议修复
for (Map<String, String> tokens : tokenList) {
String uuid = tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN);
- if (uuid.equals(l3NetworkUuid)) {
+ if (l3NetworkUuid.equals(uuid)) {
String dnsStr = tokens.get(L3NetworkSystemTags.STATIC_DNS_TOKEN);
if (dnsStr != null && !dnsStr.isEmpty()) {
return Arrays.asList(dnsStr.split(","));
}
}
}🤖 Prompt for AI Agents
In
`@network/src/main/java/org/zstack/network/service/NetworkServiceManagerImpl.java`
around lines 491 - 499, The loop in NetworkServiceManagerImpl that compares
uuid.equals(l3NetworkUuid) risks NPE if
tokens.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN) returns null; change
the check to use l3NetworkUuid.equals(uuid) or explicitly null-check uuid before
comparing, and ensure you still retrieve and validate the STATIC_DNS_TOKEN
string (dnsStr) before splitting to avoid downstream errors (refer to the
variables uuid, l3NetworkUuid and keys
L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN /
L3NetworkSystemTags.STATIC_DNS_TOKEN).
Comment on lines
+205
to
+212
|
|
||
| // Check if the IP is outside L3 CIDR range (ipRangeUuid is null) | ||
| UsedIpVO usedIpVO = dbf.findByUuid(msg.getUsedIpUuid(), UsedIpVO.class); | ||
| if (usedIpVO != null && usedIpVO.getIpRangeUuid() == null) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "cannot bindBind EIP to IP address[%s] which is outside L3 network CIDR range", | ||
| usedIpVO.getIp())); | ||
| } |
There was a problem hiding this comment.
错误消息中存在拼写错误
第 210 行的错误消息 "cannot bindBind EIP" 中 bindBind 是拼写错误，应为 bind。
🐛 建议修复
// Check if the IP is outside L3 CIDR range (ipRangeUuid is null)
UsedIpVO usedIpVO = dbf.findByUuid(msg.getUsedIpUuid(), UsedIpVO.class);
if (usedIpVO != null && usedIpVO.getIpRangeUuid() == null) {
throw new ApiMessageInterceptionException(argerr(
- "cannot bindBind EIP to IP address[%s] which is outside L3 network CIDR range",
+ "cannot bind EIP to IP address[%s] which is outside L3 network CIDR range",
usedIpVO.getIp()));
}As per coding guidelines: "报错、注释等都应当使用正确的、无拼写错误的英文来写"
🤖 Prompt for AI Agents
In
`@plugin/eip/src/main/java/org/zstack/network/service/eip/EipApiInterceptor.java`
around lines 205 - 212, In EipApiInterceptor update the error message string
used when throwing ApiMessageInterceptionException for a UsedIpVO with null
ipRangeUuid: locate the block that calls dbf.findByUuid(msg.getUsedIpUuid(),
UsedIpVO.class) and currently throws
ApiMessageInterceptionException(argerr("cannot bindBind EIP to IP address[%s]
...", usedIpVO.getIp())); and correct the typo to "cannot bind EIP to IP
address[%s] which is outside L3 network CIDR range" (retain the argerr usage and
the inserted usedIpVO.getIp() parameter).
Comment on lines
+645
to
+657
|
|
||
| // Check if any NIC's IP is outside L3 CIDR range (ipRangeUuid is null) | ||
| for (String nicUuid : msg.getVmNicUuids()) { | ||
| VmNicVO nicVO = dbf.findByUuid(nicUuid, VmNicVO.class); | ||
| if (nicVO != null && nicVO.getUsedIpUuid() != null) { | ||
| UsedIpVO usedIpVO = dbf.findByUuid(nicVO.getUsedIpUuid(), UsedIpVO.class); | ||
| if (usedIpVO != null && usedIpVO.getIpRangeUuid() == null) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "cannot add VM NIC[uuid:%s] with IP address[%s] which is outside L3 network CIDR range to load balancer", | ||
| nicUuid, usedIpVO.getIp())); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
循环内数据库查询存在性能问题，且可能遗漏双栈场景
存在以下问题：
-
性能问题：循环内执行了
dbf.findByUuid查询，违反了 "禁止循环里套查询" 的编码规范。应改为批量查询。 -
双栈场景：
nicVO.getUsedIpUuid()只返回单个 IP，但根据项目约束，同一 VmNic 可能同时存在两条 UsedIpVO（IPv4/IPv6 双栈）。应遍历nicVO.getUsedIps()检查所有关联 IP。 -
错误码缺失：错误信息应使用
ORG_ZSTACK_NETWORK_SERVICE_LB_XXXXX常量，与文件中其他错误信息保持一致。
建议的修复方案
- // Check if any NIC's IP is outside L3 CIDR range (ipRangeUuid is null)
- for (String nicUuid : msg.getVmNicUuids()) {
- VmNicVO nicVO = dbf.findByUuid(nicUuid, VmNicVO.class);
- if (nicVO != null && nicVO.getUsedIpUuid() != null) {
- UsedIpVO usedIpVO = dbf.findByUuid(nicVO.getUsedIpUuid(), UsedIpVO.class);
- if (usedIpVO != null && usedIpVO.getIpRangeUuid() == null) {
- throw new ApiMessageInterceptionException(argerr(
- "cannot add VM NIC[uuid:%s] with IP address[%s] which is outside L3 network CIDR range to load balancer",
- nicUuid, usedIpVO.getIp()));
- }
- }
- }
+ // Check if any NIC's IP is outside L3 CIDR range (ipRangeUuid is null)
+ List<UsedIpVO> usedIps = Q.New(UsedIpVO.class)
+ .in(UsedIpVO_.vmNicUuid, msg.getVmNicUuids())
+ .isNull(UsedIpVO_.ipRangeUuid)
+ .list();
+ if (!usedIps.isEmpty()) {
+ UsedIpVO usedIp = usedIps.get(0);
+ throw new ApiMessageInterceptionException(argerr(
+ ORG_ZSTACK_NETWORK_SERVICE_LB_XXXXX,
+ "cannot add VM NIC[uuid:%s] with IP address[%s] which is outside L3 network CIDR range to load balancer",
+ usedIp.getVmNicUuid(), usedIp.getIp()));
+ }Based on learnings: 同一 VmNic 可能同时存在两条 UsedIpVO，但它们只代表同一 L3 的 IPv4/IPv6 双栈。As per coding guidelines: 禁止循环里套查询，避免嵌套查询带来的性能问题。
🤖 Prompt for AI Agents
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java`
around lines 645 - 657, Replace the per-NIC DB lookups and single-usedIp check
with batch queries and full-usedIp iteration: fetch all VmNicVOs for
msg.getVmNicUuids() in one dbf query, collect all UsedIpVO UUIDs from each
VmNicVO.getUsedIps() (or from getUsedIpUuids if available), batch-load those
UsedIpVOs in one dbf.findByUuids call, then for each VmNicVO iterate its
associated UsedIpVOs and throw ApiMessageInterceptionException using the
ORG_ZSTACK_NETWORK_SERVICE_LB_XXXXX error constant when any UsedIpVO has
ipRangeUuid == null; update the code in LoadBalancerApiInterceptor where the
loop over msg.getVmNicUuids() currently lives to use these batched collections
and iteration to avoid looped queries and to handle dual-stack IPs.
Comment on lines
+152
to
+160
| // Check if the NIC's IP is outside L3 CIDR range (ipRangeUuid is null) | ||
| VmNicVO nicVO = dbf.findByUuid(msg.getVmNicUuid(), VmNicVO.class); | ||
| if (nicVO != null && nicVO.getUsedIpUuid() != null) { | ||
| UsedIpVO usedIpVO = dbf.findByUuid(nicVO.getUsedIpUuid(), UsedIpVO.class); | ||
| if (usedIpVO != null && usedIpVO.getIpRangeUuid() == null) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "cannot bindBind port forwarding rule to IP address[%s] which is outside L3 network CIDR range", | ||
| usedIpVO.getIp())); | ||
| } |
There was a problem hiding this comment.
修正错误提示的拼写与措辞。
当前提示包含明显的重复拼写（bindBind）。建议修正以提升可读性与专业性。
🛠️ 建议修复
- throw new ApiMessageInterceptionException(argerr(
- "cannot bindBind port forwarding rule to IP address[%s] which is outside L3 network CIDR range",
- usedIpVO.getIp()));
+ throw new ApiMessageInterceptionException(argerr(
+ "cannot bind port forwarding rule to IP address[%s] because it is outside the L3 network CIDR range",
+ usedIpVO.getIp()));🤖 Prompt for AI Agents
In
`@plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingApiInterceptor.java`
around lines 152 - 160, The exception message in PortForwardingApiInterceptor
when checking nicVO/usedIpVO mistakenly contains "bindBind" and should be
reworded for clarity; update the ApiMessageInterceptionException thrown via
argerr to a clear single-voice message (e.g., "cannot bind port forwarding rule
to IP address [%s] because it is outside the L3 network CIDR range") referencing
usedIpVO.getIp(), keeping the same exception type and placement so nicVO,
usedIpVO, ApiMessageInterceptionException and argerr remain unchanged except for
the corrected message text.
Comment on lines
169
to
178
| private List<String> getVmIpsBySecurityGroup(String sgUuid, int ipVersion){ | ||
| List<String> ret = new ArrayList<>(); | ||
| // Exclude IPs outside L3 CIDR range (ipRangeUuid is null) | ||
| String sql = "select ip.ip" + | ||
| " from VmNicVO nic, VmNicSecurityGroupRefVO ref, SecurityGroupVO sg, UsedIpVO ip" + | ||
| " where sg.uuid = ref.securityGroupUuid and ref.vmNicUuid = nic.uuid" + | ||
| " and ref.securityGroupUuid = :sgUuid" + | ||
| " and nic.uuid = ip.vmNicUuid and ip.ipVersion = :ipVersion"; | ||
| " and nic.uuid = ip.vmNicUuid and ip.ipVersion = :ipVersion" + | ||
| " and ip.ipRangeUuid is not null"; | ||
| TypedQuery<String> internalIpQuery = dbf.getEntityManager().createQuery(sql, String.class); |
There was a problem hiding this comment.
过滤 ipRangeUuid 可能导致 IPAM 关闭网络或允许越界 IP 的安全组成员缺失。
enableIPAM=false 的 L3 网络通常不会有 ipRangeUuid，这条过滤会把这些 NIC 的 IP 全部排除，远端安全组引用它们时会缺成员。若系统允许“IP 超出范围”，同样会被过滤掉。建议将过滤限定在 IPAM 开启且不允许越界 的场景，或显式允许 enableIPAM=false 的 L3。
💡 可选修正方向（请按实际字段名调整）
- String sql = "select ip.ip" +
- " from VmNicVO nic, VmNicSecurityGroupRefVO ref, SecurityGroupVO sg, UsedIpVO ip" +
+ String sql = "select ip.ip" +
+ " from VmNicVO nic, VmNicSecurityGroupRefVO ref, SecurityGroupVO sg, UsedIpVO ip, L3NetworkVO l3" +
" where sg.uuid = ref.securityGroupUuid and ref.vmNicUuid = nic.uuid" +
" and ref.securityGroupUuid = :sgUuid" +
- " and nic.uuid = ip.vmNicUuid and ip.ipVersion = :ipVersion" +
- " and ip.ipRangeUuid is not null";
+ " and nic.uuid = ip.vmNicUuid and ip.ipVersion = :ipVersion" +
+ " and nic.l3NetworkUuid = l3.uuid" +
+ " and (ip.ipRangeUuid is not null or l3.enableIPAM = false)";Based on learnings 在 L3NetworkApiInterceptor 中，enableIPAM=false 的 L3 网络不会有关联的 ipRangeUuid。
🤖 Prompt for AI Agents
In
`@plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupManagerImpl.java`
around lines 169 - 178, The query in getVmIpsBySecurityGroup unconditionally
excludes rows with ip.ipRangeUuid == null which drops NICs on L3s with
enableIPAM=false or IPs allowed out-of-range; change the WHERE so the
ipRangeUuid check only applies when the L3 has IPAM enabled and disallows
out-of-range addresses. Concretely, join UsedIpVO (alias ip) to L3NetworkVO and
replace the final clause "and ip.ipRangeUuid is not null" with a predicate like
"(l3.enableIpam = false OR (l3.enableIpam = true AND ip.ipRangeUuid is not
null))" (adjust field names to the actual enableIPAM/allowOver fields) so
getVmIpsBySecurityGroup returns IPs for L3s without IPAM and still filters only
when IPAM is enabled and range-enforced.
Resolves: ZSTAC-81969 Change-Id: I666b666d726963716570716178636e627377767a
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java (1)
402-499:⚠️ Potential issue | 🟠 Major多 IP 段场景下的范围判断需覆盖全部段
当前仅用第一条 IPRange 计算 ipInRange；在允许范围外时，多段网络里“在其他段内”的 IP 会被误判为超范围并强制要求网关/掩码。建议遍历全部段确认是否在任一范围内。
🔧 建议修改
- NormalIpRangeVO ipRangeVO = Q.New(NormalIpRangeVO.class) - .eq(NormalIpRangeVO_.l3NetworkUuid, l3Uuid) - .eq(NormalIpRangeVO_.ipVersion, IPv6Constants.IPv4) - .limit(1).find(); - - // Check if IP is within the range - boolean ipInRange = ipRangeVO != null && - NetworkUtils.isInRange(nicIp.ipv4Address, ipRangeVO.getStartIp(), ipRangeVO.getEndIp()); + List<NormalIpRangeVO> ipRangeVOs = Q.New(NormalIpRangeVO.class) + .eq(NormalIpRangeVO_.l3NetworkUuid, l3Uuid) + .eq(NormalIpRangeVO_.ipVersion, IPv6Constants.IPv4) + .list(); + NormalIpRangeVO ipRangeVO = ipRangeVOs.isEmpty() ? null : ipRangeVOs.get(0); + + // Check if IP is within any range + boolean ipInRange = false; + for (NormalIpRangeVO range : ipRangeVOs) { + if (NetworkUtils.isInRange(nicIp.ipv4Address, range.getStartIp(), range.getEndIp())) { + ipInRange = true; + break; + } + }- NormalIpRangeVO ipRangeVO = Q.New(NormalIpRangeVO.class) - .eq(NormalIpRangeVO_.l3NetworkUuid, l3Uuid) - .eq(NormalIpRangeVO_.ipVersion, IPv6Constants.IPv6) - .limit(1).find(); - - // Check if IPv6 is within the range - boolean ipInRange = ipRangeVO != null && - IPv6NetworkUtils.isIpv6InRange(nicIp.ipv6Address, ipRangeVO.getStartIp(), ipRangeVO.getEndIp()); + List<NormalIpRangeVO> ipRangeVOs = Q.New(NormalIpRangeVO.class) + .eq(NormalIpRangeVO_.l3NetworkUuid, l3Uuid) + .eq(NormalIpRangeVO_.ipVersion, IPv6Constants.IPv6) + .list(); + NormalIpRangeVO ipRangeVO = ipRangeVOs.isEmpty() ? null : ipRangeVOs.get(0); + + // Check if IPv6 is within any range + boolean ipInRange = false; + for (NormalIpRangeVO range : ipRangeVOs) { + if (IPv6NetworkUtils.isIpv6InRange(nicIp.ipv6Address, range.getStartIp(), range.getEndIp())) { + ipInRange = true; + break; + } + }
🤖 Fix all issues with AI agents
In `@compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java`:
- Around line 139-148: In StaticIpOperator (the block handling
L3NetworkSystemTags.STATIC_DNS), trim and sanitize the parsed dns string before
splitting: after obtaining dnsStr from the tag, run a trim, remove any internal
newlines/extra whitespace, split on commas, then trim each token and filter out
empty strings before assigning to dnsAddresses; additionally, support IPv6-safe
storage by decoding the tag value when reading (and ensure callers encode IPv6
addresses when writing) using the project’s system-tag encode/decode utilities
so addresses containing "::" are preserved. Apply the same trimming/decoding
logic for the other similar parsing sites referenced around lines 244-275 and
289-292.
Comment on lines
+139
to
+148
| if(L3NetworkSystemTags.STATIC_DNS.isMatch(sysTag)) { | ||
| Map<String, String> token = TagUtils.parse(L3NetworkSystemTags.STATIC_DNS.getTagFormat(), sysTag); | ||
| String l3Uuid = token.get(L3NetworkSystemTags.STATIC_DNS_L3_UUID_TOKEN); | ||
| if (ret.get(l3Uuid) == null) { | ||
| continue; | ||
| } | ||
| String dnsStr = token.get(L3NetworkSystemTags.STATIC_DNS_TOKEN); | ||
| if (dnsStr != null && !dnsStr.isEmpty()) { | ||
| ret.get(l3Uuid).dnsAddresses = Arrays.asList(dnsStr.split(",")); | ||
| } |
There was a problem hiding this comment.
DNS 入参需 trim 且 IPv6 需做 tag 编码/解码
dnsAddresses 可能含空格/换行，直接校验会误判；另外 IPv6 若直接写入 system tag，含 “::” 的地址在解析时可能被截断，建议写入前编码、读取时解码。
🔧 建议修改
- // Validate DNS addresses
- for (String dns : dnsAddresses) {
- if (!NetworkUtils.isIpv4Address(dns) && !IPv6NetworkUtils.isIpv6Address(dns)) {
- throw new ApiMessageInterceptionException(argerr(
- "invalid DNS address[%s], must be a valid IPv4 or IPv6 address", dns));
- }
- }
-
- String dnsStr = String.join(",", dnsAddresses);
+ // Validate DNS addresses
+ List<String> normalizedDns = new ArrayList<>();
+ for (String dns : dnsAddresses) {
+ String trimmed = dns == null ? null : dns.trim();
+ if (StringUtils.isEmpty(trimmed)) {
+ continue;
+ }
+ if (!NetworkUtils.isIpv4Address(trimmed) && !IPv6NetworkUtils.isIpv6Address(trimmed)) {
+ throw new ApiMessageInterceptionException(argerr(
+ "invalid DNS address[%s], must be a valid IPv4 or IPv6 address", trimmed));
+ }
+ if (IPv6NetworkUtils.isIpv6Address(trimmed)) {
+ trimmed = IPv6NetworkUtils.ipv6AddessToTagValue(trimmed);
+ }
+ normalizedDns.add(trimmed);
+ }
+
+ String dnsStr = String.join(",", normalizedDns);- if (dnsStr != null && !dnsStr.isEmpty()) {
- ret.get(l3Uuid).dnsAddresses = Arrays.asList(dnsStr.split(","));
- }
+ if (dnsStr != null && !dnsStr.isEmpty()) {
+ List<String> dnsList = new ArrayList<>();
+ for (String dns : dnsStr.split(",")) {
+ dnsList.add(IPv6NetworkUtils.ipv6TagValueToAddress(dns));
+ }
+ ret.get(l3Uuid).dnsAddresses = dnsList;
+ }- return Arrays.asList(dnsStr.split(","));
+ List<String> dnsList = new ArrayList<>();
+ for (String dns : dnsStr.split(",")) {
+ dnsList.add(IPv6NetworkUtils.ipv6TagValueToAddress(dns));
+ }
+ return dnsList;As per coding guidelines: 注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等。
Also applies to: 244-275, 289-292
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java` around
lines 139 - 148, In StaticIpOperator (the block handling
L3NetworkSystemTags.STATIC_DNS), trim and sanitize the parsed dns string before
splitting: after obtaining dnsStr from the tag, run a trim, remove any internal
newlines/extra whitespace, split on commas, then trim each token and filter out
empty strings before assigning to dnsAddresses; additionally, support IPv6-safe
storage by decoding the tag value when reading (and ensure callers encode IPv6
addresses when writing) using the project’s system-tag encode/decode utilities
so addresses containing "::" are preserved. Apply the same trimming/decoding
logic for the other similar parsing sites referenced around lines 244-275 and
289-292.
APIImpact Resolves: ZSTAC-81969 Change-Id: I7877737577726a736d71696e7666667661776e78
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.
APIImpact
DBImpact
GlobalConfigImpact
Resolves: ZSTAC-81969
Change-Id: I736e77646266696e646271766d7170627378796b
sync from gitlab !9122