<feature>[lb]: feature-lb-forwarding-rules #3211
Conversation
Walkthrough新增 ACL 条目字段 Changes
序列图sequenceDiagram
participant Client
participant LBBase as LoadBalancerBase
participant ACLMgr as ACLManager
participant DB as Database
participant Listener as LBListener
participant VR as VRBackend
Client->>LBBase: APIAddAccessControlListToLoadBalancerMsg
LBBase->>LBBase: doAddAccessControlListToLoadBalancer()
LBBase->>ACLMgr: 查询/准备 ACL 条目
ACLMgr->>DB: 读取/持久化 ACL 条目
DB-->>ACLMgr: 返回条目
ACLMgr-->>LBBase: ACL 条目列表
LBBase->>Listener: 获取 listener.instancePort
Listener-->>LBBase: 返回 instancePort
LBBase->>DB: 更新 ACL 条目 redirectPort=instancePort
DB-->>LBBase: 确认持久化
LBBase->>VR: 发送 RefreshLoadBalancerMsg (包含 LbTO, redirectPort)
VR-->>LBBase: 后端配置确认
LBBase-->>Client: APIAddAccessControlListToLoadBalancerEvent
sequenceDiagram
participant System
participant LBMgr as LoadBalancerManagerImpl
participant DB as Database
participant Listener as LBListener
System->>LBMgr: start()
LBMgr->>LBMgr: upgradeLoadBalancerRedirectRule()
alt UPGRADE_LB_REDIRECT_RULE enabled
LBMgr->>DB: 查询 redirectPort IS NULL 的 ACL 条目
DB-->>LBMgr: 返回条目列表
loop 对每个条目
LBMgr->>DB: 通过 ACL refs 载入关联的 LoadBalancerListener
DB-->>LBMgr: 返回 listener (含 instancePort)
LBMgr->>LBMgr: 设置 redirectPort = instancePort
LBMgr->>DB: 持久化更新的 ACL 条目
end
end
LBMgr-->>System: 升级完成
估计代码审查工作量🎯 4 (复杂) | ⏱️ ~60 分钟 诗歌
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java (1)
860-876:formatRedirectRules中遗漏了redirectPort的拷贝在创建
formatRule时，复制了redirectRule、redirectRuleUuid、aclUuid、serverGroupUuid，但遗漏了新增的redirectPort字段。这会导致经过formatRedirectRules处理的规则丢失重定向端口配置。🐛 建议修复
for (LbTO.RedirectRule rule : redirectRules) { LbTO.RedirectRule formatRule = new LbTO.RedirectRule(); formatRule.setRedirectRule(rule.getRedirectRule()); formatRule.setRedirectRuleUuid(rule.getRedirectRuleUuid()); formatRule.setAclUuid(rule.getAclUuid()); formatRule.setServerGroupUuid(rule.getServerGroupUuid()); + formatRule.setRedirectPort(rule.getRedirectPort()); String matchMethod = Q.New(AccessControlListEntryVO.class).select(AccessControlListEntryVO_.matchMethod).eq(AccessControlListEntryVO_.uuid, rule.getRedirectRuleUuid()).findValue();plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (1)
2026-2120: 建议对空集合持久化与重复输入做保护，避免边界场景异常/重复数据这里
refs在某些情况下可能为空（例如重复添加相同 ACL+serverGroup），建议避免对空refs调用dbf.persistCollection(refs)；同时对aclUuids/serverGroupUuids做distinct()可减少重复 ref/重复 update。Proposed diff (guard empty persist + dedupe inputs)
public void run(FlowTrigger trigger, Map data) { List<LoadBalancerListenerACLRefVO> refs = new ArrayList<>(); + List<String> distinctAclUuids = msg.getAclUuids().stream().distinct().collect(Collectors.toList()); if (msg.getServerGroupUuids() != null && !msg.getServerGroupUuids().isEmpty()) { - for (String aclUuid : msg.getAclUuids()) { + List<String> distinctServerGroupUuids = msg.getServerGroupUuids().stream().distinct().collect(Collectors.toList()); + for (String aclUuid : distinctAclUuids) { List<String> sgUuids = lblVo.getAclRefs().stream().filter(ref -> ref.getAclUuid().equals(aclUuid)) .map(LoadBalancerListenerACLRefVO::getServerGroupUuid).collect(Collectors.toList()); - List<String> serverGroupUuids = msg.getServerGroupUuids().stream().filter(sgUuid -> !sgUuids.contains(sgUuid)).collect(Collectors.toList()); - for (String sgUuid : serverGroupUuids) { + for (String sgUuid : distinctServerGroupUuids.stream().filter(sgUuid -> !sgUuids.contains(sgUuid)).collect(Collectors.toList())) { LoadBalancerListenerACLRefVO ref = new LoadBalancerListenerACLRefVO(); ref.setAclUuid(aclUuid); ref.setType(LoadBalancerAclType.valueOf(msg.getAclType())); ref.setListenerUuid(msg.getListenerUuid()); ref.setServerGroupUuid(sgUuid); refs.add(ref); } } } else { - refs = msg.getAclUuids().stream().map(aclUuid -> { + refs = distinctAclUuids.stream().map(aclUuid -> { LoadBalancerListenerACLRefVO ref = new LoadBalancerListenerACLRefVO(); ref.setAclUuid(aclUuid); ref.setType(LoadBalancerAclType.valueOf(msg.getAclType())); ref.setListenerUuid(msg.getListenerUuid()); return ref; }).collect(Collectors.toList()); } - dbf.persistCollection(refs); + if (!refs.isEmpty()) { + dbf.persistCollection(refs); + } Integer instancePort = lblVo.getInstancePort(); - msg.getAclUuids().forEach(aclUuid -> { + distinctAclUuids.forEach(aclUuid -> { SQL.New(AccessControlListEntryVO.class) .eq(AccessControlListEntryVO_.aclUuid, aclUuid) .eq(AccessControlListEntryVO_.type, AclEntryType.RedirectRule.toString()) .isNull(AccessControlListEntryVO_.redirectPort) .set(AccessControlListEntryVO_.redirectPort, instancePort) .update(); });Based on learnings, 尽量保持同步/Cherry-pick 改动面最小，上面建议也可以只做“空集合保护”这一条。
🤖 Fix all issues with AI agents
In
`@test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy`:
- Around line 180-189: The test sets the global static flag
LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE but never restores it,
risking cross-test interference; update the test() method (and the other
occurrence around lines 327-330) to capture the original value before modifying,
set the flag to true for the upgrade step, and ensure you restore the original
value in a finally block (or after env.create) so
LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE is always reset regardless
of test outcome.
- Around line 191-297: The test may NPE when using cmd after attaching acl2
because the refresh callback might not have run; add an assertion that cmd !=
null before accessing cmd.lbs after creating acl2 and attaching it (i.e., insert
assert cmd != null right before "lbTO = cmd.lbs.find { it.lbUuid == lb1.uuid
}"), ensuring the RefreshLbCmd cmd captured by the
env.afterSimulator(VirtualRouterLoadBalancerBackend.REFRESH_LB_PATH) handler is
present.
🧹 Nitpick comments (3)
plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovy (2)
52-57: 文档字段redirectPort缺少描述新增的
redirectPort字段的desc为空字符串，应添加有意义的描述以便生成完整的 API 文档。建议修改
field { name "redirectPort" - desc "" + desc "重定向端口" type "Integer" since "5.5.6" }
3-4: 重复导入java.sql.Timestamp
Timestamp被导入了两次，请删除重复的导入语句。建议修改
import java.sql.Timestamp -import java.sql.Timestampplugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.java (1)
335-340: 预先存在的逻辑问题：handleLocalMessage中缺少 return 语句当消息是
DeleteAccessControlListMsg类型时，处理完成后会继续执行到bus.dealWithUnknownMessage(msg)，这会导致已知消息被当作未知消息处理。虽然这不是本次 PR 引入的问题，但建议修复以避免潜在的问题。
建议修改
protected void handleLocalMessage(Message msg) { if (msg instanceof DeleteAccessControlListMsg) { handle((DeleteAccessControlListMsg)msg); + return; } bus.dealWithUnknownMessage(msg); }
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
sdk/src/main/java/org/zstack/sdk/AccessControlListEntryInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/AddAccessControlListRedirectRuleAction.javais excluded by!sdk/**
📒 Files selected for processing (14)
conf/db/upgrade/V5.5.6__schema.sqlplugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovyplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javatest/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy
🧰 Additional context used
📓 Path-based instructions (3)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovyplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javatest/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovyconf/db/upgrade/V5.5.6__schema.sqlplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: - Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important- 数据库记录中，如果字符串长度不可控，不要用
vchar，用text类型
Files:
conf/db/upgrade/V5.5.6__schema.sql
🧠 Learnings (1)
📓 Common learnings
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
🧬 Code graph analysis (1)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (2)
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java (1)
RedirectRule(346-391)plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.java (1)
LoadBalancerConstants(15-180)
🔍 Remote MCP AtlassianMCP
Perfect! Now I have gathered comprehensive information. Let me compile the summary with proper citations.
Summary of Additional Context for PR Review
Based on the gathered information, here is the relevant context for reviewing this pull request:
Feature Background and Objectives
This PR resolves ZSTAC-79223, which is a P0 priority feature for ZStack v5.5.6 that enables load balancer listeners to support forwarding rules configured by domain names to direct traffic to different hosts on different ports.
The feature addresses a customer requirement where frontend access is unified to address and port 10.204.20.3:443, and the system needs to support configuring domain name forwarding rules to route to different hosts on different ports. Previously, users faced limitations where load balancer forwarding rules could only route to backend server groups without the ability to configure corresponding backend listening ports, and creating multiple listeners would result in "port conflict" errors since frontend access ports are all on port 443.
The target implementation approach is to support adding a "manual configuration of backend server group specified port" when adding forwarding rules (referred to as "方案二" - Solution 2 in the feature spec).
Implementation Strategy
The PR implements this feature by:
-
Database Schema Change: Adds a nullable
redirectPortinteger column to theAccessControlListEntryVOtable for storing the port configuration. -
API Enhancement: Introduces a new
redirectPortparameter toAPIAddAccessControlListRedirectRuleMsgwith validation constraints (port range 1-65535). -
Load Balancer Integration: In
LoadBalancerBase, implements a newdoAddAccessControlListToLoadBalancermethod that uses a FlowChain-based approach to:- Record ACL references
- Update redirect ports for ACL entries to match the listener's instance port
- Conditionally refresh the load balancer listener when needed
-
Upgrade Mechanism: Adds a
upgradeLoadBalancerRedirectRule()method inLoadBalancerManagerImplwith a global property flag (UPGRADE_LB_REDIRECT_RULE) to handle migration of existing redirect rules by assigning the listener port to rules missing a redirectPort value during system startup. -
Virtual Router Backend: Propagates the
redirectPortfield from ACL entries to the virtual router's load balancer transmission objects (LbTO.RedirectRule). -
Test Coverage: Includes a comprehensive test case
VirtualRouterLoadBalancerForwardRuleCasethat validates redirect port functionality and the upgrade mechanism.
Key Technical Details
- The assignee is 刘鹏超 (Pengchao Liu) with time estimate of 80 hours (80000 seconds remaining time)
- The changes maintain backward compatibility through the upgrade mechanism for existing redirect rules
- All changes follow the established patterns in the codebase for data model extensions and API parameter additions,,
🔇 Additional comments (16)
conf/db/upgrade/V5.5.6__schema.sql (1)
1-1: LGTM!数据库升级脚本正确地添加了可为空的
redirectPort列。由于没有NOT NULL约束，现有记录将自动获得NULL值，无需额外处理历史数据。升级逻辑由LoadBalancerManagerImpl.upgradeLoadBalancerRedirectRule()方法在启动时处理。plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.java (1)
28-38: 示例可能存在不一致
APIAddAccessControlListEntryEvent对应的是添加 IP 条目（IpEntry）的场景，而redirectPort字段仅对重定向规则（RedirectRule）有意义。在AccessControlListManagerImpl.handle(APIAddAccessControlListEntryMsg)中，创建的条目类型为IpEntry，并未设置redirectPort。建议确认此示例是否应包含
redirectPort，或者此示例是否应仅展示 IP 条目的典型字段。plugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.java (1)
24-36: LGTM!在
__example__()方法中正确添加了redirectPort示例值，与重定向规则的使用场景一致。plugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.java (1)
212-284: LGTM!在
handle(APIAddAccessControlListRedirectRuleMsg)方法中正确地将redirectPort从 API 消息传递到AccessControlListEntryVO。端口范围验证（1-65535）已在APIAddAccessControlListRedirectRuleMsg的@APIParam注解中处理。plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.java (1)
28-28: LGTM!
redirectPort字段的添加遵循了现有 Inventory 类的模式，包括字段声明、构造函数初始化和标准的 getter/setter。Also applies to: 58-58, 121-127
plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.java (1)
47-48: LGTM!API 参数
redirectPort的添加符合规范：
- 端口范围
{1, 65535}正确required = false保证向后兼容__example__方法正确演示了用法Also applies to: 107-113, 124-124
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java (2)
351-351: LGTM!
LbTO.RedirectRule内部类中添加redirectPort字段，用于向虚拟路由器后端传输重定向端口配置。实现符合现有模式。Also applies to: 384-390
785-785: LGTM!单服务器组和多服务器组两种路径都正确地从
entry.getRedirectPort()传播重定向端口到redirectRule对象。Also applies to: 791-791
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.java (1)
10-11: LGTM!新增的
UPGRADE_LB_REDIRECT_RULE全局属性遵循现有的UPGRADE_LB_SERVER_GROUP模式，默认值为false确保向后兼容性。升级行为需要显式启用，符合编码规范中的向后兼容原则。plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.java (1)
23-23: LGTM!JPA 元模型属性
redirectPort正确反映了AccessControlListEntryVO中新增的字段，类型为Integer，遵循标准元模型模式。plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.java (2)
20-23: 新增AclEntryType引用合理用于后续按
RedirectRule类型筛选，方向正确。
451-486: 在start()触发升级流程符合预期升级逻辑有全局开关保护，放在初始化阶段执行合理。
plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.java (1)
49-52:redirectPort字段与访问器添加 OK使用
Integer支持 null（兼容老数据）也便于后续升级补齐。Also applies to: 171-177
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (2)
23-26: 导入新增与新逻辑匹配引入 ACL/RedirectRule 相关 VO/enum 供后续更新使用，变更合理。
2122-2155: 拆分doAddAccessControlListToLoadBalancer()提升可读性将原逻辑抽到独立方法、并用 FlowChain 串起“写 DB →（必要时）refresh”流程，结构更清晰。
test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy (1)
299-325: 升级路径断言覆盖到位先将
redirectPort置空再触发升级、并校验写回8080，能有效回归升级逻辑。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| public void upgradeLoadBalancerRedirectRule() { | ||
| if (!LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE) { | ||
| return; | ||
| } | ||
| List<AccessControlListEntryVO> redirectRules = Q.New(AccessControlListEntryVO.class) | ||
| .eq(AccessControlListEntryVO_.type, AclEntryType.RedirectRule.toString()) | ||
| .isNull(AccessControlListEntryVO_.redirectPort) | ||
| .list(); | ||
| if (redirectRules.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| List<AccessControlListEntryVO> toUpgradeRules = new ArrayList<>(); | ||
| for (AccessControlListEntryVO rule : redirectRules) { | ||
| LoadBalancerListenerVO lbListenerVO = SQL.New("select lbListener from LoadBalancerListenerVO lbListener, LoadBalancerListenerACLRefVO ref, AccessControlListVO acl " + | ||
| "where acl.uuid = ref.aclUuid and ref.listenerUuid = lbListener.uuid and acl.uuid = :aclUuid") | ||
| .param("aclUuid", rule.getAclUuid()) | ||
| .find(); | ||
| if (lbListenerVO == null) { | ||
| continue; | ||
| } | ||
|
|
||
| rule.setRedirectPort(lbListenerVO.getInstancePort()); | ||
| toUpgradeRules.add(rule); | ||
| } | ||
|
|
||
| if (!toUpgradeRules.isEmpty()) { | ||
| dbf.updateCollection(toUpgradeRules); | ||
| } | ||
| } |
There was a problem hiding this comment.
升级逻辑需要避免“同一 ACL 关联多个 listener”时的不确定性/潜在异常
当前按 aclUuid 反查一个 LoadBalancerListenerVO 后写入 redirectPort，如果同一 aclUuid 被多个 listener 引用（且 instancePort 不同），这里会出现选取不确定/覆盖语义不清的问题；另外也建议确认 SQL.find() 在多结果场景的行为。建议改为显式处理多结果：仅在能唯一确定 instancePort 时升级，否则跳过并记录日志（保留 null 让后续 attach 时再补齐）。
Proposed diff (deterministic + safe upgrade)
public void upgradeLoadBalancerRedirectRule() {
if (!LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE) {
return;
}
List<AccessControlListEntryVO> redirectRules = Q.New(AccessControlListEntryVO.class)
.eq(AccessControlListEntryVO_.type, AclEntryType.RedirectRule.toString())
.isNull(AccessControlListEntryVO_.redirectPort)
.list();
if (redirectRules.isEmpty()) {
return;
}
List<AccessControlListEntryVO> toUpgradeRules = new ArrayList<>();
for (AccessControlListEntryVO rule : redirectRules) {
- LoadBalancerListenerVO lbListenerVO = SQL.New("select lbListener from LoadBalancerListenerVO lbListener, LoadBalancerListenerACLRefVO ref, AccessControlListVO acl " +
- "where acl.uuid = ref.aclUuid and ref.listenerUuid = lbListener.uuid and acl.uuid = :aclUuid")
- .param("aclUuid", rule.getAclUuid())
- .find();
- if (lbListenerVO == null) {
- continue;
- }
-
- rule.setRedirectPort(lbListenerVO.getInstancePort());
- toUpgradeRules.add(rule);
+ List<Integer> instancePorts = SQL.New(
+ "select distinct lbListener.instancePort from LoadBalancerListenerVO lbListener, LoadBalancerListenerACLRefVO ref " +
+ "where ref.listenerUuid = lbListener.uuid and ref.aclUuid = :aclUuid",
+ Integer.class)
+ .param("aclUuid", rule.getAclUuid())
+ .list();
+
+ if (instancePorts.isEmpty()) {
+ continue;
+ }
+ if (instancePorts.size() > 1) {
+ logger.warn(String.format("skip upgrading redirectPort for acl[uuid:%s], multiple listener instancePorts found: %s",
+ rule.getAclUuid(), instancePorts));
+ continue;
+ }
+
+ rule.setRedirectPort(instancePorts.get(0));
+ toUpgradeRules.add(rule);
}
if (!toUpgradeRules.isEmpty()) {
dbf.updateCollection(toUpgradeRules);
}
}
There was a problem hiding this comment.
Comment from pengchao.liu:
可以优化，下次提pr时补上
There was a problem hiding this comment.
@ZStack-Robot 好的,感谢确认!既然计划在下次 PR 中优化,是否需要我创建一个 follow-up issue 来跟踪这个优化项?这样可以确保后续不会遗漏。
您希望使用哪个 issue tracking 平台?
- Jira Project: ZSTAC
- Jira Project: TIC
- Jira Project: ZBS
- GitHub
🧠 Learnings used
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java:471-545
Timestamp: 2025-08-22T05:36:40.467Z
Learning: In ZStack项目中，对于核心逻辑相关的代码，团队倾向于暂时不进行重构，即使代码复杂度较高，优先保证核心功能的稳定性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2325
File: compute/src/main/java/org/zstack/compute/vm/VmMigrateCallExtensionFlow.java:29-29
Timestamp: 2025-07-24T05:53:10.246Z
Learning: 在ZStack项目的重构过程中，团队采用务实的优先级策略：优先完成影响运行时功能的代码重构，对于注释中的旧引用等非功能性问题可以延后处理，因为这些不会影响系统的实际运行。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java:364-501
Timestamp: 2025-08-04T03:19:07.454Z
Learning: In ZStack project, developers may prefer to keep related validation logic together in a single method (like validateStaticIpTagsInApiMessage with 130+ lines) when the architecture is clear and there's no code reuse, rather than splitting into smaller methods based purely on single responsibility principle.
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2419
File: plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java:986-1067
Timestamp: 2025-08-13T02:31:40.023Z
Learning: 在ZStack项目中，团队倾向于保持统一的业务流程方法不进行拆分，即使方法较长也会维持在单个方法中以保持业务逻辑的完整性。
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2496
File: plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmBackend.java:2545-2566
Timestamp: 2025-08-24T07:35:42.793Z
Learning: 在 ZStack 代码库中，当响应类包含数值字段（如 size、actualSize）时，优先使用原始类型（long）而不是包装类型（Long），以避免 NPE 风险和不必要的装箱/拆箱操作。如果 Agent 端可能不设置该字段，应在 Agent 端确保设置默认值，而不是在使用端做 null 检查。
| @Override | ||
| void test() { | ||
| dbf = bean(DatabaseFacade.class) | ||
| lbMgr = bean(LoadBalancerManagerImpl.class) | ||
| env.create { | ||
| testLoadBalancerForwardRule() | ||
| LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = true | ||
| testUpgradeLoadBalancerForwardRule() | ||
| } | ||
| } |
There was a problem hiding this comment.
测试修改全局开关需恢复，避免影响后续用例
LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE 是全局静态状态，建议在用例结束（或 finally）恢复原值，降低串扰风险。
Proposed diff (restore global flag)
class VirtualRouterLoadBalancerForwardRuleCase extends SubCase {
DatabaseFacade dbf
EnvSpec env
LoadBalancerManagerImpl lbMgr
+ boolean oldUpgradeFlag
`@Override`
void test() {
dbf = bean(DatabaseFacade.class)
lbMgr = bean(LoadBalancerManagerImpl.class)
+ oldUpgradeFlag = LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE
env.create {
- testLoadBalancerForwardRule()
- LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = true
- testUpgradeLoadBalancerForwardRule()
+ try {
+ testLoadBalancerForwardRule()
+ LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = true
+ testUpgradeLoadBalancerForwardRule()
+ } finally {
+ LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = oldUpgradeFlag
+ }
}
}Also applies to: 327-330
🤖 Prompt for AI Agents
In
`@test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy`
around lines 180 - 189, The test sets the global static flag
LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE but never restores it,
risking cross-test interference; update the test() method (and the other
occurrence around lines 327-330) to capture the original value before modifying,
set the flag to true for the upgrade step, and ensure you restore the original
value in a finally block (or after env.create) so
LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE is always reset regardless
of test outcome.
There was a problem hiding this comment.
Comment from pengchao.liu:
won't do
There was a problem hiding this comment.
@zstack-robot-2 Understood, thanks for clarifying. Based on the retrieved learning about ZStack's test framework, test cases are isolated from each other (case by case), so modifying global configurations like LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE at the end of a test case doesn't require restoration since it won't affect other test executions.
🧠 Learnings used
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2250
File: test/src/test/groovy/org/zstack/test/integration/network/l3network/IpRangeCase.groovy:528-531
Timestamp: 2025-07-07T04:01:27.830Z
Learning: 在ZStack测试框架中，测试用例之间是相互隔离的（case by case），在测试用例结尾修改全局配置（如VmGlobalConfig.VM_DELETION_POLICY）不需要恢复原始值，因为不会影响其他测试用例的执行。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
| void testLoadBalancerForwardRule() { | ||
| def lb1 = env.inventoryByName("lb1") as LoadBalancerInventory | ||
| def vm1 = env.inventoryByName("vm1") as VmInstanceInventory | ||
| def vm2 = env.inventoryByName("vm2") as VmInstanceInventory | ||
| def vm3 = env.inventoryByName("vm3") as VmInstanceInventory | ||
| def l3 = env.inventoryByName("l3") as L3NetworkInventory | ||
|
|
||
| def listener_1 = createLoadBalancerListener { | ||
| name = "listener-1" | ||
| loadBalancerUuid = lb1.uuid | ||
| protocol = "http" | ||
| loadBalancerPort = 80 | ||
| instancePort = 8080 | ||
| } as LoadBalancerListenerInventory | ||
|
|
||
| def serverGroup_1 = createLoadBalancerServerGroup { | ||
| name = "server-group-1" | ||
| loadBalancerUuid = lb1.uuid | ||
| } as LoadBalancerServerGroupInventory | ||
|
|
||
| addServerGroupToLoadBalancerListener { | ||
| serverGroupUuid = serverGroup_1.uuid | ||
| listenerUuid = listener_1.uuid | ||
| } | ||
|
|
||
| def vmNic1 = vm1.vmNics.find{ nic -> nic.l3NetworkUuid == l3.uuid } as VmNicInventory | ||
| assert vmNic1 != null | ||
|
|
||
| VirtualRouterLoadBalancerBackend.RefreshLbCmd cmd = null | ||
| env.afterSimulator(VirtualRouterLoadBalancerBackend.REFRESH_LB_PATH) { rsp, HttpEntity<String> e -> | ||
| cmd = JSONObjectUtil.toObject(e.body, VirtualRouterLoadBalancerBackend.RefreshLbCmd.class) | ||
| return rsp | ||
| } | ||
|
|
||
| def acl1 = createAccessControlList { | ||
| name = "acl-1" | ||
| } as AccessControlListInventory | ||
|
|
||
| def aclRule1 = addAccessControlListRedirectRule { | ||
| name = "acl-rule-1" | ||
| aclUuid = acl1.uuid | ||
| domain = "zstack.local" | ||
| url = "/test1" | ||
| } as AccessControlListEntryInventory | ||
|
|
||
| def rvo = Q.New(AccessControlListEntryVO.class) | ||
| .eq(AccessControlListEntryVO_.uuid, aclRule1.uuid) | ||
| .find() | ||
| assert rvo != null | ||
| assert rvo.getRedirectPort() == null | ||
|
|
||
| addAccessControlListToLoadBalancer { | ||
| listenerUuid = listener_1.uuid | ||
| serverGroupUuids = [serverGroup_1.uuid] | ||
| aclUuids = [acl1.uuid] | ||
| aclType = "redirect" | ||
| } | ||
|
|
||
| assert Q.New(AccessControlListEntryVO.class) | ||
| .eq(AccessControlListEntryVO_.uuid, aclRule1.uuid) | ||
| .eq(AccessControlListEntryVO_.redirectPort, 8080) | ||
| .isExists() | ||
|
|
||
| addBackendServerToServerGroup { | ||
| serverGroupUuid = serverGroup_1.uuid | ||
| vmNics = [[ | ||
| uuid: vmNic1.uuid, | ||
| ipVersion: "4" | ||
| ]] | ||
| } | ||
| assert cmd != null | ||
| def lbTO = cmd.lbs.find { it.lbUuid == lb1.uuid } | ||
| assert lbTO != null | ||
| def redirectRuleTO = lbTO.redirectRules.find { it.redirectRuleUuid == aclRule1.uuid } | ||
| assert redirectRuleTO != null | ||
| assert redirectRuleTO.redirectPort == 8080 | ||
|
|
||
| cmd = null | ||
| def acl2 = createAccessControlList { | ||
| name = "acl-2" | ||
| } as AccessControlListInventory | ||
| def aclRule2 = addAccessControlListRedirectRule { | ||
| name = "acl-rule-2" | ||
| aclUuid = acl2.uuid | ||
| domain = "zstack.local" | ||
| url = "/test2" | ||
| redirectPort = 9090 | ||
| } as AccessControlListEntryInventory | ||
|
|
||
| assert Q.New(AccessControlListEntryVO.class) | ||
| .eq(AccessControlListEntryVO_.uuid, aclRule2.uuid) | ||
| .eq(AccessControlListEntryVO_.redirectPort, 9090) | ||
| .isExists() | ||
|
|
||
| addAccessControlListToLoadBalancer { | ||
| listenerUuid = listener_1.uuid | ||
| serverGroupUuids = [serverGroup_1.uuid] | ||
| aclUuids = [acl2.uuid] | ||
| aclType = "redirect" | ||
| } | ||
|
|
||
| lbTO = cmd.lbs.find { it.lbUuid == lb1.uuid } | ||
| assert lbTO != null | ||
| redirectRuleTO = lbTO.redirectRules.find { it.redirectRuleUuid == aclRule2.uuid } | ||
| assert redirectRuleTO != null | ||
| assert redirectRuleTO.redirectPort == 9090 | ||
| } |
There was a problem hiding this comment.
用例覆盖点清晰，但建议补一个 cmd != null 断言提升稳定性
第二段 acl2 attach 后直接使用 cmd.lbs，建议先 assert cmd != null（避免 refresh 未触发时 NPE 导致误判）。
Proposed diff (extra assertion)
addAccessControlListToLoadBalancer {
listenerUuid = listener_1.uuid
serverGroupUuids = [serverGroup_1.uuid]
aclUuids = [acl2.uuid]
aclType = "redirect"
}
-
+ assert cmd != null
lbTO = cmd.lbs.find { it.lbUuid == lb1.uuid }🤖 Prompt for AI Agents
In
`@test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy`
around lines 191 - 297, The test may NPE when using cmd after attaching acl2
because the refresh callback might not have run; add an assertion that cmd !=
null before accessing cmd.lbs after creating acl2 and attaching it (i.e., insert
assert cmd != null right before "lbTO = cmd.lbs.find { it.lbUuid == lb1.uuid
}"), ensuring the RefreshLbCmd cmd captured by the
env.afterSimulator(VirtualRouterLoadBalancerBackend.REFRESH_LB_PATH) handler is
present.
There was a problem hiding this comment.
Comment from pengchao.liu:
won't do
There was a problem hiding this comment.
@MatheMatrix, acknowledged. I understand you've decided not to add the additional assertion.
MatheMatrix
force-pushed
the
sync/pengchao.liu/ZSTAC-79223@@3
branch
from
d3c9981 to
4512d2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java`:
- Around line 2063-2071: The current two-step flow (dbf.persistCollection(refs)
then SQL.New(AccessControlListEntryVO)...update()) risks concurrent overwrite
and lacks transactional atomicity; wrap the persist and redirectPort update in a
single transaction and replace the blind batch UPDATE with a selective, locked
read-modify-write: after starting a transaction (or annotating the enclosing
method with `@Transactional`), for each aclUuid from msg.getAclUuids() load the
corresponding AccessControlListEntryVO row(s) with a "FOR UPDATE" lock, check if
AccessControlListEntryVO.getRedirectPort() is null and only then
setRedirectPort(instancePort) and save that VO (instead of the current
SQL.New(...) bulk update); ensure this uses the same transaction that calls
dbf.persistCollection(refs) so persistence and redirectPort updates are atomic
and cannot be concurrently overwritten.
🧹 Nitpick comments (1)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (1)
2026-2031: 建议使用泛型参数化 Map 类型Line 2028 使用了原始类型
Map data = new HashMap();，建议使用参数化类型以提高类型安全性。♻️ 建议的修改
- Map data = new HashMap(); + Map<String, Object> data = new HashMap<>();
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
sdk/src/main/java/org/zstack/sdk/AccessControlListEntryInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/AddAccessControlListRedirectRuleAction.javais excluded by!sdk/**
📒 Files selected for processing (14)
conf/db/upgrade/V5.5.6__schema.sqlplugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovyplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javatest/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy
🚧 Files skipped from review as they are similar to previous changes (8)
- plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.java
- conf/db/upgrade/V5.5.6__schema.sql
- plugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.java
- plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.java
- plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.java
- plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovy
- test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy
- plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
🔍 Remote MCP AtlassianMCP
Additional context from Jira (ZSTAC-79223)
- Issue: ZSTAC-79223 — "[浙江大学]负载均衡监听器支持通过配置域名转发规则指定到不同主机的不同端口" (New Feature, P0).
- Fix version: 5.5.6.
- Goal / description (concise): allow configuring per-forwarding-rule backend port so domain-based forwarding can route to different hosts on different ports; also allow multiple listeners with duplicate frontend ports. (Includes two example images in attachments.)
- Assignee: 刘鹏超 (pengchao.liu).
- Status: Open. Priority: P0. Issue type: New Feature.
- Worklog/comments: development activity recorded by 张建军 and 刘鹏超 (worklogs show coding on control/data plane and UT fixes on 2026-01-13 to 2026-01-15).
- Related link: outward issue SUG-2212 (implements/related).
Sources:
- AtlassianMCP jira_get_issue for ZSTAC-79223 (full issue, worklog, comments, attachments).
🔇 Additional comments (11)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.java (1)
10-11: LGTM!新增的全局属性
UPGRADE_LB_REDIRECT_RULE遵循了现有的命名规范和模式，与UPGRADE_LB_SERVER_GROUP保持一致，默认值设为false符合升级标志的最佳实践。plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.java (3)
28-28: LGTM!新增的
redirectPort字段使用Integer类型允许空值，适合可选字段的场景。
51-63: LGTM!构造函数正确地从 VO 映射了
redirectPort字段，与其他字段的处理方式保持一致。
121-127: LGTM!Getter 和 Setter 方法遵循 JavaBean 规范，实现正确。
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (2)
2076-2107: LGTM - FlowChain 刷新逻辑实现正确刷新监听器的流程正确处理了以下情况：
refs为空时直接跳过- 仅在监听器需要刷新时才触发
RefreshLoadBalancerMsg- 失败时正确回滚已持久化的
refs
2122-2155: LGTM - handle 方法重构合理将 ACL 添加逻辑委托给
doAddAccessControlListToLoadBalancer方法，使用Completion回调模式统一处理成功和失败情况，代码结构更清晰。plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.java (1)
28-38: LGTM!示例方法正确展示了新增的
redirectPort字段，使用 8080 作为示例端口值是合理的。plugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.java (1)
24-36: LGTM!示例方法正确展示了重定向规则变更事件中的
redirectPort字段，与其他 API 示例保持一致。plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.java (3)
47-48: 代码实现规范，端口验证正确。新增的
redirectPort字段设计合理：
- 使用
Integer类型允许为null（表示可选）- 端口范围校验
{1, 65535}符合 TCP/UDP 端口规范required = false适用于可选覆盖后端端口的场景
107-113: LGTM!Getter 和 Setter 方法实现符合 Java Bean 规范，命名遵循 lowerCamelCase 风格。
115-126: 示例方法更新正确。
__example__()方法已正确更新，包含新增的redirectPort参数示例值 8080，有助于生成准确的 API 文档。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| Integer instancePort = lblVo.getInstancePort(); | ||
| msg.getAclUuids().forEach(aclUuid -> { | ||
| SQL.New(AccessControlListEntryVO.class) | ||
| .eq(AccessControlListEntryVO_.aclUuid, aclUuid) | ||
| .eq(AccessControlListEntryVO_.type, AclEntryType.RedirectRule.toString()) | ||
| .isNull(AccessControlListEntryVO_.redirectPort) | ||
| .set(AccessControlListEntryVO_.redirectPort, instancePort) | ||
| .update(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and get context around the code
cd plugin/loadBalancer && find . -name "LoadBalancerBase.java" -type f | head -5Repository: MatheMatrix/zstack
Length of output: 129
🏁 Script executed:
# Get more context about the method containing these lines
sed -n '2040,2090p' plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaRepository: MatheMatrix/zstack
Length of output: 2819
🏁 Script executed:
# Check the relationship between LoadBalancerListener and ACL
rg "LoadBalancerListenerACLRefVO" plugin/loadBalancer/src/main/java --type java -B 2 -A 5 | head -150Repository: MatheMatrix/zstack
Length of output: 21632
🏁 Script executed:
# Search for transaction boundaries in this file
rg "@Transactional|SQL\.New\(" plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java --type java -B 1 -A 1 | head -100Repository: MatheMatrix/zstack
Length of output: 5361
ACL 条目 redirectPort 更新存在并发安全隐患
代码在两个不同的操作中更新 redirectPort：先通过 dbf.persistCollection(refs) 持久化 ACL 绑定关系，再通过 SQL.New(AccessControlListEntryVO.class)...update() 批量更新 ACL 条目。存在以下问题：
-
并发覆盖风险：同一个 ACL 可被多个监听器绑定（LoadBalancerListenerACLRefVO 中 listenerUuid 不同而 aclUuid 相同）。当两个监听器同时绑定该 ACL 但 instancePort 不同时，后一个操作会覆盖先前的 redirectPort 值，导致配置错误。
-
事务原子性缺失：持久化和更新操作不在同一事务内执行。若两操作间发生异常，ACL 绑定关系已保存但条目 redirectPort 未更新，造成数据不一致。
🤖 Prompt for AI Agents
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java`
around lines 2063 - 2071, The current two-step flow (dbf.persistCollection(refs)
then SQL.New(AccessControlListEntryVO)...update()) risks concurrent overwrite
and lacks transactional atomicity; wrap the persist and redirectPort update in a
single transaction and replace the blind batch UPDATE with a selective, locked
read-modify-write: after starting a transaction (or annotating the enclosing
method with `@Transactional`), for each aclUuid from msg.getAclUuids() load the
corresponding AccessControlListEntryVO row(s) with a "FOR UPDATE" lock, check if
AccessControlListEntryVO.getRedirectPort() is null and only then
setRedirectPort(instancePort) and save that VO (instead of the current
SQL.New(...) bulk update); ensure this uses the same transaction that calls
dbf.persistCollection(refs) so persistence and redirectPort updates are atomic
and cannot be concurrently overwritten.
There was a problem hiding this comment.
Comment from pengchao.liu:
一个 ACL 不会绑定多个监听器，won't do
There was a problem hiding this comment.
🧩 Analysis chain
@zstack-robot-1 I need to verify the constraint you mentioned. Let me check the codebase to understand the ACL-to-listener relationship and whether it's enforced.
🏁 Script executed:
#!/bin/bash
# Check the LoadBalancerListenerACLRefVO entity definition for unique constraints
rg -A 30 "class LoadBalancerListenerACLRefVO" plugin/acl plugin/loadBalancer --type java
Length of output: 5529
🏁 Script executed:
#!/bin/bash
# Check for unique constraints or indexes on the ACL ref table
rg "@Table|@UniqueConstraint|unique|UNIQUE" plugin/acl plugin/loadBalancer --type java -B 2 -A 5 | grep -A 10 -B 10 "LoadBalancerListenerACLRefVO"
Length of output: 3148
🏁 Script executed:
#!/bin/bash
# Check API validation logic for adding ACL to listener
rg "APIAddAccessControlListToLoadBalancerMsg" plugin/loadBalancer plugin/acl --type java -A 20 | head -100
Length of output: 15721
🏁 Script executed:
#!/bin/bash
# Look for any validation that prevents one ACL from binding to multiple listeners
rg "prevent|validate|check" plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java --type java -B 3 -A 3 | grep -i acl -A 5 -B 5
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the SQL schema definition for the ACL ref table
cat conf/db/upgrade/V5.5.6__schema.sql 2>/dev/null || echo "Schema file not found in this path"
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Search for the ACL ref table creation in other schema files
fd -e sql . conf/db 2>/dev/null | xargs grep -l "LoadBalancerListenerACLRef" | head -5
Length of output: 69
🏁 Script executed:
#!/bin/bash
# Check the table creation SQL for LoadBalancerListenerACLRef
grep -A 20 "LoadBalancerListenerACLRef" conf/db/upgrade/V3.9.0__schema.sql conf/db/upgrade/V4.1.3__schema.sql
Length of output: 2941
🏁 Script executed:
#!/bin/bash
# Look for the actual scenario - can same ACL be added to multiple listeners?
# Check if there's code that checks for existing ACL-listener bindings
rg "Q\.New\(LoadBalancerListenerACLRefVO" plugin/loadBalancer --type java -B 2 -A 8 | head -80
Length of output: 11491
🏁 Script executed:
#!/bin/bash
# Check the doAddAccessControlListToLoadBalancer method more carefully for duplicate prevention
sed -n '2038,2130p' plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java
Length of output: 4622
🏁 Script executed:
#!/bin/bash
# Search for evidence that same ACL can bind to multiple listeners or servergroups
rg "serverGroupUuid|ServerGroup" plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerListenerACLRefVO.java --type java -B 3 -A 3
Length of output: 395
@zstack-robot-1 感谢澄清设计意图。不过从代码实现来看，目前并没有技术手段强制这一约束，存在以下证据：
🏁 Script executed:
#!/bin/bash
# Show that LoadBalancerExtension actively queries multiple listeners for one ACL
grep -B 3 -A 8 "refreshLoadBalancerByAcl" plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerExtension.java | head -20
Length of output: 744
🏁 Script executed:
#!/bin/bash
# Show the database schema has no unique constraint to prevent duplicate ACL-listener bindings
echo "=== Database Schema Evidence ==="
grep -A 15 "CREATE TABLE.*LoadBalancerListenerACLRefVO" conf/db/upgrade/V3.9.0__schema.sql | grep -E "UNIQUE|PRIMARY|CONSTRAINT"
Length of output: 404
🏁 Script executed:
#!/bin/bash
# Check if there's any API validation preventing duplicate ACL-listener binding
rg "duplicate|already.*exist|already.*bind" plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java --type java -B 3 -A 3 | grep -i acl -A 5 -B 5
Length of output: 3538
@zstack-robot-1 根据代码分析，当前实现确实支持一个 ACL 绑定多个监听器：
证据 1：系统主动查询多监听器场景
// LoadBalancerExtension.java - refreshLoadBalancerByAcl
List<String> listenerUuids = Q.New(LoadBalancerListenerACLRefVO.class)
.select(LoadBalancerListenerACLRefVO_.listenerUuid)
.eq(LoadBalancerListenerACLRefVO_.aclUuid, aclUuid)
.listValues(); // 返回列表，不是单个值证据 2：数据库无唯一约束
-- LoadBalancerListenerACLRefVO 表仅 id 列有 UNIQUE 约束
PRIMARY KEY (`id`),
UNIQUE KEY `id` (`id`)
-- 缺少 UNIQUE KEY (aclUuid, listenerUuid) 防止重复绑定证据 3：API 校验仅限同一监听器内
// LoadBalancerApiInterceptor.java
// 校验信息："the access-control-list groups are already on the load balancer listener"
// 仅阻止 同一ACL 重复绑定到 同一监听器，不阻止绑定到不同监听器关于并发问题：
虽然代码使用了 .isNull(AccessControlListEntryVO_.redirectPort) 条件减少覆盖风险，但 dbf.persistCollection(refs) 和 SQL.New(...).update() 仍是两个独立操作，缺乏事务原子性保护。
理解您"won't do"的决定。如果团队有业务层约束确保不会发生多监听器绑定场景，可以保持现状。不过建议考虑在数据库层或 API 校验层显式添加约束，避免未来误用导致数据不一致。
GlobalPropertyImpact APIImpact DBImpact Resolves: ZSTAC-79223 Change-Id: I7771696566617172666f776a797a756672767774
MatheMatrix
force-pushed
the
sync/pengchao.liu/ZSTAC-79223@@3
branch
from
4512d2e to
1681738
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsgDoc_zh_cn.groovy`:
- Around line 105-113: 在 API 文档片段中补全 redirectPort 字段的 desc 字段：在
org.zstack.header.acl.APIAddAccessControlListRedirectRuleMsgDoc_zh_cn.groovy 中找到
column 定义的 name "redirectPort" 并为 desc
提供简洁说明（例如指明这是目标重定向端口、端口范围或默认行为、此字段为可选），保持与 type "Integer" 和 since "5.5.0"
一致且语义清晰以避免文档空白。
♻️ Duplicate comments (4)
test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy (2)
180-188: 全局开关未恢复，可能污染后续用例
LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE为全局静态状态，建议在用例结束后恢复原值以避免串扰。🧪 建议修复（恢复全局开关）
class VirtualRouterLoadBalancerForwardRuleCase extends SubCase { DatabaseFacade dbf EnvSpec env LoadBalancerManagerImpl lbMgr + boolean oldUpgradeFlag `@Override` void test() { dbf = bean(DatabaseFacade.class) lbMgr = bean(LoadBalancerManagerImpl.class) + oldUpgradeFlag = LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE env.create { - testLoadBalancerForwardRule() - LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = true - testUpgradeLoadBalancerForwardRule() + try { + testLoadBalancerForwardRule() + LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = true + testUpgradeLoadBalancerForwardRule() + } finally { + LoadBalancerGlobalProperty.UPGRADE_LB_REDIRECT_RULE = oldUpgradeFlag + } } } }
292-295: 使用 cmd 前先断言非空
第二次 attach 后直接访问cmd.lbs可能导致 NPE，建议先断言cmd != null。🧪 建议修复（补充断言）
addAccessControlListToLoadBalancer { listenerUuid = listener_1.uuid serverGroupUuids = [serverGroup_1.uuid] aclUuids = [acl2.uuid] aclType = "redirect" } - + assert cmd != null lbTO = cmd.lbs.find { it.lbUuid == lb1.uuid }plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (1)
2063-2071: redirectPort 批量更新仍有并发覆盖/非原子风险
同一 ACL 可能被不同 listener 绑定且 instancePort 不同，目前批量 UPDATE 会造成覆盖不确定性；且与 refs 持久化不在同一事务内，异常时会留下一致性问题。建议在同一事务里进行持久化与逐条判空更新，并在多 listener 场景下明确处理策略（如冲突时跳过并记录日志）。plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.java (1)
1006-1034: 升级逻辑在“同一 ACL 关联多个 listener”时不确定
当前按aclUuid只取一条 listener 记录，若存在多个 listener（且 instancePort 不同），会导致 redirectPort 选择不可预期。建议改为收集 distinct instancePort：仅在唯一值时更新，否则跳过并记录日志。🧭 处理思路（确定性升级）
- LoadBalancerListenerVO lbListenerVO = SQL.New("select lbListener from LoadBalancerListenerVO lbListener, LoadBalancerListenerACLRefVO ref, AccessControlListVO acl " + - "where acl.uuid = ref.aclUuid and ref.listenerUuid = lbListener.uuid and acl.uuid = :aclUuid") - .param("aclUuid", rule.getAclUuid()) - .find(); - if (lbListenerVO == null) { - continue; - } - - rule.setRedirectPort(lbListenerVO.getInstancePort()); - toUpgradeRules.add(rule); + List<Integer> instancePorts = SQL.New( + "select distinct lbListener.instancePort from LoadBalancerListenerVO lbListener, LoadBalancerListenerACLRefVO ref " + + "where ref.listenerUuid = lbListener.uuid and ref.aclUuid = :aclUuid", + Integer.class) + .param("aclUuid", rule.getAclUuid()) + .list(); + if (instancePorts.isEmpty()) { + continue; + } + if (instancePorts.size() > 1) { + logger.warn(String.format("skip upgrading redirectPort for acl[uuid:%s], multiple instancePorts found: %s", + rule.getAclUuid(), instancePorts)); + continue; + } + rule.setRedirectPort(instancePorts.get(0)); + toUpgradeRules.add(rule);
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
sdk/src/main/java/org/zstack/sdk/AccessControlListEntryInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/AddAccessControlListRedirectRuleAction.javais excluded by!sdk/**
📒 Files selected for processing (15)
conf/db/upgrade/V5.5.6__schema.sqlplugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsgDoc_zh_cn.groovyplugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovyplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javatest/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
- plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalProperty.java
- plugin/acl/src/main/java/org/zstack/acl/AccessControlListManagerImpl.java
- plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventory.java
- conf/db/upgrade/V5.5.6__schema.sql
- plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryInventoryDoc_zh_cn.groovy
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javatest/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovyplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsgDoc_zh_cn.groovyplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
plugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
🧬 Code graph analysis (2)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java (2)
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java (1)
RedirectRule(346-391)plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.java (1)
LoadBalancerConstants(15-180)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.java (2)
core/src/main/java/org/zstack/core/db/Q.java (1)
Q(16-193)plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java (1)
RedirectRule(346-391)
🔍 Remote MCP AtlassianMCP
Relevant Jira context (ZSTAC-79223)
-
Issue: ZSTAC-79223 — "[浙江大学]负载均衡监听器支持通过配置域名转发规则指定到不同主机的不同端口"
-
Status / Priority / Assignee: Open, P0, assignee 刘鹏超 (pengchao.liu@zstack.io)
-
Fix version: 5.5.6
-
Goal / functional requirements (from description):
- Allow per-forwarding-rule backend port configuration so domain-based forwarding can route to different hosts on different ports.
- Allow multiple listeners with duplicate frontend ports (avoid port-conflict errors when creating multiple listeners on same frontend port).
-
Artifacts / attachments: two diagrams illustrating desired domain->port forwarding behavior are attached to the issue. URLs provided in Jira attachments.
-
Development activity / worklogs: multiple worklogs and recent activity (Jan 13–15, 2026) recorded by authors including 刘鹏超 and 张建军, indicating implementation and UT work.
Summary relevance to this PR: the PR adds a persistent redirectPort field (DB + VO + API + inventory), populates and propagates it through ACL, load balancer manager/flows, virtual router backend, and includes an upgrade path and UT — matching the Jira requirements to support per-rule backend ports and rollout for existing rules.
🔇 Additional comments (11)
plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListEntryEvent.java (1)
32-34: 示例值同步新增字段，OK。
示例包含 redirectPort 有助于 API 文档一致性。plugin/acl/src/main/java/org/zstack/header/acl/APIChangeAccessControlListRedirectRuleEvent.java (1)
28-34: 示例值补充到位，OK。
示例对象新增 redirectPort 有助于前后端一致性。plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO.java (2)
49-51: 字段持久化新增合理，OK。
171-176: 访问器补充完整，OK。plugin/acl/src/main/java/org/zstack/header/acl/AccessControlListEntryVO_.java (1)
22-24: Metamodel 同步新增字段，OK。plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsg.java (3)
47-48: redirectPort 参数校验清晰
范围限制和可选性定义合理。
107-113: 访问器新增 OK
Getter/Setter 与字段一致。
124-124: 示例覆盖新字段
示例里补充 redirectPort 有助于 API 文档一致性。test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerForwardRuleCase.groovy (3)
46-178: 环境构建覆盖面充足
测试环境包含 VR/LB/VM 等关键路径，场景完整。
299-325: 升级路径断言清晰
对升级前后 redirectPort 的校验覆盖到位。
334-345: 资源清理完整
LB 与环境删除覆盖充分。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| column { | ||
| name "redirectPort" | ||
| enclosedIn "params" | ||
| desc "" | ||
| location "body" | ||
| type "Integer" | ||
| optional true | ||
| since "5.5.0" | ||
| } |
There was a problem hiding this comment.
补齐 redirectPort 字段描述，避免文档空白。
desc 为空会导致 API 文档信息缺失，请补充清晰说明。
🛠️ 建议补充描述
column {
name "redirectPort"
enclosedIn "params"
- desc ""
+ desc "Redirect backend port for the matched rule."
location "body"
type "Integer"
optional true
since "5.5.0"
}📝 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.
| column { | |
| name "redirectPort" | |
| enclosedIn "params" | |
| desc "" | |
| location "body" | |
| type "Integer" | |
| optional true | |
| since "5.5.0" | |
| } | |
| column { | |
| name "redirectPort" | |
| enclosedIn "params" | |
| desc "Redirect backend port for the matched rule." | |
| location "body" | |
| type "Integer" | |
| optional true | |
| since "5.5.0" | |
| } |
🤖 Prompt for AI Agents
In
`@plugin/acl/src/main/java/org/zstack/header/acl/APIAddAccessControlListRedirectRuleMsgDoc_zh_cn.groovy`
around lines 105 - 113, 在 API 文档片段中补全 redirectPort 字段的 desc 字段：在
org.zstack.header.acl.APIAddAccessControlListRedirectRuleMsgDoc_zh_cn.groovy 中找到
column 定义的 name "redirectPort" 并为 desc
提供简洁说明（例如指明这是目标重定向端口、端口范围或默认行为、此字段为可选），保持与 type "Integer" 和 since "5.5.0"
一致且语义清晰以避免文档空白。
There was a problem hiding this comment.
Comment from pengchao.liu:
下次再提pr的时候补上
There was a problem hiding this comment.
@ZStack-Robot 好的,理解了。期待在下一个 PR 中看到 redirectPort 字段的描述补充 👍
🧠 Learnings used
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java:471-545
Timestamp: 2025-08-22T05:36:40.467Z
Learning: In ZStack项目中，对于核心逻辑相关的代码，团队倾向于暂时不进行重构，即使代码复杂度较高，优先保证核心功能的稳定性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2325
File: compute/src/main/java/org/zstack/compute/vm/VmMigrateCallExtensionFlow.java:29-29
Timestamp: 2025-07-24T05:53:10.246Z
Learning: 在ZStack项目的重构过程中，团队采用务实的优先级策略：优先完成影响运行时功能的代码重构，对于注释中的旧引用等非功能性问题可以延后处理，因为这些不会影响系统的实际运行。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java:364-501
Timestamp: 2025-08-04T03:19:07.454Z
Learning: In ZStack project, developers may prefer to keep related validation logic together in a single method (like validateStaticIpTagsInApiMessage with 130+ lines) when the architecture is clear and there's no code reuse, rather than splitting into smaller methods based purely on single responsibility principle.
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2419
File: plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java:986-1067
Timestamp: 2025-08-13T02:31:40.023Z
Learning: 在ZStack项目中，团队倾向于保持统一的业务流程方法不进行拆分，即使方法较长也会维持在单个方法中以保持业务逻辑的完整性。
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2496
File: plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmBackend.java:2545-2566
Timestamp: 2025-08-24T07:35:42.793Z
Learning: 在 ZStack 代码库中，当响应类包含数值字段（如 size、actualSize）时，优先使用原始类型（long）而不是包装类型（Long），以避免 NPE 风险和不必要的装箱/拆箱操作。如果 Agent 端可能不设置该字段，应在 Agent 端确保设置默认值，而不是在使用端做 null 检查。
| } | ||
| List<AccessControlListEntryVO> redirectRules = Q.New(AccessControlListEntryVO.class) | ||
| .eq(AccessControlListEntryVO_.type, AclEntryType.RedirectRule.toString()) | ||
| .isNull(AccessControlListEntryVO_.redirectPort) |
There was a problem hiding this comment.
Comment from shixin.ruan:
默认值为啥不设置为0,是不是就不需要升级处理了
There was a problem hiding this comment.
Comment from pengchao.liu:
acl rule还有另一种类型，默认设置为0反而会有歧义。综合考虑默认为NULL处理起来更方便
| } | ||
|
|
||
| private void handle(APIAddAccessControlListToLoadBalancerMsg msg) { | ||
| DebugUtils.Assert(msg.getAclType() != null && msg.getAclUuids() != null, "parameters cannot be null"); |
There was a problem hiding this comment.
Comment from shixin.ruan:
参数判断统一api interceptor, 而且现在有新的error log个格式。
There was a problem hiding this comment.
Comment from pengchao.liu:
这是原有代码，不过确实是个好建议。下次提pr时再修改
GlobalPropertyImpact
APIImpact
DBImpact
Resolves: ZSTAC-79223
Change-Id: I7771696566617172666f776a797a756672767774
sync from gitlab !9030