Skip to content

Support VPC router DNS records#4010

Open
zstack-robot-1 wants to merge 1 commit into
5.5.22from
sync/hongbo.liu/feature-vpc-dns-record
Open

Support VPC router DNS records#4010
zstack-robot-1 wants to merge 1 commit into
5.5.22from
sync/hongbo.liu/feature-vpc-dns-record

Conversation

@zstack-robot-1
Copy link
Copy Markdown
Collaborator

sync from gitlab !9903

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Review Change Stack

Walkthrough

新增 VPC 路由器 DNS 记录表与索引;添加域名校验工具与单元测试;补充中英文 i18n 文案;在测试库新增 add/remove/update/get VPC Router DNS 记录的 ApiHelper 方法;新增一组 VPC 错误码常量。

Changes

VPC 路由器 DNS 记录支持

Layer / File(s) Summary
数据库模式与索引
conf/db/upgrade/V5.5.22__schema.sql
创建 zstack.VpcRouterDnsRecordVO 表,包含 idvpcRouterUuidvpcHaGroupUuidtype(默认 'A')、domainipcreateDatelastOpDate,为 vpcRouterUuid/vpcHaGroupUuid/type/domain 建立索引,外键采用 ON DELETE CASCADE
错误码常量
utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
新增 VPC 相关错误码常量 ORG_ZSTACK_VPC_10050ORG_ZSTACK_VPC_10057(字符串取值与常量名一致)。
域名校验能力与测试
utils/src/main/java/org/zstack/utils/network/NetworkUtils.java, utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java
新增域名总长与单标签长度常量,并实现 isDomainName(String);新增 testDomainName() 覆盖合法/非法与边界场景的单元测试并加入静态断言导入。
国际化消息定义 (EN)
conf/i18n/messages_en_US.properties
新增与域名/IPv4 校验、VPC 路由状态、DNS 记录添加/未添加/超限及同步失败相关的英文消息模板(带占位符)。
国际化消息定义 (ZH)
conf/i18n/messages_zh_CN.properties
新增与域名/IPv4 校验、VPC 路由状态、DNS 记录添加/未添加/超限及同步失败相关的中文消息模板(带占位符)。
DNS 记录测试 API 辅助方法
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
新增 addDnsRecordToVpcRouterremoveDnsRecordFromVpcRouterupdateDnsRecordToVpcRoutergetVpcRouterDnsRecord 四个辅助方法,支持闭包配置、session 注入和 ApiPath 跟踪,返回经 errorOut 处理的调用结果。

预估代码审查工作量

🎯 3 (Moderate) | ⏱️ ~20 minutes

我是小兔写补丁,数据库里种新林,
域名细看每一段,文本中英映心,
测试助手来排队,错误码也记名。 🐰


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Title check ❌ Error 标题不符合规范格式:缺少必需的 [scope]: 前缀,仅为简单的主题描述。 标题应遵循 '[scope]: ' 格式(如 'feat[vpc]: Support VPC router DNS records'),总长度不超过 72 字符。
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed 描述与变更集相关,清晰指出了来源(GitLab MR !9903),虽然简洁但足以说明同步来源。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/hongbo.liu/feature-vpc-dns-record
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch sync/hongbo.liu/feature-vpc-dns-record

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.42.2)
utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java (1)

57-58: 💤 Low value

建议增强测试用例的可读性和完整性。

当前边界测试使用了魔法值 64 和 254,建议改进:

  1. 可以引用 NetworkUtils 中的常量或添加注释说明测试意图
  2. 考虑补充以下边界测试用例以提高覆盖率:
    • 空字符串 ""
    • 连续点号 "a..b"
    • 最大合法长度域名(253 字符且所有标签 ≤ 63 字符)
可读性改进示例
-        assertFalse(NetworkUtils.isDomainName(new String(new char[64]).replace('\0', 'a') + ".local"));
-        assertFalse(NetworkUtils.isDomainName(new String(new char[254]).replace('\0', 'a')));
+        // Label exceeds MAX_DOMAIN_NAME_LABEL_LENGTH (63)
+        assertFalse(NetworkUtils.isDomainName(new String(new char[64]).replace('\0', 'a') + ".local"));
+        // Total length exceeds MAX_DOMAIN_NAME_LENGTH (253)
+        assertFalse(NetworkUtils.isDomainName(new String(new char[254]).replace('\0', 'a')));
+        
+        // Additional edge cases
+        assertFalse(NetworkUtils.isDomainName(""));
+        assertFalse(NetworkUtils.isDomainName("a..b"));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java` around lines
57 - 58, Replace the magic numeric tests in TestNetworkUtils by clarifying
intent and adding boundary cases: instead of raw 64 and 254, reference or
comment the related length constants from NetworkUtils and/or explain that 63
and 253 are per-label and total limits; add assertions calling
NetworkUtils.isDomainName for an empty string (""), a name with consecutive dots
("a..b"), and a constructed maximum-legal domain (total length 253 with all
labels ≤63) to validate the true/false expectations; keep existing long-name
negative tests but document why each length is chosen and use helper(s) to build
the repeated-label strings so the test is readable and self-explanatory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@conf/db/upgrade/V5.5.22__schema.sql`:
- Around line 90-91: The SQL uses illegal zero DATETIME defaults which break
strict SQL modes; update the two TIMESTAMP columns so they do not use DEFAULT
'0000-00-00 00:00:00': change `createDate` to use a non-zero sentinel (e.g.
'2000-01-01 00:00:00') as its DEFAULT, and change `lastOpDate` to use the same
non-zero DEFAULT while keeping ON UPDATE CURRENT_TIMESTAMP (i.e. `lastOpDate`
DEFAULT '2000-01-01 00:00:00' ON UPDATE CURRENT_TIMESTAMP) to avoid
NO_ZERO_DATE/1293 errors while preserving auto-update behavior.

In `@conf/i18n/messages_en_US.properties`:
- Line 14063: Update the message value for the property key currently written as
"domain[{0}] is not a valid format" to a more natural English phrase such as
"domain[{0}] is not in a valid format"; also locate the related message around
line 14083 and change its wording to "has not been added" (ensure placeholders
remain as {0} where used) so both entries use correct, natural English and no
Chinese.

In `@utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java`:
- Line 52: The test TestNetworkUtils currently uses a literal Chinese string in
the assertion (NetworkUtils.isDomainName("中文.local")); replace the non-ASCII
literal with a Unicode-escaped form (e.g. "\u4E2D\u6587.local") or extract it
into a clearly named variable (e.g. CHINESE_LABEL) and add an English comment
explaining it tests rejection of Unicode/Chinese labels so the test remains
clear while avoiding raw Chinese characters in source; update the assertion to
use the new escaped string or variable and keep the test name and behavior
unchanged.

---

Nitpick comments:
In `@utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java`:
- Around line 57-58: Replace the magic numeric tests in TestNetworkUtils by
clarifying intent and adding boundary cases: instead of raw 64 and 254,
reference or comment the related length constants from NetworkUtils and/or
explain that 63 and 253 are per-label and total limits; add assertions calling
NetworkUtils.isDomainName for an empty string (""), a name with consecutive dots
("a..b"), and a constructed maximum-legal domain (total length 253 with all
labels ≤63) to validate the true/false expectations; keep existing long-name
negative tests but document why each length is chosen and use helper(s) to build
the repeated-label strings so the test is readable and self-explanatory.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 4dc42d44-3476-4c7c-90af-930cbd7364ac

📥 Commits

Reviewing files that changed from the base of the PR and between f83c595 and fc7371a.

⛔ Files ignored due to path filters (19)
  • conf/i18n.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-de-DE.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-fr-FR.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-id-ID.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-ja-JP.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-ko-KR.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-ru-RU.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-th-TH.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_TW.json is excluded by !**/*.json
  • sdk/src/main/java/org/zstack/sdk/AddDnsRecordToVpcRouterAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/AddDnsRecordToVpcRouterResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetVpcRouterDnsRecordAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetVpcRouterDnsRecordResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveDnsRecordFromVpcRouterAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveDnsRecordFromVpcRouterResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/VpcRouterDnsRecordInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/VpcRouterVmInventory.java is excluded by !sdk/**
📒 Files selected for processing (7)
  • conf/db/upgrade/V5.5.22__schema.sql
  • conf/i18n/messages_en_US.properties
  • conf/i18n/messages_zh_CN.properties
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
  • utils/src/main/java/org/zstack/utils/network/NetworkUtils.java
  • utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java

Comment on lines +90 to +91
`createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
`lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

修复零时间默认值,避免升级兼容性风险

createDate/lastOpDate 使用 DEFAULT '0000-00-00 00:00:00' 有升级失败风险(严格 SQL 模式下常见)。建议改为仓库内已采用的非零 sentinel(如 '2000-01-01 00:00:00')以保持一致并避免 NO_ZERO_DATE 问题。Line 90、Line 91 需要调整。

建议修改
 CREATE TABLE IF NOT EXISTS `zstack`.`VpcRouterDnsRecordVO` (
@@
-    `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
-    `lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
+    `createDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00',
+    `lastOpDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
@@
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;

As per coding guidelines: **/*.sql requires “Do not use DEFAULT 0000-00-00 00:00:00”. Based on learnings: in conf/db/upgrade/*.sql, when another TIMESTAMP already uses ON UPDATE CURRENT_TIMESTAMP, prefer a non-CURRENT_TIMESTAMP sentinel default to avoid MySQL 5.7 Error 1293.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
`createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
`lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
`createDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00',
`lastOpDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conf/db/upgrade/V5.5.22__schema.sql` around lines 90 - 91, The SQL uses
illegal zero DATETIME defaults which break strict SQL modes; update the two
TIMESTAMP columns so they do not use DEFAULT '0000-00-00 00:00:00': change
`createDate` to use a non-zero sentinel (e.g. '2000-01-01 00:00:00') as its
DEFAULT, and change `lastOpDate` to use the same non-zero DEFAULT while keeping
ON UPDATE CURRENT_TIMESTAMP (i.e. `lastOpDate` DEFAULT '2000-01-01 00:00:00' ON
UPDATE CURRENT_TIMESTAMP) to avoid NO_ZERO_DATE/1293 errors while preserving
auto-update behavior.


# at: src/main/java/org/zstack/vpc/VpcApiInterceptor.java:627
# args: domain
domain[%s]\ is\ not\ a\ valid\ format = domain[{0}] is not a valid format
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

修正文案语法以避免用户侧英文提示不自然

Line 14063 和 Line 14083 的英文表达建议调整为更自然的写法(in a valid formathas not been added)。

建议修改
-domain[%s]\ is\ not\ a\ valid\ format = domain[{0}] is not a valid format
+domain[%s]\ is\ not\ in\ a\ valid\ format = domain[{0}] is not in a valid format
@@
-dns\ record[domain\:%s]\ is\ not\ added\ to\ vpc\ router[uuid\:%s] = dns record[domain:{0}] is not added to vpc router[uuid:{1}]
+dns\ record[domain\:%s]\ has\ not\ been\ added\ to\ vpc\ router[uuid\:%s] = dns record[domain:{0}] has not been added to vpc router[uuid:{1}]

As per coding guidelines, “代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写”.

Also applies to: 14083-14083

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conf/i18n/messages_en_US.properties` at line 14063, Update the message value
for the property key currently written as "domain[{0}] is not a valid format" to
a more natural English phrase such as "domain[{0}] is not in a valid format";
also locate the related message around line 14083 and change its wording to "has
not been added" (ensure placeholders remain as {0} where used) so both entries
use correct, natural English and no Chinese.

assertFalse(NetworkUtils.isDomainName(null));
assertFalse(NetworkUtils.isDomainName("*.app.local"));
assertFalse(NetworkUtils.isDomainName("db_prod.local"));
assertFalse(NetworkUtils.isDomainName("中文.local"));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

测试数据中包含中文字符,建议使用 Unicode 转义或添加英文注释。

根据编码规范:"代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写"。虽然这是用于测试 Unicode 字符拒绝的测试数据,但为了符合规范并提高代码可维护性,建议改为:

建议的修改方案

方案 1:使用 Unicode 转义序列

-        assertFalse(NetworkUtils.isDomainName("中文.local"));
+        assertFalse(NetworkUtils.isDomainName("\u4e2d\u6587.local")); // Chinese characters

方案 2:使用变量并添加英文注释

+        // Test rejection of non-ASCII characters (Chinese)
+        String unicodeDomain = "\u4e2d\u6587.local";
+        assertFalse(NetworkUtils.isDomainName(unicodeDomain));
-        assertFalse(NetworkUtils.isDomainName("中文.local"));

As per coding guidelines: 代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java` at line 52,
The test TestNetworkUtils currently uses a literal Chinese string in the
assertion (NetworkUtils.isDomainName("中文.local")); replace the non-ASCII literal
with a Unicode-escaped form (e.g. "\u4E2D\u6587.local") or extract it into a
clearly named variable (e.g. CHINESE_LABEL) and add an English comment
explaining it tests rejection of Unicode/Chinese labels so the test remains
clear while avoiding raw Chinese characters in source; update the assertion to
use the new escaped string or variable and keep the test name and behavior
unchanged.

@MatheMatrix MatheMatrix force-pushed the sync/hongbo.liu/feature-vpc-dns-record branch from fc7371a to 264e9c4 Compare May 18, 2026 09:55
Regenerate SDK actions and API artifacts for add, remove, and get VPC router DNS record operations.

Resolves: ZSTAC-85249

Change-Id: I1bbbe741422d273dc8d631d6ead98405c7ad1532
@MatheMatrix MatheMatrix force-pushed the sync/hongbo.liu/feature-vpc-dns-record branch from 264e9c4 to 9d88cef Compare May 19, 2026 10:57
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
conf/db/upgrade/V5.5.22__schema.sql (1)

92-93: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

修复零时间默认值以避免升级失败风险

Line 92-93 仍使用 DEFAULT '0000-00-00 00:00:00'。这在严格 SQL 模式和部分 MySQL 版本升级时容易报错,建议改为非零 sentinel,并保留 lastOpDateON UPDATE CURRENT_TIMESTAMP

建议修改
-    `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
-    `lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
+    `createDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00',
+    `lastOpDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00' ON UPDATE CURRENT_TIMESTAMP,

As per coding guidelines: **/*.sql 要求 “Do not use DEFAULT 0000-00-00 00:00:00”。
Based on learnings: 在 conf/db/upgrade/*.sql 且存在另一个带 ON UPDATE CURRENT_TIMESTAMP 的 TIMESTAMP 列时,使用非 CURRENT_TIMESTAMP 的非零默认值可避免 MySQL 5.7 Error 1293/严格模式兼容问题。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conf/db/upgrade/V5.5.22__schema.sql` around lines 92 - 93, The SQL uses
invalid zero DATETIME defaults for columns createDate and lastOpDate which can
fail in strict SQL modes; update the column definitions to replace DEFAULT
'0000-00-00 00:00:00' with a non-zero sentinel (e.g., a valid past timestamp
like '1970-01-01 00:00:00' or CURRENT_TIMESTAMP where appropriate) while
preserving the ON UPDATE CURRENT_TIMESTAMP behavior for lastOpDate so that
lastOpDate still auto-updates and createDate has a valid non-zero default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy`:
- Line 57778: The code calls a.conditions.collect directly which throws a NPE
when conditions is null; update the handling of a.conditions (e.g., in the block
where a.conditions is transformed) to guard against null by using a null-check
or defaulting to an empty list before calling collect (for example, replace
direct a.conditions.collect with (a.conditions ?: []) .collect or check if
(a.conditions != null) then map), ensuring downstream logic still works when
a.conditions is absent.

---

Duplicate comments:
In `@conf/db/upgrade/V5.5.22__schema.sql`:
- Around line 92-93: The SQL uses invalid zero DATETIME defaults for columns
createDate and lastOpDate which can fail in strict SQL modes; update the column
definitions to replace DEFAULT '0000-00-00 00:00:00' with a non-zero sentinel
(e.g., a valid past timestamp like '1970-01-01 00:00:00' or CURRENT_TIMESTAMP
where appropriate) while preserving the ON UPDATE CURRENT_TIMESTAMP behavior for
lastOpDate so that lastOpDate still auto-updates and createDate has a valid
non-zero default.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 5c86049b-4b29-41ac-b939-93e7f17d043f

📥 Commits

Reviewing files that changed from the base of the PR and between 264e9c4 and 9d88cef.

⛔ Files ignored due to path filters (21)
  • conf/i18n.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-de-DE.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-fr-FR.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-id-ID.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-ja-JP.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-ko-KR.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-ru-RU.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-th-TH.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_TW.json is excluded by !**/*.json
  • sdk/src/main/java/org/zstack/sdk/AddDnsRecordToVpcRouterAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/AddDnsRecordToVpcRouterResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryVpcRouterDnsRecordAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryVpcRouterDnsRecordResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveDnsRecordFromVpcRouterAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveDnsRecordFromVpcRouterResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/UpdateDnsRecordToVpcRouterAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/UpdateDnsRecordToVpcRouterResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/VpcRouterDnsRecordInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/VpcRouterVmInventory.java is excluded by !sdk/**
📒 Files selected for processing (7)
  • conf/db/upgrade/V5.5.22__schema.sql
  • conf/i18n/messages_en_US.properties
  • conf/i18n/messages_zh_CN.properties
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
  • utils/src/main/java/org/zstack/utils/network/NetworkUtils.java
  • utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java
✅ Files skipped from review due to trivial changes (3)
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
  • conf/i18n/messages_en_US.properties
  • conf/i18n/messages_zh_CN.properties
🚧 Files skipped from review as they are similar to previous changes (2)
  • utils/src/main/java/org/zstack/utils/network/NetworkUtils.java
  • utils/src/test/java/org/zstack/utils/test/TestNetworkUtils.java

c.delegate = a
c()

a.conditions = a.conditions.collect { it.toString() }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

避免 conditions 为空时触发空指针异常。

Line 57778 直接对 a.conditions 调用 collect。当调用方不传 conditions(例如查询全部记录)时,这里会抛 NullPointerException,导致查询辅助方法不可用。

建议修复
-        a.conditions = a.conditions.collect { it.toString() }
+        a.conditions = (a.conditions ?: []).collect { it.toString() }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
a.conditions = a.conditions.collect { it.toString() }
a.conditions = (a.conditions ?: []).collect { it.toString() }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy` at line 57778, The
code calls a.conditions.collect directly which throws a NPE when conditions is
null; update the handling of a.conditions (e.g., in the block where a.conditions
is transformed) to guard against null by using a null-check or defaulting to an
empty list before calling collect (for example, replace direct
a.conditions.collect with (a.conditions ?: []) .collect or check if
(a.conditions != null) then map), ensuring downstream logic still works when
a.conditions is absent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant