<feature>[kms]: support kms trust API#3397
Open
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Open
<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 操作（证书/身份上传与检索）；从 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
from
7c40c8e to
10bddcc
Compare
Resolves: ZSV-11331 Change-Id: I63646d7974756278777565696276797066796f68
MatheMatrix
force-pushed
the
sync/tao.yang/ZSV-11331@@2
branch
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
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
1 participant
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