Skip to content

<feature>[core]: Add external service configuration APIs#4200

Open
zstack-robot-2 wants to merge 1 commit into
zsv_5.1.0from
sync/zstackio/ZSV-12395@@2
Open

<feature>[core]: Add external service configuration APIs#4200
zstack-robot-2 wants to merge 1 commit into
zsv_5.1.0from
sync/zstackio/ZSV-12395@@2

Conversation

@zstack-robot-2

Copy link
Copy Markdown
Collaborator
  • Add ExternalServiceConfigurationVO schema, inventory, query API, and add/update/delete APIs
  • Apply configuration changes to matching external services across management nodes
  • Expose serviceType in external service inventory for configuration routing
  • Generate SDK actions/results and testlib helpers for external service configuration operations
  • Register persistence and external service API metadata

DBImpact
APIImpact

Resolves: ZSV-12395

Change-Id: I6a6b76626770736871716563676b6a7a74736966

sync from gitlab !10157

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@MatheMatrix, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 42a5f16d-69dd-4a43-8684-ae51af44dc9b

📥 Commits

Reviewing files that changed from the base of the PR and between 81d29ed and 4447258.

⛔ Files ignored due to path filters (2)
  • conf/persistence.xml is excluded by !**/*.xml
  • conf/serviceConfig/externalService.xml is excluded by !**/*.xml
📒 Files selected for processing (41)
  • conf/db/zsv/V5.1.0__schema.sql
  • core/src/main/java/org/zstack/core/externalservice/ExternalService.java
  • core/src/main/java/org/zstack/core/externalservice/ExternalServiceManagerImpl.java
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationEvent.java
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationEventDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEvent.java
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEventDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationReply.java
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationReplyDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationEvent.java
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationEventDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/ApplyExternalConfigurationResult.java
  • header/src/main/java/org/zstack/header/core/external/service/ApplyExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/ApplyExternalServiceConfigurationReply.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationInventory.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationInventoryDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationVO.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationVO_.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceInventory.java
  • header/src/main/java/org/zstack/header/core/external/service/RBACInfo.java
  • sdk/src/main/java/org/zstack/sdk/AddExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/AddExternalServiceConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/ApplyExternalConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/DeleteExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/DeleteExternalServiceConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/ExternalServiceConfigurationInventory.java
  • sdk/src/main/java/org/zstack/sdk/ExternalServiceInventory.java
  • sdk/src/main/java/org/zstack/sdk/QueryExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/QueryExternalServiceConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/SourceClassMap.java
  • sdk/src/main/java/org/zstack/sdk/UpdateExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/UpdateExternalServiceConfigurationResult.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy

Walkthrough

该 PR 在 ZStack 中引入外部服务配置管理:新增数据库模型与 JPA/Inventory 映射、REST API 消息/事件、跨管理节点的下发与回滚逻辑、SDK 客户端支持及测试辅助方法。

Changes

External Service Configuration Management

Layer / File(s) Summary
数据库模型与领域实体
conf/db/zsv/V5.1.0__schema.sql, header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfiguration*.java
新增 ExternalServiceConfigurationVO 表与 JPA 实体与元模型,Inventory 映射支持对 remote_write[].basic_auth.password 的脱敏与还原(JSON 解析与正则回退)。
API 消息与事件合约
header/src/main/java/org/zstack/header/core/external/service/API*ExternalServiceConfiguration*.java, *.Doc_zh_cn.groovy
新增 Add/Update/Delete/Query 的 API 消息、事件与 Reply,声明 REST 路由与参数约束;消息实现审计接口并提供示例数据;中文文档(Groovy DSL)补充字段与说明。
节点分发消息与应用结果
header/src/main/java/org/zstack/header/core/external/service/ApplyExternal*.java
新增 ApplyExternalServiceConfigurationMsgApplyExternalServiceConfigurationReplyApplyExternalConfigurationResult,用于跨管理节点下发配置与承载单节点应用结果。
核心服务管理与分发回滚
core/src/main/java/org/zstack/core/externalservice/ExternalService*.java, header/src/main/java/org/zstack/header/core/external/service/ExternalServiceInventory.java, RBACInfo.java
在 ExternalService 接口中新增 getServiceType()externalConfig(String) 默认方法;在 ExternalServiceManagerImpl 中实现新增/更新/删除流程,包含校验、密码解码/比较、持久化、向所有管理节点下发 ApplyExternalServiceConfigurationMsg 的链式执行、收集应用结果并在失败时回滚(恢复数据库与资源关系);ExternalServiceInventory 新增 serviceType 字段;权限名称调整。
SDK 操作与结果映射
sdk/src/main/java/org/zstack/sdk/*ExternalServiceConfiguration*.java, sdk/src/main/java/org/zstack/sdk/SourceClassMap.java
新增 Add/Query/Update/Delete Action 与对应 Result/Inventory POJO,包含参数定义、REST 配置、同步/异步调用和 ApiResult->Result 的映射;更新 SourceClassMap 的头/SDK 映射。
测试辅助方法
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
新增 addExternalServiceConfigurationqueryExternalServiceConfigurationupdateExternalServiceConfigurationdeleteExternalServiceConfiguration Groovy 辅助方法,集成 SDK Action 并支持 API 路径追踪。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 我是小兔写配置,轻敲键盘不迟疑,
密码星号藏深处,查询回写能复原,
链式下发遍节点,若遇失败轻回滚,
REST 与 SDK 一并来,外服管理始成诗。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
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 (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了PR的主要改动:添加外部服务配置API。标题简洁明了,涵盖了该变更集的核心功能。
Description check ✅ Passed 描述与变更集相关,列举了新增的主要功能模块(schema、API、SDK、testlib等)和impact标签。虽然缺乏细节,但准确反映了实际改动的范围。
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/zstackio/ZSV-12395@@2

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🧹 Nitpick comments (9)
header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsg.java (1)

43-46: 💤 Low value

可简化审计方法实现

Line 45 中将 msg 参数强制转换为 APIUpdateExternalServiceConfigurationMsg 是不必要的。由于 audit() 方法在当前类中定义,可以直接使用 this.getUuid() 或在方法签名处明确参数类型,提高代码可读性。

可选的重构方案
 `@Override`
 public Result audit(APIMessage msg, APIEvent rsp) {
-    return new APIAuditor.Result(((APIUpdateExternalServiceConfigurationMsg)msg).getUuid(), ExternalServiceConfigurationVO.class);
+    return new APIAuditor.Result(this.uuid, ExternalServiceConfigurationVO.class);
 }

注:此重构假设 audit() 被调用时 this 指向正确的消息实例。如果框架要求必须从 msg 参数中提取,则当前实现是正确的。

🤖 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
`@header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsg.java`
around lines 43 - 46, Replace the unnecessary cast inside
APIUpdateExternalServiceConfigurationMsg.audit(): instead of casting the msg
parameter to APIUpdateExternalServiceConfigurationMsg, directly use
this.getUuid() when constructing the APIAuditor.Result (or change the audit
method signature to accept APIUpdateExternalServiceConfigurationMsg if framework
allows) so the code in audit() becomes new APIAuditor.Result(this.getUuid(),
ExternalServiceConfigurationVO.class) and removes the redundant
((APIUpdateExternalServiceConfigurationMsg) msg) cast.
header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEvent.java (1)

28-32: ⚡ Quick win

建议在 example() 中填充 applyResults 字段以完善 API 文档。

当前示例方法仅设置了 success=true,但未填充 applyResults 字段。为了生成更完整的 API 文档,建议添加示例数据展示该字段的结构。

♻️ 建议的改进
 public static APIDeleteExternalServiceConfigurationEvent __example__() {
     APIDeleteExternalServiceConfigurationEvent event = new APIDeleteExternalServiceConfigurationEvent();
     event.setSuccess(true);
+    
+    ApplyExternalConfigurationResult result = new ApplyExternalConfigurationResult();
+    result.setManagementNodeUuid(uuid());
+    result.setSuccess(true);
+    event.setApplyResults(Collections.singletonList(result));
+    
     return event;
 }
🤖 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
`@header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEvent.java`
around lines 28 - 32, The __example__() in
APIDeleteExternalServiceConfigurationEvent currently only sets success=true;
update the APIDeleteExternalServiceConfigurationEvent.__example__ method to also
populate the applyResults field with a representative example (e.g., a
list/array containing one or more ApplyResult-like objects or maps showing
status, resourceId, message) so the generated API docs show the structure;
locate APIDeleteExternalServiceConfigurationEvent.__example__ and add a sample
applyResults value consistent with the ApplyResult type used elsewhere in the
codebase.
sdk/src/main/java/org/zstack/sdk/external/service/ExternalServiceConfigurationInventory.java (1)

55-61: ⚡ Quick win

建议为 List 字段添加泛型类型参数

字段 applyResults 声明为原始类型 java.util.List,缺少泛型类型参数。根据领域模型,应指定为 List<ApplyExternalConfigurationResult>,以提升类型安全性。

♻️ 建议的修复
-    public java.util.List applyResults;
-    public void setApplyResults(java.util.List applyResults) {
+    public java.util.List<ApplyExternalConfigurationResult> applyResults;
+    public void setApplyResults(java.util.List<ApplyExternalConfigurationResult> applyResults) {
         this.applyResults = applyResults;
     }
-    public java.util.List getApplyResults() {
+    public java.util.List<ApplyExternalConfigurationResult> getApplyResults() {
         return this.applyResults;
     }
🤖 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
`@sdk/src/main/java/org/zstack/sdk/external/service/ExternalServiceConfigurationInventory.java`
around lines 55 - 61, 将 ExternalServiceConfigurationInventory 中的原始类型字段
applyResults 及其访问器改为使用泛型
List<ApplyExternalConfigurationResult>:把字段声明、setApplyResults 和 getApplyResults
的参数/返回类型从 java.util.List 改为
java.util.List<ApplyExternalConfigurationResult>,并在必要时导入或使用全限定名
ApplyExternalConfigurationResult,以恢复类型安全并更新签名一致性。
sdk/src/main/java/org/zstack/sdk/external/service/QueryExternalServiceConfigurationResult.java (1)

6-12: ⚡ Quick win

建议为 List 字段添加泛型类型参数

字段 inventories 声明为原始类型 java.util.List,缺少泛型类型参数,降低了类型安全性。建议指定为 List<ExternalServiceConfigurationInventory>,以便在编译期捕获类型错误。

♻️ 建议的修复
-    public java.util.List inventories;
-    public void setInventories(java.util.List inventories) {
+    public java.util.List<ExternalServiceConfigurationInventory> inventories;
+    public void setInventories(java.util.List<ExternalServiceConfigurationInventory> inventories) {
         this.inventories = inventories;
     }
-    public java.util.List getInventories() {
+    public java.util.List<ExternalServiceConfigurationInventory> getInventories() {
         return this.inventories;
     }
🤖 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
`@sdk/src/main/java/org/zstack/sdk/external/service/QueryExternalServiceConfigurationResult.java`
around lines 6 - 12, 将原始类型的字段与访问器改为使用泛型:把
QueryExternalServiceConfigurationResult 类中的字段 inventories 及其 getter/setter
(inventories, setInventories, getInventories) 的类型从 raw java.util.List 改为
java.util.List<ExternalServiceConfigurationInventory>,并在类顶部引入或使用该类型的完整限定名;同时更新
setInventories 参数类型和 getInventories 返回类型以匹配泛型签名,确保编译通过并恢复类型安全。
sdk/src/main/java/org/zstack/sdk/external/service/GetExternalServicesResult.java (1)

6-12: ⚡ Quick win

建议为 List 字段添加泛型类型参数

字段 inventories 声明为原始类型 java.util.List,缺少泛型类型参数。应指定为 List<ExternalServiceInventory> 以提升类型安全性。

♻️ 建议的修复
-    public java.util.List inventories;
-    public void setInventories(java.util.List inventories) {
+    public java.util.List<ExternalServiceInventory> inventories;
+    public void setInventories(java.util.List<ExternalServiceInventory> inventories) {
         this.inventories = inventories;
     }
-    public java.util.List getInventories() {
+    public java.util.List<ExternalServiceInventory> getInventories() {
         return this.inventories;
     }
🤖 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
`@sdk/src/main/java/org/zstack/sdk/external/service/GetExternalServicesResult.java`
around lines 6 - 12, 字段 inventories 在类 GetExternalServicesResult 中被声明为原始类型
java.util.List,缺乏泛型。将字段类型、getter 和 setter 的签名从 java.util.List 改为
java.util.List<ExternalServiceInventory>(并导入或使用全限定名
ExternalServiceInventory),同时更新 setInventories(List<ExternalServiceInventory>
inventories) 和 getInventories() 返回
List<ExternalServiceInventory>,并修正所有引用该字段/方法的调用点以匹配泛型类型以保证类型安全。
sdk/src/main/java/org/zstack/sdk/external/service/ApplyExternalConfigurationResult.java (3)

7-29: ⚡ Quick win

建议将字段设为 private 以符合封装原则。

当前所有字段(managementNodeUuiderrorCodesuccess)均声明为 public,虽然已提供 getter/setter 方法,但这违反了 Java 的封装最佳实践。建议将字段改为 private,仅通过 getter/setter 访问。

注:如果 SDK 层框架对序列化有特殊要求(例如某些框架要求 public 字段),则可保持当前设计。

♻️ 建议的封装改进
-    public java.lang.String managementNodeUuid;
+    private java.lang.String managementNodeUuid;
     public void setManagementNodeUuid(java.lang.String managementNodeUuid) {
         this.managementNodeUuid = managementNodeUuid;
     }
     public java.lang.String getManagementNodeUuid() {
         return this.managementNodeUuid;
     }

-    public ErrorCode errorCode;
+    private ErrorCode errorCode;
     public void setErrorCode(ErrorCode errorCode) {
         this.errorCode = errorCode;
     }
     public ErrorCode getErrorCode() {
         return this.errorCode;
     }

-    public boolean success;
+    private boolean success;
     public void setSuccess(boolean success) {
         this.success = success;
     }
     public boolean getSuccess() {
         return this.success;
     }
🤖 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
`@sdk/src/main/java/org/zstack/sdk/external/service/ApplyExternalConfigurationResult.java`
around lines 7 - 29, Make the fields in ApplyExternalConfigurationResult
private: change managementNodeUuid, errorCode, and success from public to
private and keep their existing setManagementNodeUuid/getManagementNodeUuid,
setErrorCode/getErrorCode, and setSuccess/getSuccess methods as the only
accessors; if the SDK serialization layer requires public fields, add the
appropriate annotations or explicit serializer support instead of leaving fields
public.

5-5: ⚡ Quick win

建议为 SDK 类添加 Javadoc 文档。

作为面向外部开发者的 SDK 类,ApplyExternalConfigurationResult 缺少类级别和成员级别的 Javadoc 注释。建议添加文档说明类的用途、各字段的含义以及使用场景,以提升 SDK 的可用性和可维护性。

📝 建议的 Javadoc 示例
+/**
+ * 外部配置应用结果,用于封装在管理节点上应用外部服务配置的执行结果。
+ */
 public class ApplyExternalConfigurationResult  {

+    /**
+     * 管理节点 UUID
+     */
     public java.lang.String managementNodeUuid;
+    
+    /**
+     * 错误码,仅在应用失败时设置
+     */
     public ErrorCode errorCode;
+    
+    /**
+     * 应用是否成功
+     */
     public boolean success;
🤖 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
`@sdk/src/main/java/org/zstack/sdk/external/service/ApplyExternalConfigurationResult.java`
at line 5, Add Javadoc comments for the SDK class
ApplyExternalConfigurationResult: add a class-level Javadoc describing the
purpose and typical usage of ApplyExternalConfigurationResult, and add Javadoc
for each member (fields and public getters/setters) describing what each field
represents, its possible values and any nullability/constraints; ensure the
comments follow standard Javadoc format (/** ... */) and include examples or
`@see/`@since tags where helpful to guide external developers.

27-27: ⚡ Quick win

建议 boolean 类型的 getter 使用 is 前缀。

根据 Java Bean 命名规范,boolean 类型字段的 getter 方法应使用 is 前缀而非 get 前缀。建议将 getSuccess() 重命名为 isSuccess(),以符合惯例并提升代码可读性。

♻️ 建议的命名改进
-    public boolean getSuccess() {
+    public boolean isSuccess() {
         return this.success;
     }
🤖 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
`@sdk/src/main/java/org/zstack/sdk/external/service/ApplyExternalConfigurationResult.java`
at line 27, Rename the boolean getter getSuccess() in class
ApplyExternalConfigurationResult to isSuccess() to follow Java Bean conventions:
add a new method public boolean isSuccess() that returns the same field as
getSuccess(), update any usages (calls to getSuccess()) to call isSuccess(), and
optionally deprecate or remove getSuccess() after updating references; ensure
the method signature and access remain identical aside from the name so
serialization/beans tools recognize the property.
core/src/main/java/org/zstack/core/externalservice/ExternalServiceManagerImpl.java (1)

257-291: 💤 Low value

简化 syncKey 的确定逻辑。

Line 260 在 vonull 时回退到 msg.getUuid() 作为同步键,但随后在 Line 296-299 的更新逻辑中,如果 vonull 会直接失败。因此,这个回退逻辑实际上不会产生有意义的行为,反而可能造成混淆。

建议直接在 ChainTask 外部查询 vo 并提前失败,或在确定同步键时假设 vo 一定存在(因为后续逻辑已依赖此前提)。

可选的简化方案
 private void handle(APIUpdateExternalServiceConfigurationMsg msg){
     APIUpdateExternalServiceConfigurationEvent event = new APIUpdateExternalServiceConfigurationEvent(msg.getId());
     ExternalServiceConfigurationVO vo = dbf.findByUuid(msg.getUuid(), ExternalServiceConfigurationVO.class);
-    final String syncKey = vo != null ? vo.getServiceType() : msg.getUuid();
+    if (vo == null) {
+        event.setError(operr("unable to find external service configuration with uuid [%s]", msg.getUuid()));
+        bus.publish(event);
+        return;
+    }
+    final String syncKey = vo.getServiceType();
     
     thdf.chainSubmit(new ChainTask(msg) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@core/src/main/java/org/zstack/core/externalservice/ExternalServiceManagerImpl.java`
around lines 257 - 291, In handle(APIUpdateExternalServiceConfigurationMsg msg)
simplify syncKey logic by resolving ExternalServiceConfigurationVO vo before
creating the ChainTask and failing early if vo is null; compute syncKey =
vo.getServiceType() (no fallback to msg.getUuid()), and only then submit the
ChainTask that calls updateExternalServiceConfiguration(msg, event, ...). This
removes the misleading fallback, aligns with
updateExternalServiceConfiguration's expectations, and preserves the existing
ChainTask/getSyncSignature and getName usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/src/main/java/org/zstack/core/externalservice/ExternalService.java`:
- Around line 20-22: 为 ExternalService 接口中的 getServiceType() 方法添加 Javadoc
注释,说明该方法用于返回服务类型标识(service type),并解释默认实现会调用 getName()
作为返回值以保持与服务名称一致;在注释中还可指出实现类可覆盖此方法以提供不同的类型值。请在 ExternalService 接口的
getServiceType() 方法上添加这段文档,引用方法名 getServiceType() 和 getName() 以便定位。
- Around line 24-26: Add a Javadoc comment to the ExternalService interface
method externalConfig(String serviceType) that documents the method's purpose
(hook for applying external configuration for the given service type), explains
the parameter serviceType (the identifier/name of the external configuration
scope), and clarifies the default no-op implementation semantics (services that
do not support external configuration may leave the method unimplemented).
Mention that implementers should override externalConfig to apply configuration
and that the default is intentionally empty to indicate unsupported external
configuration.

In
`@header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationMsg.java`:
- Line 16: Update the REST path strings for the external service configuration
APIs to use plural resource names: change the path value from
"/external/service/configuration" to "/external/service/configurations" in
APIAddExternalServiceConfigurationMsg and make the same change in
APIUpdateExternalServiceConfigurationMsg and any other related classes that
declare the same path constant/annotation; search for the exact string
"/external/service/configuration" and replace with
"/external/service/configurations" in the annotations or path variables so all
API endpoints are consistent with the plural naming convention.
- Around line 54-57: The audit method
APIAddExternalServiceConfigurationMsg.audit currently calls
evt.getInventory().getUuid() directly which can NPE if inventory is null; modify
the success branch to guard against null by checking rsp.isSuccess() &&
evt.getInventory() != null and use evt.getInventory().getUuid() only when
non-null, otherwise return an empty string (or a safe placeholder) in the Result
for ExternalServiceConfigurationVO; update references in
APIAddExternalServiceConfigurationMsg.audit and ensure the cast to
APIAddExternalServiceConfigurationEvent (evt) remains unchanged.

In
`@header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEventDoc_zh_cn.groovy`:
- Around line 5-23: The API doc is missing the applyResults field from
APIDeleteExternalServiceConfigurationEvent; add a field block in
APIDeleteExternalServiceConfigurationEventDoc_zh_cn.groovy that documents the
applyResults property (name "applyResults"), describes it as the list of
per-configuration ApplyExternalConfigurationResult entries returned by the
operation, specifies the type as "List<ApplyExternalConfigurationResult>" (or
type "List" with a ref to ApplyExternalConfigurationResult), and include
since/version info and class reference to ApplyExternalConfigurationResult to
match how other fields (e.g., error) are documented.

In
`@header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationMsg.java`:
- Around line 15-19: Update the REST resource path strings to use plural
"configurations" instead of singular "configuration": change the `@RestRequest`
path in APIDeleteExternalServiceConfigurationMsg from
"/external/service/configuration/{uuid}" to
"/external/service/configurations/{uuid}" and make the equivalent pluralization
change in other related API message classes such as
APIQueryExternalServiceConfigurationMsg and any other classes referencing
"/external/service/configuration" so all request annotations and any consumers
use "/external/service/configurations".

In
`@header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationEventDoc_zh_cn.groovy`:
- Around line 18-23: The field block for name "success" in
APIUpdateExternalServiceConfigurationEventDoc_zh_cn.groovy has an empty desc;
update the desc value to a meaningful Chinese description (for example "操作是否成功")
to match the style used in
APIAddExternalServiceConfigurationEventDoc_zh_cn.groovy, ensuring the field {
name "success" desc "操作是否成功" type "boolean" since "5.1.0" } contains a non-empty
description.
- Around line 10-17: 将 ref 中的 inventory 描述从 desc "null" 替换为有意义的中文描述;找到
APIUpdateExternalServiceConfigurationEventDoc_zh_cn.groovy 中 ref 名为 "inventory"
的定义(path
"org.zstack.header.core.external.service.APIUpdateExternalServiceConfigurationEvent.inventory"、clz
ExternalServiceConfigurationInventory.class),将 desc 字段改为类似 "外部服务配置详情"
或其他准确中文描述以与 APIAddExternalServiceConfigurationEventDoc_zh_cn.groovy
的风格保持一致并提供可读文档。

In
`@header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsg.java`:
- Line 15: Update the REST path in APIUpdateExternalServiceConfigurationMsg to
use the plural resource name to match conventions and
APIAddExternalServiceConfigurationMsg: change the path value from
"/external/service/configuration/{uuid}" to
"/external/service/configurations/{uuid}" in the class-level annotation or field
that defines the path.

In
`@header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationInventoryDoc_zh_cn.groovy`:
- Around line 22-26: The inventory currently exposes the "configuration" field
in plain text (see ExternalServiceConfigurationInventoryDoc_zh_cn.groovy) while
the persistence entity ExternalServiceConfigurationVO stores it as a plain
String; fix by implementing encryption-at-rest for the configuration field in
the persistence layer (add AES encryption/decryption hooks in
ExternalServiceConfigurationVO's DAO/persistence methods) and ensure
API/serialization masks or omits sensitive subfields when returning
ExternalServiceConfigurationInventory (replace password-like values with fixed
mask like "******" or strip them entirely in the serializer/mapper used to
produce the inventory); update create/update code paths to encrypt before save
and decrypt only for authorized internal use, and add unit tests to verify
stored data is encrypted and API responses never return raw credentials.

In
`@header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationVO.java`:
- Around line 23-28: The configuration field in ExternalServiceConfigurationVO
currently has no column type/length and may be mapped to VARCHAR(255); update
the entity to store larger structured data by annotating the configuration field
with either `@Lob` or `@Column`(columnDefinition = "TEXT"/"MEDIUMTEXT") to avoid
truncation, and add sensible length constraints to serviceType and description
(e.g., `@Column`(length = 512)) so their columns are explicitly sized; modify the
annotations on the fields serviceType, configuration, and description in class
ExternalServiceConfigurationVO accordingly.

In
`@sdk/src/main/java/org/zstack/sdk/external/service/QueryExternalServiceConfigurationAction.java`:
- Around line 65-73: Update the REST path to use the plural form: change the
path string in QueryExternalServiceConfigurationAction.getRestInfo() from
"/external/service/configuration" to "/external/service/configurations"; also
update the `@RestRequest` annotation on APIQueryExternalServiceConfigurationMsg
(and any related message/handler classes) to use
"/external/service/configurations" and propagate the change to API
documentation/templates so all references remain consistent.

In `@testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy`:
- Around line 15436-15537: The four methods addExternalServiceConfiguration,
queryExternalServiceConfiguration, updateExternalServiceConfiguration and
deleteExternalServiceConfiguration duplicate session setup, apipath tracking and
error handling; extract that shared logic into a private helper (e.g.,
executeExternalServiceAction(action, Closure postConfig = null)) which sets
action.sessionId = Test.currentEnvSpec?.session?.uuid, applies an optional
postConfig (used by query to transform a.conditions via a.conditions =
a.conditions.collect{it.toString()}), handles apipath/apiId via ApiPathTracker
and Test.apiPaths, calls errorOut(action.call()) and returns the result;
refactor each public method to construct the specific org.zstack.sdk.*Action,
delegate the closure, then call executeExternalServiceAction with any per-action
post-processing.
- Around line 15436-15537: The four helper methods
addExternalServiceConfiguration, queryExternalServiceConfiguration,
updateExternalServiceConfiguration, and deleteExternalServiceConfiguration
reference action classes under org.zstack.sdk.* which no longer exist; update
both the `@DelegatesTo` annotations and the instantiations (new ...Action()) to
use the correct package org.zstack.sdk.external.service (e.g.
org.zstack.sdk.external.service.AddExternalServiceConfigurationAction etc.), and
also add corresponding old→new mappings for these four
*ExternalServiceConfigurationAction classes in SourceClassMap.java so
compilation won't fail with ClassNotFoundException.

---

Nitpick comments:
In
`@core/src/main/java/org/zstack/core/externalservice/ExternalServiceManagerImpl.java`:
- Around line 257-291: In handle(APIUpdateExternalServiceConfigurationMsg msg)
simplify syncKey logic by resolving ExternalServiceConfigurationVO vo before
creating the ChainTask and failing early if vo is null; compute syncKey =
vo.getServiceType() (no fallback to msg.getUuid()), and only then submit the
ChainTask that calls updateExternalServiceConfiguration(msg, event, ...). This
removes the misleading fallback, aligns with
updateExternalServiceConfiguration's expectations, and preserves the existing
ChainTask/getSyncSignature and getName usage.

In
`@header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEvent.java`:
- Around line 28-32: The __example__() in
APIDeleteExternalServiceConfigurationEvent currently only sets success=true;
update the APIDeleteExternalServiceConfigurationEvent.__example__ method to also
populate the applyResults field with a representative example (e.g., a
list/array containing one or more ApplyResult-like objects or maps showing
status, resourceId, message) so the generated API docs show the structure;
locate APIDeleteExternalServiceConfigurationEvent.__example__ and add a sample
applyResults value consistent with the ApplyResult type used elsewhere in the
codebase.

In
`@header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsg.java`:
- Around line 43-46: Replace the unnecessary cast inside
APIUpdateExternalServiceConfigurationMsg.audit(): instead of casting the msg
parameter to APIUpdateExternalServiceConfigurationMsg, directly use
this.getUuid() when constructing the APIAuditor.Result (or change the audit
method signature to accept APIUpdateExternalServiceConfigurationMsg if framework
allows) so the code in audit() becomes new APIAuditor.Result(this.getUuid(),
ExternalServiceConfigurationVO.class) and removes the redundant
((APIUpdateExternalServiceConfigurationMsg) msg) cast.

In
`@sdk/src/main/java/org/zstack/sdk/external/service/ApplyExternalConfigurationResult.java`:
- Around line 7-29: Make the fields in ApplyExternalConfigurationResult private:
change managementNodeUuid, errorCode, and success from public to private and
keep their existing setManagementNodeUuid/getManagementNodeUuid,
setErrorCode/getErrorCode, and setSuccess/getSuccess methods as the only
accessors; if the SDK serialization layer requires public fields, add the
appropriate annotations or explicit serializer support instead of leaving fields
public.
- Line 5: Add Javadoc comments for the SDK class
ApplyExternalConfigurationResult: add a class-level Javadoc describing the
purpose and typical usage of ApplyExternalConfigurationResult, and add Javadoc
for each member (fields and public getters/setters) describing what each field
represents, its possible values and any nullability/constraints; ensure the
comments follow standard Javadoc format (/** ... */) and include examples or
`@see/`@since tags where helpful to guide external developers.
- Line 27: Rename the boolean getter getSuccess() in class
ApplyExternalConfigurationResult to isSuccess() to follow Java Bean conventions:
add a new method public boolean isSuccess() that returns the same field as
getSuccess(), update any usages (calls to getSuccess()) to call isSuccess(), and
optionally deprecate or remove getSuccess() after updating references; ensure
the method signature and access remain identical aside from the name so
serialization/beans tools recognize the property.

In
`@sdk/src/main/java/org/zstack/sdk/external/service/ExternalServiceConfigurationInventory.java`:
- Around line 55-61: 将 ExternalServiceConfigurationInventory 中的原始类型字段
applyResults 及其访问器改为使用泛型
List<ApplyExternalConfigurationResult>:把字段声明、setApplyResults 和 getApplyResults
的参数/返回类型从 java.util.List 改为
java.util.List<ApplyExternalConfigurationResult>,并在必要时导入或使用全限定名
ApplyExternalConfigurationResult,以恢复类型安全并更新签名一致性。

In
`@sdk/src/main/java/org/zstack/sdk/external/service/GetExternalServicesResult.java`:
- Around line 6-12: 字段 inventories 在类 GetExternalServicesResult 中被声明为原始类型
java.util.List,缺乏泛型。将字段类型、getter 和 setter 的签名从 java.util.List 改为
java.util.List<ExternalServiceInventory>(并导入或使用全限定名
ExternalServiceInventory),同时更新 setInventories(List<ExternalServiceInventory>
inventories) 和 getInventories() 返回
List<ExternalServiceInventory>,并修正所有引用该字段/方法的调用点以匹配泛型类型以保证类型安全。

In
`@sdk/src/main/java/org/zstack/sdk/external/service/QueryExternalServiceConfigurationResult.java`:
- Around line 6-12: 将原始类型的字段与访问器改为使用泛型:把 QueryExternalServiceConfigurationResult
类中的字段 inventories 及其 getter/setter (inventories, setInventories, getInventories)
的类型从 raw java.util.List 改为
java.util.List<ExternalServiceConfigurationInventory>,并在类顶部引入或使用该类型的完整限定名;同时更新
setInventories 参数类型和 getInventories 返回类型以匹配泛型签名,确保编译通过并恢复类型安全。
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f1eb6f88-956a-4d11-8d55-b3bc94df06da

📥 Commits

Reviewing files that changed from the base of the PR and between d02b56c and 37655fe.

⛔ Files ignored due to path filters (2)
  • conf/persistence.xml is excluded by !**/*.xml
  • conf/serviceConfig/externalService.xml is excluded by !**/*.xml
📒 Files selected for processing (47)
  • conf/db/zsv/V5.1.0__schema.sql
  • core/src/main/java/org/zstack/core/externalservice/ExternalService.java
  • core/src/main/java/org/zstack/core/externalservice/ExternalServiceManagerImpl.java
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationEvent.java
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationEventDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEvent.java
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEventDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationReply.java
  • header/src/main/java/org/zstack/header/core/external/service/APIQueryExternalServiceConfigurationReplyDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationEvent.java
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationEventDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/APIUpdateExternalServiceConfigurationMsgDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/ApplyExternalConfigurationResult.java
  • header/src/main/java/org/zstack/header/core/external/service/ApplyExternalServiceConfigurationMsg.java
  • header/src/main/java/org/zstack/header/core/external/service/ApplyExternalServiceConfigurationReply.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationInventory.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationInventoryDoc_zh_cn.groovy
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationVO.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationVO_.java
  • header/src/main/java/org/zstack/header/core/external/service/ExternalServiceInventory.java
  • header/src/main/java/org/zstack/header/core/external/service/PackageInfo.java
  • sdk/src/main/java/org/zstack/sdk/ExternalServiceCapabilitiesBuilder.java
  • sdk/src/main/java/org/zstack/sdk/SourceClassMap.java
  • sdk/src/main/java/org/zstack/sdk/external/service/AddExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/external/service/AddExternalServiceConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/external/service/ApplyExternalConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/external/service/DeleteExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/external/service/DeleteExternalServiceConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/external/service/ExternalServiceCapabilities.java
  • sdk/src/main/java/org/zstack/sdk/external/service/ExternalServiceConfigurationInventory.java
  • sdk/src/main/java/org/zstack/sdk/external/service/ExternalServiceInventory.java
  • sdk/src/main/java/org/zstack/sdk/external/service/GetExternalServicesAction.java
  • sdk/src/main/java/org/zstack/sdk/external/service/GetExternalServicesResult.java
  • sdk/src/main/java/org/zstack/sdk/external/service/QueryExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/external/service/QueryExternalServiceConfigurationResult.java
  • sdk/src/main/java/org/zstack/sdk/external/service/ReloadExternalServiceAction.java
  • sdk/src/main/java/org/zstack/sdk/external/service/ReloadExternalServiceResult.java
  • sdk/src/main/java/org/zstack/sdk/external/service/UpdateExternalServiceConfigurationAction.java
  • sdk/src/main/java/org/zstack/sdk/external/service/UpdateExternalServiceConfigurationResult.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy

Comment on lines +20 to +22
default String getServiceType() {
return getName();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

为接口方法添加 Javadoc 注释。

根据编码规范,接口方法必须配有有效的 Javadoc 注释。请为 getServiceType() 添加文档说明,解释其用途以及默认实现返回 getName() 的行为。

建议的文档格式
+/**
+ * 获取外部服务的类型标识。
+ * 默认实现返回服务名称,子类可根据需要覆盖此方法。
+ *
+ * `@return` 服务类型
+ */
 default String getServiceType() {
     return getName();
 }
📝 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
default String getServiceType() {
return getName();
}
/**
* 获取外部服务的类型标识
* 默认实现返回服务名称子类可根据需要覆盖此方法
*
* `@return` 服务类型
*/
default String getServiceType() {
return getName();
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/main/java/org/zstack/core/externalservice/ExternalService.java`
around lines 20 - 22, 为 ExternalService 接口中的 getServiceType() 方法添加 Javadoc
注释,说明该方法用于返回服务类型标识(service type),并解释默认实现会调用 getName()
作为返回值以保持与服务名称一致;在注释中还可指出实现类可覆盖此方法以提供不同的类型值。请在 ExternalService 接口的
getServiceType() 方法上添加这段文档,引用方法名 getServiceType() 和 getName() 以便定位。

Source: Coding guidelines

Comment on lines +24 to +26
default void externalConfig(String serviceType) {
// no-op by default
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

为接口方法添加 Javadoc 注释。

根据编码规范,接口方法必须配有有效的 Javadoc 注释。请为 externalConfig(String serviceType) 添加文档,说明其用途、参数含义以及默认空实现的语义(即不支持外部配置的服务类型可保持空实现)。

建议的文档格式
+/**
+ * 应用外部服务配置。
+ * 默认实现为空操作。不支持外部配置的服务类型可保持此默认行为。
+ *
+ * `@param` serviceType 服务类型
+ */
 default void externalConfig(String serviceType) {
     // no-op by default
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/main/java/org/zstack/core/externalservice/ExternalService.java`
around lines 24 - 26, Add a Javadoc comment to the ExternalService interface
method externalConfig(String serviceType) that documents the method's purpose
(hook for applying external configuration for the given service type), explains
the parameter serviceType (the identifier/name of the external configuration
scope), and clarifies the default no-op implementation semantics (services that
do not support external configuration may leave the method unimplemented).
Mention that implementers should override externalConfig to apply configuration
and that the default is intentionally empty to indicate unsupported external
configuration.

Sources: Coding guidelines, Learnings

Comment on lines +54 to +57
public Result audit(APIMessage msg, APIEvent rsp) {
APIAddExternalServiceConfigurationEvent evt = (APIAddExternalServiceConfigurationEvent) rsp;
return new Result(rsp.isSuccess() ? evt.getInventory().getUuid(): "", ExternalServiceConfigurationVO.class);
}

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 56 在成功时直接调用 evt.getInventory().getUuid(),如果 inventorynull 将导致 NullPointerException。虽然从上下文代码来看,inventory 应该在成功时已经设置,但添加防御性检查可以提高代码的健壮性。

建议的防御性修正
 `@Override`
 public Result audit(APIMessage msg, APIEvent rsp) {
     APIAddExternalServiceConfigurationEvent evt = (APIAddExternalServiceConfigurationEvent) rsp;
-    return new Result(rsp.isSuccess() ? evt.getInventory().getUuid(): "", ExternalServiceConfigurationVO.class);
+    String auditUuid = "";
+    if (rsp.isSuccess() && evt.getInventory() != null) {
+        auditUuid = evt.getInventory().getUuid();
+    }
+    return new Result(auditUuid, ExternalServiceConfigurationVO.class);
 }
📝 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
public Result audit(APIMessage msg, APIEvent rsp) {
APIAddExternalServiceConfigurationEvent evt = (APIAddExternalServiceConfigurationEvent) rsp;
return new Result(rsp.isSuccess() ? evt.getInventory().getUuid(): "", ExternalServiceConfigurationVO.class);
}
public Result audit(APIMessage msg, APIEvent rsp) {
APIAddExternalServiceConfigurationEvent evt = (APIAddExternalServiceConfigurationEvent) rsp;
String auditUuid = "";
if (rsp.isSuccess() && evt.getInventory() != null) {
auditUuid = evt.getInventory().getUuid();
}
return new Result(auditUuid, ExternalServiceConfigurationVO.class);
}
🤖 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
`@header/src/main/java/org/zstack/header/core/external/service/APIAddExternalServiceConfigurationMsg.java`
around lines 54 - 57, The audit method
APIAddExternalServiceConfigurationMsg.audit currently calls
evt.getInventory().getUuid() directly which can NPE if inventory is null; modify
the success branch to guard against null by checking rsp.isSuccess() &&
evt.getInventory() != null and use evt.getInventory().getUuid() only when
non-null, otherwise return an empty string (or a safe placeholder) in the Result
for ExternalServiceConfigurationVO; update references in
APIAddExternalServiceConfigurationMsg.audit and ensure the cast to
APIAddExternalServiceConfigurationEvent (evt) remains unchanged.

Comment on lines +5 to +23
doc {

title "删除外部服务配置返回"

field {
name "success"
desc "操作是否成功"
type "boolean"
since "5.1.0"
}
ref {
name "error"
path "org.zstack.header.core.external.service.APIDeleteExternalServiceConfigurationEvent.error"
desc "错误码,若不为null,则表示操作失败, 操作成功时该字段为null",false
type "ErrorCode"
since "5.1.0"
clz ErrorCode.class
}
}

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

缺少 applyResults 字段的文档定义。

APIDeleteExternalServiceConfigurationEvent 类中包含 applyResults 字段(类型为 List<ApplyExternalConfigurationResult>),但当前文档中未对其进行说明。这将导致 API 文档不完整。

📝 建议添加缺失的字段文档
 	ref {
 		name "error"
 		path "org.zstack.header.core.external.service.APIDeleteExternalServiceConfigurationEvent.error"
 		desc "错误码,若不为null,则表示操作失败, 操作成功时该字段为null",false
 		type "ErrorCode"
 		since "5.1.0"
 		clz ErrorCode.class
 	}
+	ref {
+		name "applyResults"
+		path "org.zstack.header.core.external.service.APIDeleteExternalServiceConfigurationEvent.applyResults"
+		desc "各管理节点应用配置的结果列表"
+		type "List"
+		since "5.1.0"
+		clz ApplyExternalConfigurationResult.class
+	}
 }
🤖 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
`@header/src/main/java/org/zstack/header/core/external/service/APIDeleteExternalServiceConfigurationEventDoc_zh_cn.groovy`
around lines 5 - 23, The API doc is missing the applyResults field from
APIDeleteExternalServiceConfigurationEvent; add a field block in
APIDeleteExternalServiceConfigurationEventDoc_zh_cn.groovy that documents the
applyResults property (name "applyResults"), describes it as the list of
per-configuration ApplyExternalConfigurationResult entries returned by the
operation, specifies the type as "List<ApplyExternalConfigurationResult>" (or
type "List" with a ref to ApplyExternalConfigurationResult), and include
since/version info and class reference to ApplyExternalConfigurationResult to
match how other fields (e.g., error) are documented.

Comment on lines +22 to +26
name "configuration"
desc "外部服务配置, 使用 json 格式。该字段按原文返回已保存配置;若包含 remote_write.basic_auth.password 等凭据,查询结果会返回明文,请仅通过管理员级接口使用"
type "String"
since "5.1.0"
}

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 | 🔴 Critical | 🏗️ Heavy lift

严重安全隐患:明文返回敏感凭据

文档明确说明配置字段可能包含密码等敏感凭据,并且查询结果会以明文形式返回。这构成严重的安全漏洞:

  1. 明文暴露风险:密码以明文形式通过 API 返回,可能被日志记录、缓存或在传输过程中被拦截
  2. 存储安全:查看数据库 schema(conf/db/zsv/V5.1.0__schema.sql 第 141 行)和实体类(ExternalServiceConfigurationVO.java),configuration 字段为普通 text/String 类型,未见加密处理
  3. 不足的防护:仅依赖"管理员级接口"限制访问不足以保护敏感凭据,即使是管理员日志、审计记录、浏览器缓存等也会暴露明文密码

建议修复方案

  • 对敏感凭据字段进行加密存储(如使用 AES 加密)
  • API 返回时对密码类字段进行脱敏处理(返回 ****** 或完全排除)
  • 或者提供单独的安全凭据管理机制,将敏感字段与配置分离
🤖 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
`@header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationInventoryDoc_zh_cn.groovy`
around lines 22 - 26, The inventory currently exposes the "configuration" field
in plain text (see ExternalServiceConfigurationInventoryDoc_zh_cn.groovy) while
the persistence entity ExternalServiceConfigurationVO stores it as a plain
String; fix by implementing encryption-at-rest for the configuration field in
the persistence layer (add AES encryption/decryption hooks in
ExternalServiceConfigurationVO's DAO/persistence methods) and ensure
API/serialization masks or omits sensitive subfields when returning
ExternalServiceConfigurationInventory (replace password-like values with fixed
mask like "******" or strip them entirely in the serializer/mapper used to
produce the inventory); update create/update code paths to encrypt before save
and decrypt only for authorized internal use, and add unit tests to verify
stored data is encrypted and API responses never return raw credentials.

Comment on lines +23 to +28
@Column
private String serviceType;
@Column
private String configuration;
@Column
private String description;

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

configuration 字段指定适当的列类型定义。

configuration 字段未指定长度约束或列类型,默认情况下 JPA 提供者可能将其映射为 VARCHAR(255)。如果该字段用于存储 JSON、XML 或其他结构化配置数据(这在外部服务配置场景中很常见),则可能导致数据截断。

建议:

  • 如果配置数据可能较大,使用 @Lob 注解或明确指定 columnDefinition = "TEXT""MEDIUMTEXT"
  • serviceTypedescription 字段也建议添加合理的长度约束,例如 @Column(length = 512)
建议的修复
 `@Column`
 private String serviceType;
-@Column
+@Column(columnDefinition = "TEXT")
 private String configuration;
 `@Column`
 private String description;

或使用 @Lob

-@Column
+@Lob
+@Column
 private String configuration;
🤖 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
`@header/src/main/java/org/zstack/header/core/external/service/ExternalServiceConfigurationVO.java`
around lines 23 - 28, The configuration field in ExternalServiceConfigurationVO
currently has no column type/length and may be mapped to VARCHAR(255); update
the entity to store larger structured data by annotating the configuration field
with either `@Lob` or `@Column`(columnDefinition = "TEXT"/"MEDIUMTEXT") to avoid
truncation, and add sensible length constraints to serviceType and description
(e.g., `@Column`(length = 512)) so their columns are explicitly sized; modify the
annotations on the fields serviceType, configuration, and description in class
ExternalServiceConfigurationVO accordingly.

Comment thread testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
@MatheMatrix MatheMatrix force-pushed the sync/zstackio/ZSV-12395@@2 branch from 37655fe to 869020c Compare June 9, 2026 05:39
@zstack-robot-2

Copy link
Copy Markdown
Collaborator Author

Comment from yaohua.wu:

Review: MR !10157 — ZSV-12395

Background

  • Jira: ZSV-12395 — Zcf需要ZSV提供API配置Prometheus remote-write(Improvement, P2)
  • 需求: 为 ZCF 提供 API,配置外部服务(Prometheus2)的 remote_write。
  • 本 MR 职责(core 侧): 新增 ExternalServiceConfigurationVO/Inventory、Add/Update/Delete/Query API、跨管理节点 apply 框架(ApplyExternalServiceConfigurationMsg + ChainTask 串行 + 失败回滚)、ExternalService.getServiceType/externalConfig 扩展点、SDK 与 testlib helper。
  • 关联 premium MR: !14277 实现 Prometheus2 的 externalConfig 与 remote_write 解析/生成。
  • 状态: 本 MR 当前为 Draft

关联 MR

MR 仓库 作用
!14277 zstackio/premium Prometheus2 侧实现:消费本 MR 的 externalConfig 钩子与 ExternalServiceConfigurationVO,解析 remote_write / basic_auth / relabel 并生成 prometheus 配置

两个 MR 共用源分支 ZSV-12395@@2,必须一起合入。

🔴 Critical

# File:Line 问题 修复
1 header/src/main/java/org/zstack/header/core/external/service/PackageInfo.java:5 新增 @SDKPackage(packageName="org.zstack.sdk.external.service") 把本包所有生成的 SDK 类(含已存在的 ExternalServiceInventory)从 org.zstack.sdk 迁到 org.zstack.sdk.external.service。已核实:target zsv_5.1.0sdk/.../org/zstack/sdk/ExternalServiceInventory.java(package org.zstack.sdk),本分支该文件已删除,只剩 external/service/ 下;org/zstack/sdk/AddExternalServiceConfigurationAction.java 也不存在。但本 MR 的 testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(~L15436-15540 的 4 个 helper)用 new org.zstack.sdk.AddExternalServiceConfigurationAction()(及 Query/Update/Delete)引用 default 包testlib 编译失败;关联 premium 测试同样引用 default 包 → 编译失败;且把既有 org.zstack.sdk.ExternalServiceInventory 搬走是对已发布 SDK 的破坏性变更,会波及其它引用方。 删除 PackageInfo.java(去掉 @SDKPackage),重新生成 SDK 回到 default org.zstack.sdk,使 testlib + premium 所有引用对齐,并保持 ExternalServiceInventory 原位。如确需新包名,则同步改 ApiHelper.groovy 与 premium 两处 import 到 org.zstack.sdk.external.service.*(不推荐,破坏性更大)。

🟡 Warning

# File:Line 问题 修复
1 APIQueryExternalServiceConfigurationMsg.java / APIAddExternalServiceConfigurationMsg.java:~24 / conf/db/zsv/V5.1.0__schema.sql configuration 可含 remote_write.basic_auth.password,以 text 明文入库、Query API 原文回显、且作为 Add 的 @APIParam 会进 API 操作/审计日志明文。代码层无脱敏、无管理员限定(仅 doc 注明"请通过管理员级接口",未强制)。若非管理员账号可查询即造成凭据泄露。 Query API 限管理员;查询结果对 password 脱敏(或密码单独加密存储);Add 的 configuration@NoLogging
2 core/.../ExternalServiceManagerImpl.java deleteExternalServiceConfiguration / rollbackDeletedExternalServiceConfiguration dbf.remove(vo) 再在 apply 失败时 dbf.persist(copy)ResourceVO 的 remove 会级联删除 AccountResourceRefVO/标签等,回滚只重建主行、不恢复这些关联,失败回滚后配置可能成为孤儿资源(账号映射丢失)。 改为"先 apply 成功再 remove",或回滚走正规 create 路径恢复账号映射/标签。
3 APIUpdateExternalServiceConfigurationMsg.java Update 只有 uuid+descriptionupdateExternalServiceConfiguration 不改 configuration 也不 re-apply。要改 remote-write 端点/凭据只能 delete+add(uuid 变化、期间监控 remote-write 中断)。与"配置 remote-write"的诉求相比是功能缺口。 确认是否有意;如需在线编辑配置,补 configuration 字段并在更新后 re-apply。

🟢 Suggestion

# File:Line 问题
1 core/.../ExternalServiceManagerImpl.java regenerateExternalServiceConfiguration 生产路径判断 CoreGlobalProperty.UNIT_TEST_ON 返回成功,属测试捷径混入生产代码,建议测试里注册 stub service 替代该分支。
2 core/.../ExternalServiceManagerImpl.java applyExternalServiceConfigurationToAllNodes apply 为全节点 all-or-nothing:任一管理节点失败即回滚整个 create/delete,单节点抖动会让整个 API 失败,确认是否可接受(可考虑容忍部分失败 + 异步重试)。

✅ 做得好

  • VO 唯一约束双保险(JPA @UniqueConstraint + SQL UNIQUE KEY),并发 create 由 ChainTask 按 serviceType 串行化。
  • createDate 用 @PrePersist 兜底,测试断言 !startsWith("1999") 防住历史"落库 NULL → 1999"问题。
  • create/delete 失败的 DB 回滚 + 跨节点 re-apply 形成闭环。
  • ApplyExternalConfigurationResult.setErrorCode 自动置 success=false,避免漏标。

Coverage

  • 文件:~49(含生成的 SDK/Doc)。深读 core 逻辑(ExternalService/Manager/VO/Inventory/4×API/Apply 消息/schema/testlib),并跨仓核对了 premium 侧消费点与 SDK 实际生成包路径。
  • 跨仓一致性:接口签名(externalConfig(String)/getServiceType())、Inventory 字段、SDK 字段(externalServiceTypeapplyResults)均匹配;唯一阻断点为 SDK 包路径(Critical Test1 #1)。

Verdict: REVISION_REQUIRED

1 × Critical(@SDKPackage 包路径错配,阻断 testlib + premium 编译)+ 3 × Warning。框架设计整体合理,修复 Critical 与鉴权/脱敏后可推进(注意当前为 Draft)。


🤖 Robot Reviewer

@MatheMatrix MatheMatrix force-pushed the sync/zstackio/ZSV-12395@@2 branch 4 times, most recently from 92ccca1 to 81d29ed Compare June 9, 2026 07:43
- Add ExternalServiceConfigurationVO schema, inventory, query API, and add/update/delete APIs
- Apply configuration changes to matching external services across management nodes
- Expose serviceType in external service inventory for configuration routing
- Generate SDK actions/results and testlib helpers for external service configuration operations
- Register persistence and external service API metadata

DBImpact
APIImpact

Resolves: ZSV-12395

Change-Id: I6a6b76626770736871716563676b6a7a74736966
@MatheMatrix MatheMatrix force-pushed the sync/zstackio/ZSV-12395@@2 branch from 81d29ed to 4447258 Compare June 9, 2026 08:17
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