<feature>[kms]: support kms trust API#3397
Closed
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Closed
<feature>[kms]: support kms trust API#3397zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Conversation
MatheMatrix
force-pushed
the
sync/tao.yang/ZSV-11331@@2
branch
from
3c6fa7d to
7c40c8e
Compare
Walkthrough新增多项 KMS 相关 SDK API 操作（证书/客户端 CSR/身份的上传与服务器证书检索）；移除 CreateKmsAction 与 CreateNkpAction 中的公共字段 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client"
participant Action as "SDK Action\n(e.g. Upload/Get Action)"
participant ZSClient as "ZSClient"
participant KMS as "KMS Backend"
participant Result as "Result Mapper"
Client->>Action: 设置参数并调用 call()/call(async)
Action->>ZSClient: 发送 REST 请求 (PUT /key-providers/kms/{uuid}/actions)
ZSClient->>KMS: 转发请求并等待响应
KMS-->>ZSClient: 返回 ApiResult
ZSClient->>Result: 将 ApiResult 转换为强类型 Result
Result-->>Action: 返回 Result（value 或 error）
Action-->>Client: 返回或抛出异常
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.java (1)
31-35: 建议使用泛型参数化 List 类型。
systemTags和userTags使用了原始类型java.util.List，建议使用参数化类型List<String>以提高类型安全性并避免编译器警告。不过考虑到这可能是自动生成的 SDK 代码且需要与代码库中其他 KMS action 保持一致，此建议可延后处理。
♻️ 建议的改进
`@Param`(required = false) -public java.util.List systemTags; +public java.util.List<String> systemTags; `@Param`(required = false) -public java.util.List userTags; +public java.util.List<String> userTags;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.java` around lines 31 - 35, Change the raw java.util.List fields systemTags and userTags in class GetKmsServerCertFromKmsAction to use a parameterized type (List<String>) to improve type safety and remove compiler warnings; update the field declarations for systemTags and userTags to use java.util.List<String> (and add/import java.util.List if missing) while keeping the `@Param` annotations and the rest of the class unchanged so it stays consistent with other KMS action classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.java`:
- Around line 31-35: Change the raw java.util.List fields systemTags and
userTags in class GetKmsServerCertFromKmsAction to use a parameterized type
(List<String>) to improve type safety and remove compiler warnings; update the
field declarations for systemTags and userTags to use java.util.List<String>
(and add/import java.util.List if missing) while keeping the `@Param` annotations
and the rest of the class unchanged so it stays consistent with other KMS action
classes.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/CreateKmsAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/CreateNkpAction.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
💤 Files with no reviewable changes (2)
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/CreateKmsAction.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/CreateNkpAction.java
MatheMatrix
force-pushed
the
sync/tao.yang/ZSV-11331@@2
branch
2 times, most recently
from
10bddcc to
ed54bda
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsResult.java (1)
5-22: DTO 结构基本正确，但字段可见性可考虑优化。该类作为 KMS 服务证书查询结果的数据传输对象，结构清晰，命名规范符合编码指南。
如果这不是自动生成的代码，建议将
public字段改为private，以保持封装性的一致性（既然已经提供了 getter/setter）。♻️ 可选：将字段改为 private 以增强封装性
public class GetKmsServerCertFromKmsResult { - public java.lang.String serverCertPem; + private java.lang.String serverCertPem; public void setServerCertPem(java.lang.String serverCertPem) { this.serverCertPem = serverCertPem; } public java.lang.String getServerCertPem() { return this.serverCertPem; } - public java.sql.Timestamp serverCertExpiredDate; + private java.sql.Timestamp serverCertExpiredDate; public void setServerCertExpiredDate(java.sql.Timestamp serverCertExpiredDate) { this.serverCertExpiredDate = serverCertExpiredDate; } public java.sql.Timestamp getServerCertExpiredDate() { return this.serverCertExpiredDate; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsResult.java` around lines 5 - 22, Change the two public DTO fields in GetKmsServerCertFromKmsResult (serverCertPem and serverCertExpiredDate) to private while keeping the existing setServerCertPem/getServerCertPem and setServerCertExpiredDate/getServerCertExpiredDate methods unchanged; this ensures proper encapsulation for the class GetKmsServerCertFromKmsResult without altering the external API.sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrAction.java (1)
37-41: 考虑为 List 添加泛型类型参数（可选改进）
systemTags和userTags使用了原始类型java.util.List，建议添加泛型参数以提升类型安全性，例如java.util.List<String>。不过，这可能是 SDK 代码生成器的既定模式，若与其他 Action 类保持一致则可忽略此建议。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrAction.java` around lines 37 - 41, Update the raw List fields in UploadKmsClientCsrAction to use a generic type for type safety: change the fields systemTags and userTags from java.util.List to java.util.List<String> (update their declarations and any related getters/setters or usages inside UploadKmsClientCsrAction so types remain consistent, e.g., references to systemTags and userTags and any methods that accept/return them).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsResult.java`:
- Around line 5-22: Change the two public DTO fields in
GetKmsServerCertFromKmsResult (serverCertPem and serverCertExpiredDate) to
private while keeping the existing setServerCertPem/getServerCertPem and
setServerCertExpiredDate/getServerCertExpiredDate methods unchanged; this
ensures proper encapsulation for the class GetKmsServerCertFromKmsResult without
altering the external API.
In
`@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrAction.java`:
- Around line 37-41: Update the raw List fields in UploadKmsClientCsrAction to
use a generic type for type safety: change the fields systemTags and userTags
from java.util.List to java.util.List<String> (update their declarations and any
related getters/setters or usages inside UploadKmsClientCsrAction so types
remain consistent, e.g., references to systemTags and userTags and any methods
that accept/return them).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
sdk/src/main/java/org/zstack/sdk/NkpRestoreInfo.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/CreateKmsAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/CreateNkpAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/ParseNkpRestoreResult.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
💤 Files with no reviewable changes (2)
- sdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/CreateNkpAction.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/CreateKmsAction.java
🚧 Files skipped from review as they are similar to previous changes (5)
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertAction.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertResult.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityResult.java
Resolves: ZSV-11331 Change-Id: I63646d7974756278777565696276797066796f68
MatheMatrix
force-pushed
the
sync/tao.yang/ZSV-11331@@2
branch
from
ed54bda to
fa473ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.java (1)
40-44: 此 Action 类由代码生成器自动生成，手动修改会被覆盖Lines [40]-[44] 中的
systemTags和userTags使用了原始java.util.List，这是 SDK 所有 Action 类的统一代码生成模式。该文件由SdkApiTemplate.groovy自动生成，手动修改此处会在代码重新生成时被覆盖。若需统一添加泛型参数List<String>，应在模板生成器中修改，而非在个别生成的 Action 文件中修改。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.java` around lines 40 - 44, The generated Action uses raw java.util.List for the fields systemTags and userTags in UploadKmsClientIdentityAction; update the code generator template SdkApiTemplate.groovy to emit generic List<String> for these fields (replace raw List emission logic for tag fields with List<String>) so all generated Action classes produce typed lists instead of raw types, then regenerate the SDK so UploadKmsClientIdentityAction and other Actions reflect the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/src/main/java/org/zstack/sdk/KmsInventory.java`:
- Around line 56-70: Keep backward compatibility by adding deprecated
transitional accessors that delegate to the new fields: add `@Deprecated` public
String getCertPem() / setCertPem(String) which read/write serverCertPem, and
`@Deprecated` public CertificateInfo getCertInfo() / setCertInfo(CertificateInfo)
which read/write serverCertInfo; mark each with `@Deprecated` and a javadoc
explaining they delegate to serverCertPem/serverCertInfo and will be removed in
a future release.
---
Nitpick comments:
In
`@sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.java`:
- Around line 40-44: The generated Action uses raw java.util.List for the fields
systemTags and userTags in UploadKmsClientIdentityAction; update the code
generator template SdkApiTemplate.groovy to emit generic List<String> for these
fields (replace raw List emission logic for tag fields with List<String>) so all
generated Action classes produce typed lists instead of raw types, then
regenerate the SDK so UploadKmsClientIdentityAction and other Actions reflect
the change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
sdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CertificateInfo.javasdk/src/main/java/org/zstack/sdk/KmsInventory.javasdk/src/main/java/org/zstack/sdk/NkpRestoreInfo.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/CreateKmsAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/GetKmsServerCertFromKmsResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsServerCertResult.javasdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/CreateNkpAction.javasdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/ParseNkpRestoreResult.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
💤 Files with no reviewable changes (2)
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/CreateKmsAction.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/CreateNkpAction.java
🚧 Files skipped from review as they are similar to previous changes (4)
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientIdentityResult.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientCsrResult.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/kms/api/UploadKmsClientSignedCertResult.java
- sdk/src/main/java/org/zstack/sdk/keyprovider/nkp/api/ParseNkpRestoreResult.java
Comment on lines
+56
to
70
| public java.lang.String serverCertPem; | ||
| public void setServerCertPem(java.lang.String serverCertPem) { | ||
| this.serverCertPem = serverCertPem; | ||
| } | ||
| public java.sql.Timestamp getServerCertExpiredDate() { | ||
| return this.serverCertExpiredDate; | ||
| public java.lang.String getServerCertPem() { | ||
| return this.serverCertPem; | ||
| } | ||
|
|
||
| public CertificateInfo serverCertInfo; | ||
| public void setServerCertInfo(CertificateInfo serverCertInfo) { | ||
| this.serverCertInfo = serverCertInfo; | ||
| } | ||
| public CertificateInfo getServerCertInfo() { | ||
| return this.serverCertInfo; | ||
| } |
There was a problem hiding this comment.
替换证书字段会引入 SDK 向后兼容风险。
这里将旧的过期时间表达迁移为 serverCertPem/serverCertInfo 后，历史 SDK 调用方（尤其依赖旧 accessor 的调用）会出现编译或行为不兼容。建议至少保留一版过渡兼容访问器并标记 @Deprecated。
🔧 建议的兼容层补丁（过渡期）
+ /**
+ * `@deprecated` 请改用 getServerCertInfo().getExpiredDate()
+ */
+ `@Deprecated`
+ public java.sql.Timestamp getServerCertExpiredDate() {
+ return this.serverCertInfo == null ? null : this.serverCertInfo.getExpiredDate();
+ }
+
+ /**
+ * `@deprecated` 请改用 setServerCertInfo()
+ */
+ `@Deprecated`
+ public void setServerCertExpiredDate(java.sql.Timestamp serverCertExpiredDate) {
+ if (this.serverCertInfo == null) {
+ this.serverCertInfo = new CertificateInfo();
+ }
+ this.serverCertInfo.setExpiredDate(serverCertExpiredDate);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/main/java/org/zstack/sdk/KmsInventory.java` around lines 56 - 70,
Keep backward compatibility by adding deprecated transitional accessors that
delegate to the new fields: add `@Deprecated` public String getCertPem() /
setCertPem(String) which read/write serverCertPem, and `@Deprecated` public
CertificateInfo getCertInfo() / setCertInfo(CertificateInfo) which read/write
serverCertInfo; mark each with `@Deprecated` and a javadoc explaining they
delegate to serverCertPem/serverCertInfo and will be removed in a future
release.
MatheMatrix
reviewed
Mar 4, 2026
|
|
||
| public class GetKmsServerCertFromKmsAction extends AbstractAction { | ||
|
|
||
| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); |
Owner
There was a problem hiding this comment.
Comment from zhijian.liu:
这里多个非必要的空格，定义变量直接函数内部第一行，相同的变量之间也没必要空格
Collaborator
There was a problem hiding this comment.
Comment from tao.yang:
sdk脚本自动生成的内容
| return this.commonName; | ||
| } | ||
|
|
||
| public java.util.List subjectAltNamesDns; |
Collaborator
There was a problem hiding this comment.
Comment from zhijian.liu:
最好能够指定泛型
Collaborator
There was a problem hiding this comment.
Comment from tao.yang:
sdk脚本自动生成的内容
Collaborator
Author
|
Comment on Comment from zhijian.liu: 这个soltpolicy是providerName是用来干什么的，目前看很多参数都没什么用 |
Owner
|
Comment on Comment from tao.yang: 保留参数 |
Collaborator
|
Comment from gitlab: 检测到REVIEWED标签添加者(zhijian.liu)不属于Maintainers:[yuerong.su xin.zhang lock-files ye.zou shixin.ruan wenhao.zhang zstackio gitlab jin.ma zhangjianjun qun.li wei.wang ye.tian lei.liu yaohua.wu], 所以移除了REVIEWED标签 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: ZSV-11331
Change-Id: I63646d7974756278777565696276797066796f68
sync from gitlab !9245