<fix>[crypto]: secret get in vm pre instantiate#3685
Open
ZStack-Robot wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Open
<fix>[crypto]: secret get in vm pre instantiate#3685ZStack-Robot wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
ZStack-Robot wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Conversation
WalkthroughPR 在 TPM 加密资源密钥系统中引入了密钥版本号（keyVersion）支持，新增接口方法、消息类型、KVM agent/host 命令与处理逻辑，并在 TPM 扩展与测试模拟器中将 providerName 替换为 keyVersion 及相关流程调整。 Changes
Sequence DiagramsequenceDiagram
participant Client as VM/Requester
participant KVMHost as KVM Host
participant Agent as KVM Agent
participant Cache as Secret Cache
Client->>KVMHost: SecretHostDefineMsg(hostUuid, keyVersion)
activate KVMHost
KVMHost->>Agent: POST SecretHostDefineCmd(keyVersion)
activate Agent
Agent->>Cache: Store secret by cacheKey(vmUuid, purpose, keyVersion)
Agent-->>KVMHost: SecretHostDefineResponse(success / error)
deactivate Agent
KVMHost-->>Client: Reply
deactivate KVMHost
Client->>KVMHost: SecretHostGetMsg(hostUuid, vmUuid, purpose, keyVersion)
activate KVMHost
KVMHost->>Agent: POST SecretHostGetCmd(vmUuid, purpose, keyVersion)
activate Agent
Agent->>Cache: Query by cacheKey(vmUuid, purpose, keyVersion)
alt Secret Found
Agent-->>KVMHost: SecretHostGetResponse(secretUuid)
else Secret Not Found
Agent-->>KVMHost: Error(KEY_AGENT_SECRET_NOT_FOUND)
end
deactivate Agent
KVMHost-->>Client: SecretHostGetReply(secretUuid or error)
deactivate KVMHost
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 Generate unit tests (beta)
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.0)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java (3)
371-376: 冗余的providerUuid加载逻辑在调用
loadResourceKeyRefContext(context)之前（第 144 行），context.providerUuid已经被设置为resourceKeyBackend.findKeyProviderUuidByTpm(context.tpmUuid)的结果。因此第 372-374 行的空值检查和重新加载是多余的。♻️ 建议简化
private void loadResourceKeyRefContext(PrepareTpmResourceContext context) { - if (StringUtils.isBlank(context.providerUuid)) { - context.providerUuid = resourceKeyBackend.findKeyProviderUuidByTpm(context.tpmUuid); - } context.keyVersion = resourceKeyBackend.findKeyVersionByTpm(context.tpmUuid); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around lines 371 - 376, The method loadResourceKeyRefContext currently re-fetches providerUuid redundantly: remove the conditional block that checks and sets context.providerUuid (the StringUtils.isBlank(...) branch) because providerUuid is already set before this call; keep only the assignment of context.keyVersion = resourceKeyBackend.findKeyVersionByTpm(context.tpmUuid) so loadResourceKeyRefContext(PrepareTpmResourceContext) only populates keyVersion.
261-296: 建议：考虑合并或澄清重复的getOrCreateKey调用
load-dek-for-host流程与ensure-resource-key-ref流程都调用了resourceKeyManager.getOrCreateKey()，逻辑相似。虽然通过 skip 条件避免了重复执行，但代码结构可能让维护者困惑。当前设计意图：
ensure-resource-key-ref：首次创建/获取 key（当 keyVersion 为空时）load-dek-for-host：当 host 上没有找到 secret 且缺少 dekBase64 时重新加载建议添加注释说明两个流程的分工，或考虑提取共享逻辑到私有方法中以减少重复。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around lines 261 - 296, The two flows "load-dek-for-host" and "ensure-resource-key-ref" both call resourceKeyManager.getOrCreateKey(...) with very similar GetOrCreateResourceKeyContext setup, which is confusing and duplicates logic; refactor by extracting the shared get-or-create-key logic into a single private helper (e.g., a private method that builds GetOrCreateResourceKeyContext, invokes resourceKeyManager.getOrCreateKey and populates tpmSpec/context fields) and call that helper from both flows while preserving the distinct skip conditions and post-success behaviors, or alternatively add concise comments above the run() implementations of load-dek-for-host and ensure-resource-key-ref explaining their different responsibilities (first: create/get key when keyVersion is empty; second: reload dek for host when secret/dek missing) so maintainers understand why both calls exist.
249-255: 简化错误码检查逻辑，仅检查getCode()当前代码同时检查
errorCode.getCode()和errorCode.getDetails()。但在buildSecretAgentError()方法中（KVMHost.java 第 5468-5476 行），这两个字段被设置为相同的值：err.setCode(rsp.getError()); err.setDetails(rsp.getError());因此对
getDetails()的检查是多余的。建议仅检查getCode()：if (errorCode != null && SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode())) { trigger.next(); return; }这样可以遵循标准错误码检查模式，提高代码可读性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around lines 249 - 255, The error-checking branch in KvmTpmExtensions (where reply.getError() is assigned to errorCode) unnecessarily inspects errorCode.getDetails(); update the condition to only check errorCode.getCode() against SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND (i.e., if errorCode != null && SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode()) then call trigger.next(); return;) to simplify logic and follow the standard error-code check pattern.testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy (1)
362-374: 推荐使用常量而非硬编码错误码字符串模拟器第 368 行使用硬编码字符串
"KEY_AGENT_SECRET_NOT_FOUND"设置错误，该值与SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND常量定义一致，当前生产代码中的错误匹配逻辑可以正确识别。但建议改为引用常量以提高可维护性，避免硬编码字符串在将来常量更新时造成同步问题。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy` around lines 362 - 374, Replace the hardcoded error string in the KVM GET secret simulator with the canonical constant: in the spec.simulator block handling KVMConstant.KVM_GET_SECRET_PATH (the lambda that builds cacheKey and creates SecretHostGetResponse rsp), set the error using the constant SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND instead of the literal "KEY_AGENT_SECRET_NOT_FOUND" so the simulator stays in sync with production error codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 5317-5351: The SecretHostGetMsg handler is calling
restf.asyncJsonPost without the host resource header, which can cause secret
cache key collisions across hosts; modify the asyncJsonPost call in
handle(SecretHostGetMsg) to include the HTTP header
Constants.AGENT_HTTP_HEADER_RESOURCE_UUID populated with the host UUID (use
msg.getHostUuid() or the host's uuid field) so the agent receives the host
resource header, and make the analogous change in the SecretHostEnsureMsg
handling code (the ensure-path call around
KVMAgentCommands.SecretHostEnsureCmd/KVMConstant.KVM_ENSURE_SECRET_PATH) so both
get and ensure include the header when invoking restf.asyncJsonPost.
---
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java`:
- Around line 371-376: The method loadResourceKeyRefContext currently re-fetches
providerUuid redundantly: remove the conditional block that checks and sets
context.providerUuid (the StringUtils.isBlank(...) branch) because providerUuid
is already set before this call; keep only the assignment of context.keyVersion
= resourceKeyBackend.findKeyVersionByTpm(context.tpmUuid) so
loadResourceKeyRefContext(PrepareTpmResourceContext) only populates keyVersion.
- Around line 261-296: The two flows "load-dek-for-host" and
"ensure-resource-key-ref" both call resourceKeyManager.getOrCreateKey(...) with
very similar GetOrCreateResourceKeyContext setup, which is confusing and
duplicates logic; refactor by extracting the shared get-or-create-key logic into
a single private helper (e.g., a private method that builds
GetOrCreateResourceKeyContext, invokes resourceKeyManager.getOrCreateKey and
populates tpmSpec/context fields) and call that helper from both flows while
preserving the distinct skip conditions and post-success behaviors, or
alternatively add concise comments above the run() implementations of
load-dek-for-host and ensure-resource-key-ref explaining their different
responsibilities (first: create/get key when keyVersion is empty; second: reload
dek for host when secret/dek missing) so maintainers understand why both calls
exist.
- Around line 249-255: The error-checking branch in KvmTpmExtensions (where
reply.getError() is assigned to errorCode) unnecessarily inspects
errorCode.getDetails(); update the condition to only check errorCode.getCode()
against SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND (i.e., if errorCode !=
null &&
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode()) then
call trigger.next(); return;) to simplify logic and follow the standard
error-code check pattern.
In `@testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy`:
- Around line 362-374: Replace the hardcoded error string in the KVM GET secret
simulator with the canonical constant: in the spec.simulator block handling
KVMConstant.KVM_GET_SECRET_PATH (the lambda that builds cacheKey and creates
SecretHostGetResponse rsp), set the error using the constant
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND instead of the literal
"KEY_AGENT_SECRET_NOT_FOUND" so the simulator stays in sync with production
error codes.
🪄 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: a7381ba2-14be-4fc6-8d5d-2f6a51be1fed
📒 Files selected for processing (11)
compute/src/main/java/org/zstack/compute/vm/devices/DummyTpmEncryptedResourceKeyBackend.javacompute/src/main/java/org/zstack/compute/vm/devices/TpmEncryptedResourceKeyBackend.javaheader/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.javaheader/src/main/java/org/zstack/header/secret/SecretHostDefineMsg.javaheader/src/main/java/org/zstack/header/secret/SecretHostGetMsg.javaheader/src/main/java/org/zstack/header/secret/SecretHostGetReply.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.javatestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy
Collaborator
|
Comment from yaohua.wu: Review: MR !9547 — ZSV-11630 (zstack)关联 MRs: premium !13427, zstack-utility !6883, key-manager !20 变更概述: 在 VM 启动前的 TPM 资源准备流程中，引入 Warning
Suggestion
整体评估新增的
Verdict: APPROVED🤖 Robot Reviewer |
MatheMatrix
force-pushed
the
sync/zstackio/secret-get@@3
branch
from
a0354b4 to
7cc5ff3
Compare
Resolves: ZSV-11630 Change-Id: I616a776e78666179637976616c776162616c6577
MatheMatrix
force-pushed
the
sync/zstackio/secret-get@@3
branch
from
7cc5ff3 to
78579b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
5325-5330:⚠️ Potential issue | 🔴 Critical为 secret agent 调用补上 host 资源头。
secret 是 host 侧资源，这两个请求绕过了
Http.call()，没有带Constants.AGENT_HTTP_HEADER_RESOURCE_UUID。这样同一组vmUuid/purpose/keyVersion会丢失 host 维度：迁移或重启后可能命中别的 host 的 secret，或者拿不到当前 host 上已创建的 secret。最好复用Http包装层，至少显式传 header。🔧 建议修复
String url = buildUrl(KVMConstant.KVM_GET_SECRET_PATH); + Map<String, String> headers = new HashMap<>(); + headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid()); KVMAgentCommands.SecretHostGetCmd cmd = new KVMAgentCommands.SecretHostGetCmd(); cmd.setVmUuid(msg.getVmUuid()); cmd.setPurpose(msg.getPurpose()); cmd.setKeyVersion(msg.getKeyVersion()); - restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) { + restf.asyncJsonPost(url, JSONObjectUtil.toJsonString(cmd), headers, + new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) { @@ String envelopeDekBase64 = java.util.Base64.getEncoder().encodeToString(envelope); String url = buildUrl(KVMConstant.KVM_ENSURE_SECRET_PATH); + Map<String, String> headers = new HashMap<>(); + headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid()); KVMAgentCommands.SecretHostDefineCmd cmd = new KVMAgentCommands.SecretHostDefineCmd(); cmd.setEncryptedDek(envelopeDekBase64); cmd.setVmUuid(msg.getVmUuid()); cmd.setPurpose(msg.getPurpose()); cmd.setKeyVersion(msg.getKeyVersion()); cmd.setDescription(msg.getDescription() != null ? msg.getDescription() : ""); - restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) { + restf.asyncJsonPost(url, JSONObjectUtil.toJsonString(cmd), headers, + new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) {Also applies to: 5435-5442
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 5325 - 5330, The secret agent calls (building URL via buildUrl(KVMConstant.KVM_GET_SECRET_PATH) and sending KVMAgentCommands.SecretHostGetCmd via restf.asyncJsonPost) are missing the host resource header; modify the asyncJsonPost invocation to include the Constants.AGENT_HTTP_HEADER_RESOURCE_UUID header with the host UUID (or, alternatively, route the request through the existing Http wrapper used elsewhere so the header is set automatically) so the secret lookup is scoped to the host; apply the same change to the other secret-related asyncJsonPost call in this class (the similar block around the SecretHost create/send).
🧹 Nitpick comments (4)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)
413-413: 建议补一行协议桥接注释，降低后续误改风险
keyVersion在 Java 侧是Integer，建议在字段旁补充“跨语言协议侧可能按字符串处理”的说明，便于后续维护时快速对齐多端定义。Also applies to: 472-472
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` at line 413, Add a bridging comment next to the private Integer keyVersion field(s) in KVMAgentCommands.java (the field named keyVersion) stating that on the cross-language protocol layer this value may be handled/serialized as a string, so other-language clients/servers should align types; update both occurrences of the keyVersion field in the class to include this explanatory note to reduce future mis-editing across language boundaries.testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy (1)
368-370: 建议避免硬编码错误码字符串Line [369] 直接写
"KEY_AGENT_SECRET_NOT_FOUND"，后续若常量变更会造成测试与实现脱节。建议复用SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND。建议修改
+import org.zstack.header.secret.SecretHostGetReply ... - rsp.setError("KEY_AGENT_SECRET_NOT_FOUND") + rsp.setError(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy` around lines 368 - 370, Replace the hard-coded error string in the KVMSimulator.groovy secret check with the shared constant to avoid divergence: in the code branch that currently calls rsp.setError("KEY_AGENT_SECRET_NOT_FOUND") (inside the secretUuid null check), use SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND instead; update the import or fully qualify the constant if needed so the test uses the same error code symbol as the implementation.plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java (2)
359-369: 建议为keyVersion字段添加文档注释说明类型转换。
keyVersion在 Java 端为Integer类型，但 proto 定义为string，Python 层通过str(key_version)进行转换。建议在此处或相关的SecretHostGetMsg/SecretHostDefineMsg类中添加 Javadoc 说明这一类型桥接，以避免后续维护时产生混淆。📝 建议的文档注释
static class PrepareTpmResourceContext { String tpmUuid; String backupFileUuid; String providerUuid; + /** + * Key version identifier. Note: Java uses Integer while proto/Python uses string. + * Python agent converts via str(key_version). + */ Integer keyVersion; String dekBase64;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around lines 359 - 369, PrepareTpmResourceContext.keyVersion is an Integer in Java but the proto uses a string (and Python sends str(key_version)); add a Javadoc comment on the keyVersion field (or on related message classes like SecretHostGetMsg/SecretHostDefineMsg) stating that the proto represents key_version as a string and callers convert between Integer and string (e.g., Python uses str(key_version)), so maintainers understand the type bridge and avoid incorrect conversions.
277-295: 建议提取重复的回调代码到私有辅助方法。此回调逻辑（设置
tpmSpec属性、context.dekBase64、context.keyVersion及 null 检查）与 "ensure-resource-key-ref" 流程（第 203-221 行）高度重复。提取为一个私有辅助方法可减少代码重复，便于后续维护。♻️ 建议的重构方案
private void handleResourceKeyResult(ResourceKeyResult result, TpmSpec tpmSpec, PrepareTpmResourceContext context, FlowTrigger trigger, String errorContext) { tpmSpec.setResourceKeyCreatedNew(result.isCreatedNewKey()); tpmSpec.setResourceKeyProviderUuid(result.getKeyProviderUuid()); context.dekBase64 = result.getDekBase64(); context.keyVersion = result.getKeyVersion(); if (context.keyVersion == null) { trigger.fail(operr("missing keyVersion for tpm[uuid:%s] %s", context.tpmUuid, errorContext)); return; } trigger.next(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around lines 277 - 295, Duplicate callback logic used in resourceKeyManager.getOrCreateKey (the anonymous ReturnValueCompletion<ResourceKeyResult> that sets tpmSpec, context.dekBase64 and context.keyVersion, checks null and calls trigger.next()/trigger.fail()) should be extracted into a private helper method (e.g., handleResourceKeyResult) that accepts ResourceKeyResult, TpmSpec, PrepareTpmResourceContext, FlowTrigger and an errorContext string; replace the anonymous success()/fail() bodies in both the current invocation and the analogous "ensure-resource-key-ref" flow with calls to this helper and keep fail() forwarding errorCode to trigger.fail().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 5468-5476: buildSecretAgentError currently writes the raw agent
message (rsp.getError()) into both ErrorCode.code and ErrorCode.details which
breaks exact matching like SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND in
KvmTpmExtensions; change buildSecretAgentError to set ErrorCode.code to a
stable, canonical error identifier (e.g. a new rsp.getErrorCode() or a
normalized enum/string) and set ErrorCode.details to the original rsp.getError()
text; if KVMAgentCommands.AgentResponse lacks a dedicated errorCode field, add
one (e.g. getErrorCode()/setErrorCode()) and populate it from the agent so
KvmTpmExtensions can perform exact matches against ERROR_CODE_SECRET_NOT_FOUND
while preserving full raw message in details.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java`:
- Around line 249-255: The current error check in KvmTpmExtensions uses strict
equals on errorCode.getCode() and errorCode.getDetails() against
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND which is brittle; update the
logic that inspects reply.getError() (the ErrorCode returned by
reply.getError()) to use a safer match such as errorCode.getCode() != null &&
errorCode.getCode().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND) OR
errorCode.getDetails() != null &&
errorCode.getDetails().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND),
or better still, add and check a dedicated errorCode field from the Python/Go
layer if available; preserve the existing flow (call trigger.next() and return)
when a "secret not found" match is found.
---
Duplicate comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 5325-5330: The secret agent calls (building URL via
buildUrl(KVMConstant.KVM_GET_SECRET_PATH) and sending
KVMAgentCommands.SecretHostGetCmd via restf.asyncJsonPost) are missing the host
resource header; modify the asyncJsonPost invocation to include the
Constants.AGENT_HTTP_HEADER_RESOURCE_UUID header with the host UUID (or,
alternatively, route the request through the existing Http wrapper used
elsewhere so the header is set automatically) so the secret lookup is scoped to
the host; apply the same change to the other secret-related asyncJsonPost call
in this class (the similar block around the SecretHost create/send).
---
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Line 413: Add a bridging comment next to the private Integer keyVersion
field(s) in KVMAgentCommands.java (the field named keyVersion) stating that on
the cross-language protocol layer this value may be handled/serialized as a
string, so other-language clients/servers should align types; update both
occurrences of the keyVersion field in the class to include this explanatory
note to reduce future mis-editing across language boundaries.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java`:
- Around line 359-369: PrepareTpmResourceContext.keyVersion is an Integer in
Java but the proto uses a string (and Python sends str(key_version)); add a
Javadoc comment on the keyVersion field (or on related message classes like
SecretHostGetMsg/SecretHostDefineMsg) stating that the proto represents
key_version as a string and callers convert between Integer and string (e.g.,
Python uses str(key_version)), so maintainers understand the type bridge and
avoid incorrect conversions.
- Around line 277-295: Duplicate callback logic used in
resourceKeyManager.getOrCreateKey (the anonymous
ReturnValueCompletion<ResourceKeyResult> that sets tpmSpec, context.dekBase64
and context.keyVersion, checks null and calls trigger.next()/trigger.fail())
should be extracted into a private helper method (e.g., handleResourceKeyResult)
that accepts ResourceKeyResult, TpmSpec, PrepareTpmResourceContext, FlowTrigger
and an errorContext string; replace the anonymous success()/fail() bodies in
both the current invocation and the analogous "ensure-resource-key-ref" flow
with calls to this helper and keep fail() forwarding errorCode to
trigger.fail().
In `@testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy`:
- Around line 368-370: Replace the hard-coded error string in the
KVMSimulator.groovy secret check with the shared constant to avoid divergence:
in the code branch that currently calls
rsp.setError("KEY_AGENT_SECRET_NOT_FOUND") (inside the secretUuid null check),
use SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND instead; update the import or
fully qualify the constant if needed so the test uses the same error code symbol
as the implementation.
🪄 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: 6480e92f-3aed-4c3b-bed5-4d306a1bb78b
📒 Files selected for processing (11)
compute/src/main/java/org/zstack/compute/vm/devices/DummyTpmEncryptedResourceKeyBackend.javacompute/src/main/java/org/zstack/compute/vm/devices/TpmEncryptedResourceKeyBackend.javaheader/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.javaheader/src/main/java/org/zstack/header/secret/SecretHostDefineMsg.javaheader/src/main/java/org/zstack/header/secret/SecretHostGetMsg.javaheader/src/main/java/org/zstack/header/secret/SecretHostGetReply.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.javatestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy
✅ Files skipped from review due to trivial changes (5)
- plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
- compute/src/main/java/org/zstack/compute/vm/devices/TpmEncryptedResourceKeyBackend.java
- header/src/main/java/org/zstack/header/secret/SecretHostGetReply.java
- header/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.java
- header/src/main/java/org/zstack/header/secret/SecretHostGetMsg.java
🚧 Files skipped from review as they are similar to previous changes (2)
- compute/src/main/java/org/zstack/compute/vm/devices/DummyTpmEncryptedResourceKeyBackend.java
- header/src/main/java/org/zstack/header/secret/SecretHostDefineMsg.java
Comment on lines
+5468
to
+5476
| private ErrorCode buildSecretAgentError(KVMAgentCommands.AgentResponse rsp, String defaultMessage) { | ||
| if (rsp != null && rsp.getError() != null) { | ||
| ErrorCode err = new ErrorCode(); | ||
| err.setCode(rsp.getError()); | ||
| err.setDetails(rsp.getError()); | ||
| return err; | ||
| } | ||
| return operr(defaultMessage); | ||
| } |
There was a problem hiding this comment.
不要把原始错误文案当成稳定的 error code。
buildSecretAgentError() 现在把 rsp.getError() 同时写进 code 和 details。但 plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java:248-259 对 SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND 做的是精确匹配；只要 agent 在错误文案后追加上下文，首次启动的“secret 不存在”就会被误判成致命失败。这里应保留独立、稳定的 errorCode，details 再保留原始错误文本；如果 KVMAgentCommands.AgentResponse 还没暴露该字段，建议先在那里补齐。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 5468 -
5476, buildSecretAgentError currently writes the raw agent message
(rsp.getError()) into both ErrorCode.code and ErrorCode.details which breaks
exact matching like SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND in
KvmTpmExtensions; change buildSecretAgentError to set ErrorCode.code to a
stable, canonical error identifier (e.g. a new rsp.getErrorCode() or a
normalized enum/string) and set ErrorCode.details to the original rsp.getError()
text; if KVMAgentCommands.AgentResponse lacks a dedicated errorCode field, add
one (e.g. getErrorCode()/setErrorCode()) and populate it from the agent so
KvmTpmExtensions can perform exact matches against ERROR_CODE_SECRET_NOT_FOUND
while preserving full raw message in details.
Comment on lines
+249
to
+255
| ErrorCode errorCode = reply.getError(); | ||
| if (errorCode != null | ||
| && (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode()) | ||
| || SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getDetails()))) { | ||
| trigger.next(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
错误码匹配方式脆弱，建议使用 contains() 或独立的错误码字段。
当前使用 equals() 进行精确匹配来检测 "secret not found" 错误。如果 Go 或 Python 层在错误消息中附加了上下文信息，equals() 将失败，可能导致首次启动 VM 被错误地判定为异常。
建议改用 contains() 或从 Python 层传递独立的 errorCode 字段：
🔧 建议的修复方案
ErrorCode errorCode = reply.getError();
if (errorCode != null
- && (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode())
- || SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getDetails()))) {
+ && (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode())
+ || (errorCode.getDetails() != null
+ && errorCode.getDetails().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND)))) {
trigger.next();
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around
lines 249 - 255, The current error check in KvmTpmExtensions uses strict equals
on errorCode.getCode() and errorCode.getDetails() against
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND which is brittle; update the
logic that inspects reply.getError() (the ErrorCode returned by
reply.getError()) to use a safer match such as errorCode.getCode() != null &&
errorCode.getCode().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND) OR
errorCode.getDetails() != null &&
errorCode.getDetails().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND),
or better still, add and check a dedicated errorCode field from the Python/Go
layer if available; preserve the existing flow (call trigger.next() and return)
when a "secret not found" match is found.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (2)
5468-5476:⚠️ Potential issue | 🟠 Major不要把原始错误文案写成稳定的 error code。
Line 5471-5472 现在把
rsp.getError()同时写进code和details。下游 TPM 流程会精确匹配SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND；只要 agent 在错误文案后追加上下文，首次启动时“secret 不存在”就会被误判成致命失败。这里应保留独立、稳定的errorCode，原始文案只放在details；如果KVMAgentCommands.AgentResponse还没有该字段，需要连同 agent 返回一起补齐。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 5468 - 5476, The buildSecretAgentError method currently writes rsp.getError() into both ErrorCode.code and .details which treats a free-form agent message as a stable error code; change buildSecretAgentError to set ErrorCode.code to a stable, predefined error code (e.g., SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND or a mapped constant) and put the agent's textual message only into ErrorCode.details; if KVMAgentCommands.AgentResponse lacks a dedicated errorCode field, extend the agent response (and agent-side return) to include a separate errorCode to be used for ErrorCode.code while preserving rsp.getError() as the human-readable details; ensure callers that match on SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND continue to compare against the stable code.
5325-5351:⚠️ Potential issue | 🔴 Critical为 secret agent 调用补上 host 资源头。
Line 5330 和 Line 5442 这里绕过了
Http.call()，请求没有带Constants.AGENT_HTTP_HEADER_RESOURCE_UUID。这条 secret 复用链路是 host 侧语义；丢掉 host 维度后，相同vmUuid/purpose/keyVersion在不同 host 上会互相串扰，迁移或重启时可能命中错误的 secret，或者误判目标 host 已经存在该 secret。🔧 建议修复
String url = buildUrl(KVMConstant.KVM_GET_SECRET_PATH); + Map<String, String> headers = new HashMap<>(); + headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid()); KVMAgentCommands.SecretHostGetCmd cmd = new KVMAgentCommands.SecretHostGetCmd(); cmd.setVmUuid(msg.getVmUuid()); cmd.setPurpose(msg.getPurpose()); cmd.setKeyVersion(msg.getKeyVersion()); - restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) { + restf.asyncJsonPost(url, JSONObjectUtil.toJsonString(cmd), headers, + new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) { `@Override` public void fail(ErrorCode err) { reply.setError(err != null ? err : operr("get secret on agent failed")); @@ String envelopeDekBase64 = java.util.Base64.getEncoder().encodeToString(envelope); String url = buildUrl(KVMConstant.KVM_ENSURE_SECRET_PATH); + Map<String, String> headers = new HashMap<>(); + headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid()); KVMAgentCommands.SecretHostDefineCmd cmd = new KVMAgentCommands.SecretHostDefineCmd(); cmd.setEncryptedDek(envelopeDekBase64); cmd.setVmUuid(msg.getVmUuid()); cmd.setPurpose(msg.getPurpose()); cmd.setKeyVersion(msg.getKeyVersion()); cmd.setDescription(msg.getDescription() != null ? msg.getDescription() : ""); - restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) { + restf.asyncJsonPost(url, JSONObjectUtil.toJsonString(cmd), headers, + new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) { `@Override` public void fail(ErrorCode err) { reply.setError(err != null ? err : operr("ensure secret on agent failed"));Also applies to: 5435-5465
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 5325 - 5351, The secret-agent asyncJsonPost calls in KVMHost (the SecretHostGet flow using KVMAgentCommands.SecretHostGetCmd and the similar call around the 5435-5465 region) are missing the host resource header, causing cross-host secret collisions; fix by adding the agent resource header Constants.AGENT_HTTP_HEADER_RESOURCE_UUID (or the project constant used for agent resource headers) with the host UUID to the HTTP request headers when calling restf.asyncJsonPost/buildUrl(KVMConstant.KVM_GET_SECRET_PATH), i.e., include the header key and the host's UUID (this.getSelf().getUuid() or msg.getHostUuid() as appropriate) in those asyncJsonPost calls so the agent sees the host resource scope.plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java (1)
249-255:⚠️ Potential issue | 🟠 Major“secret not found” 的判定仍然过于脆弱。
这里对
errorCode.getDetails()做精确匹配，agent 侧只要在详情里追加上下文，首次启动就会把“secret 不存在”误判成真正失败。至少把details分支改成contains(...)；更稳妥的是透传独立错误码字段。🔧 建议修改
ErrorCode errorCode = reply.getError(); if (errorCode != null && (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode()) - || SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getDetails()))) { + || (errorCode.getDetails() != null + && errorCode.getDetails().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND)))) { trigger.next(); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around lines 249 - 255, The current check for "secret not found" in KvmTpmExtensions (using reply.getError() and comparing SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND to errorCode.getDetails() by exact match) is brittle; change the details branch to use contains(...) instead of equals(...) or, better, rely only on the canonical error code field (errorCode.getCode()) so agent-added context in details won't break detection. Update the conditional around reply.getError() in KvmTpmExtensions.java to check SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode()) || (errorCode.getDetails() != null && errorCode.getDetails().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND)) and keep the existing trigger.next() behavior; ideally refactor to prefer the code field and treat details as a fallback.
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)
408-455: SecretHostDefineCmd 字段变更符合预期。将
providerName替换为keyVersion与 PR 目标一致，使用密钥版本号作为秘密标识符。根据 PR 评审意见，建议在此类或相关消息类（如SecretHostGetMsg/SecretHostDefineMsg）的 Javadoc 中记录 JavaInteger与 proto/Pythonstring之间的类型转换约定，以防止未来出现类型不匹配问题。📝 建议添加 Javadoc 文档
public static class SecretHostDefineCmd extends AgentCommand { + /** + * Key version number for secret identification. + * Note: Java uses Integer while Python agent expects string (converted via str(key_version)). + */ private Integer keyVersion;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines 408 - 455, SecretHostDefineCmd now replaces providerName with Integer keyVersion, but there's no Javadoc explaining the Integer <-> proto/Python string conversion convention; add a clear Javadoc to SecretHostDefineCmd (and mirror notes in related message classes SecretHostGetMsg and SecretHostDefineMsg) stating that keyVersion is a Java Integer and will be serialized/deserialized to/from a proto/Python string representation (include expected format, nullability and any sentinel values), and mention the historical providerName rename so future readers know why keyVersion is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java`:
- Around line 177-181: 在 KvmTpmExtensions 中当前对 ALLOWED_TPM_VM_WITHOUT_KMS
的放行判断没有传递到后续的 getOrCreateKey() 调用，导致在 providerUuid/secretUuid 为空的合法 without-kms
场景仍会走到 getOrCreateKey() 并可能失败；修改方法是在触发 skip/继续流程前用同样的 shouldSkip 条件（基于
VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS 和 context.providerUuid/secretUuid
是否为空）直接返回或跳过后续调用，并确保在调用 getOrCreateKey() 之前明确检查并拒绝空的
providerUuid/secretUuid，亦在同样的逻辑分支（同样在文件中另一处 264-275
区域）做对应修复，参考符号：KvmTpmExtensions, getOrCreateKey(), context.providerUuid,
secretUuid, VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS。
---
Duplicate comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 5468-5476: The buildSecretAgentError method currently writes
rsp.getError() into both ErrorCode.code and .details which treats a free-form
agent message as a stable error code; change buildSecretAgentError to set
ErrorCode.code to a stable, predefined error code (e.g.,
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND or a mapped constant) and put the
agent's textual message only into ErrorCode.details; if
KVMAgentCommands.AgentResponse lacks a dedicated errorCode field, extend the
agent response (and agent-side return) to include a separate errorCode to be
used for ErrorCode.code while preserving rsp.getError() as the human-readable
details; ensure callers that match on
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND continue to compare against the
stable code.
- Around line 5325-5351: The secret-agent asyncJsonPost calls in KVMHost (the
SecretHostGet flow using KVMAgentCommands.SecretHostGetCmd and the similar call
around the 5435-5465 region) are missing the host resource header, causing
cross-host secret collisions; fix by adding the agent resource header
Constants.AGENT_HTTP_HEADER_RESOURCE_UUID (or the project constant used for
agent resource headers) with the host UUID to the HTTP request headers when
calling restf.asyncJsonPost/buildUrl(KVMConstant.KVM_GET_SECRET_PATH), i.e.,
include the header key and the host's UUID (this.getSelf().getUuid() or
msg.getHostUuid() as appropriate) in those asyncJsonPost calls so the agent sees
the host resource scope.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java`:
- Around line 249-255: The current check for "secret not found" in
KvmTpmExtensions (using reply.getError() and comparing
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND to errorCode.getDetails() by
exact match) is brittle; change the details branch to use contains(...) instead
of equals(...) or, better, rely only on the canonical error code field
(errorCode.getCode()) so agent-added context in details won't break detection.
Update the conditional around reply.getError() in KvmTpmExtensions.java to check
SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode()) ||
(errorCode.getDetails() != null &&
errorCode.getDetails().contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND))
and keep the existing trigger.next() behavior; ideally refactor to prefer the
code field and treat details as a fallback.
---
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 408-455: SecretHostDefineCmd now replaces providerName with
Integer keyVersion, but there's no Javadoc explaining the Integer <->
proto/Python string conversion convention; add a clear Javadoc to
SecretHostDefineCmd (and mirror notes in related message classes
SecretHostGetMsg and SecretHostDefineMsg) stating that keyVersion is a Java
Integer and will be serialized/deserialized to/from a proto/Python string
representation (include expected format, nullability and any sentinel values),
and mention the historical providerName rename so future readers know why
keyVersion is used.
🪄 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: d9558478-24c2-4cee-9296-3833e4a26dec
📒 Files selected for processing (11)
compute/src/main/java/org/zstack/compute/vm/devices/DummyTpmEncryptedResourceKeyBackend.javacompute/src/main/java/org/zstack/compute/vm/devices/TpmEncryptedResourceKeyBackend.javaheader/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.javaheader/src/main/java/org/zstack/header/secret/SecretHostDefineMsg.javaheader/src/main/java/org/zstack/header/secret/SecretHostGetMsg.javaheader/src/main/java/org/zstack/header/secret/SecretHostGetReply.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.javatestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy
✅ Files skipped from review due to trivial changes (5)
- header/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.java
- header/src/main/java/org/zstack/header/secret/SecretHostGetReply.java
- header/src/main/java/org/zstack/header/secret/SecretHostGetMsg.java
- testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy
- header/src/main/java/org/zstack/header/secret/SecretHostDefineMsg.java
🚧 Files skipped from review as they are similar to previous changes (3)
- compute/src/main/java/org/zstack/compute/vm/devices/DummyTpmEncryptedResourceKeyBackend.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
- compute/src/main/java/org/zstack/compute/vm/devices/TpmEncryptedResourceKeyBackend.java
Comment on lines
+177
to
181
| boolean shouldSkip = VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS.value(Boolean.class) | ||
| && StringUtils.isBlank(context.providerUuid); | ||
| if (shouldSkip) { | ||
| logger.info("skip create-dek: allowed.tpm.vm.without.kms is enabled and no KMS provider bound"); | ||
| } |
There was a problem hiding this comment.
无 KMS 的放行路径在这里又被拉回 getOrCreateKey() 了。
前一个 flow 已经明确允许 ALLOWED_TPM_VM_WITHOUT_KMS && providerUuid 为空时继续，但这里的 skip() 没带上同样的条件，secretUuid 为空时仍会继续调用 getOrCreateKey()。这样会把合法的 without-kms 场景重新变成 KMS/DEK 依赖，空 providerUuid 还可能直接导致失败。
🔧 建议修改
`@Override`
public boolean skip(Map data) {
- return StringUtils.isNotBlank(spec.getDevicesSpec().getTpm().getSecretUuid()) || context.dekBase64 != null;
+ boolean withoutKms = VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS.value(Boolean.class)
+ && StringUtils.isBlank(context.providerUuid);
+ return withoutKms
+ || StringUtils.isNotBlank(spec.getDevicesSpec().getTpm().getSecretUuid())
+ || context.dekBase64 != null;
}Also applies to: 264-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java` around
lines 177 - 181, 在 KvmTpmExtensions 中当前对 ALLOWED_TPM_VM_WITHOUT_KMS
的放行判断没有传递到后续的 getOrCreateKey() 调用，导致在 providerUuid/secretUuid 为空的合法 without-kms
场景仍会走到 getOrCreateKey() 并可能失败；修改方法是在触发 skip/继续流程前用同样的 shouldSkip 条件（基于
VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS 和 context.providerUuid/secretUuid
是否为空）直接返回或跳过后续调用，并确保在调用 getOrCreateKey() 之前明确检查并拒绝空的
providerUuid/secretUuid，亦在同样的逻辑分支（同样在文件中另一处 264-275
区域）做对应修复，参考符号：KvmTpmExtensions, getOrCreateKey(), context.providerUuid,
secretUuid, VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS。
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
2 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-11630
Change-Id: I616a776e78666179637976616c776162616c6577
sync from gitlab !9547