<fix>[compute]: add allowed.tpm.vm.without.kms global config

[zstack-robot-1]

Related: ZSV-11310

Change-Id: I67787561736968637666626a627366757775696a

sync from gitlab !9521

[coderabbitai]

Walkthrough

新增布尔全局配置 ALLOWED_TPM_VM_WITHOUT_KMS（默认 false），并在 KVM TPM 扩展的 VM 实例化流程中，当该标志开启且 TPM 提供者信息缺失时，条件跳过 DEK 创建步骤。

Changes

Cohort / File(s)	Summary
全局配置 compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java	新增公共全局配置 ALLOWED_TPM_VM_WITHOUT_KMS（key=allowed.tpm.vm.without.kms），类型 Boolean，默认 false，并添加值校验与描述。
KVM TPM 扩展逻辑 plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java	在 preInstantiateVmResource 的 "create-dek" 步骤中，修改 skip(Map data) 条件：当新配置启用且 context.providerUuid 或 context.providerName 为空时返回 true（跳过 DEK 创建），并记录信息日志；否则保持原行为。

Sequence Diagram(s)

（无序列图）

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 我在代码园里蹦跶笑，
加了开关轻轻巧，
TPM 若无 KMS 也能跑，
步骤跳过不慌张，
小兔鼓掌庆一遭 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	PR title accurately describes the main change: adding a new global config 'allowed.tpm.vm.without.kms' to the compute module, which is the primary modification in the changeset.
Description check	✅ Passed	PR description is related to the changeset, mentioning the related issue (ZSV-11310) and sync source, though brief and lacking implementation details.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sync/wenhao.zhang/zsv-ldap-2@@2

---

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

[coderabbitai]

🧹 Nitpick comments (1)

compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java (1)

143-145: 缺少 @BindResourceConfig 注解——确认是否需要支持资源级别的配置覆盖

对比同类 TPM 配置 RESET_TPM_AFTER_VM_CLONE（第 140 行），该配置具有 @BindResourceConfig(value = {VmInstanceVO.class, ClusterVO.class})，允许按 VM 或集群级别覆盖全局配置。

当前新增的 ALLOWED_TPM_VM_WITHOUT_KMS 没有此注解，意味着只能在全局级别配置。如果这是有意设计（例如出于安全策略考虑，不应允许单个 VM 绕过 KMS 要求），则无需修改。否则可以考虑添加：

`@BindResourceConfig`(value = {VmInstanceVO.class, ClusterVO.class})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java` around lines
143 - 145, ALLOWED_TPM_VM_WITHOUT_KMS lacks the resource-binding annotation so
it cannot be overridden per-VM or per-cluster like RESET_TPM_AFTER_VM_CLONE; if
per-resource overrides are desired, add the `@BindResourceConfig` annotation (e.g.
`@BindResourceConfig`(value = {VmInstanceVO.class, ClusterVO.class})) above the
ALLOWED_TPM_VM_WITHOUT_KMS GlobalConfig declaration so it supports
VM/Cluster-level configuration; if this is intentionally global-only, leave
as-is and add a comment explaining the security rationale.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java`:
- Around line 143-145: ALLOWED_TPM_VM_WITHOUT_KMS lacks the resource-binding
annotation so it cannot be overridden per-VM or per-cluster like
RESET_TPM_AFTER_VM_CLONE; if per-resource overrides are desired, add the
`@BindResourceConfig` annotation (e.g. `@BindResourceConfig`(value =
{VmInstanceVO.class, ClusterVO.class})) above the ALLOWED_TPM_VM_WITHOUT_KMS
GlobalConfig declaration so it supports VM/Cluster-level configuration; if this
is intentionally global-only, leave as-is and add a comment explaining the
security rationale.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 047d107b-17d9-4615-9a5b-c82cc7184720

📥 Commits

Reviewing files that changed from the base of the PR and between c7c24cf and 22763e0.

📒 Files selected for processing (2)

• compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java
• plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java

[MatheMatrix]

Comment from yaohua.wu:

Review: MR !9521 — ZSV-11310

变更概述：新增全局配置 allowed.tpm.vm.without.kms（默认 false），允许带 TPM 的 VM 在没有 KMS 的情况下启动。当配置开启且 TPM 未绑定 KMS provider 时，跳过 DEK 创建和 secret 定义流程。

正确性分析 ✅

核心逻辑 正确且完整：

1. skip() 逻辑（KvmTpmExtensions.java:174-175）：config=true && providerInfo缺失 时跳过 "create-dek" flow，条件设计合理 — 只在两个条件同时满足时才跳过：

   • config=true 但 provider 存在 → 不跳过，正常创建 DEK ✅
   • config=false 但 provider 缺失 → 不跳过，进入 run() 后 trigger.fail() 报错 ✅
   • config=true 且 provider 缺失 → 跳过，允许无 KMS 启动 ✅

2. 下游 flow 联动正确："define-secret-on-host" 的 skip() 检查 context.dekBase64 == null，当 "create-dek" 被跳过时 dekBase64 保持 null → 自动跳过 secret 定义 ✅

3. preReleaseVmResource 兼容：无 KMS 时 isResourceKeyCreatedNew() 为 false → 直接 success，无需 rollback ✅

Warning

1. [KvmTpmExtensions.java:70-84] beforeStartVmOnKvm 未做 config 联动检查 — 当 ALLOWED_TPM_VM_WITHOUT_KMS=true 且无 KMS provider 时，此方法仍会将 keyProviderUuid=null 和 secretUuid=null 设入 TpmTO 并发送给 KVM agent。

   需确认 agent 端（kvmagent start_vm 处理 TPM 部分）能正确处理 keyProviderUuid/secretUuid 为空的情况。如果 agent 端对这些字段做了非空校验，VM 启动会在 agent 侧报错，给出的错误信息不如管理节点侧清晰。

   建议：在 beforeStartVmOnKvm 中，当 config 启用且 provider 为空时，考虑不设置 keyProviderUuid/secretUuid（或设为特定标识），让 agent 明确知道这是"允许无 KMS"的场景而非配置错误。

Suggestion

1. [KvmTpmExtensions.java:174] 建议增加 skip 日志 — 当 skip() 返回 true 跳过 DEK 创建时，缺少日志记录。在 debug 场景下如果有人误开此配置，排查问题时无从得知 DEK 创建被跳过了。建议：

   @Override
   public boolean skip(Map data) {
       boolean shouldSkip = VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS.value(Boolean.class) &&
               (StringUtils.isBlank(context.providerUuid) || StringUtils.isBlank(context.providerName));
       if (shouldSkip) {
           logger.info("skip create-dek for tpm[uuid:%s]: allowed.tpm.vm.without.kms is enabled and no KMS provider bound", context.tpmUuid);
       }
       return shouldSkip;
   }

2. [VmGlobalConfig.java:143] description 语法 — "allowed TPM VM start without KMS" 建议改为 "allow TPM VM to start without KMS"，当前语法不太自然。

3. [VmGlobalConfig.java:143] 缺少 @BindResourceConfig — 对比同类配置 RESET_TPM_AFTER_VM_CLONE（支持 VM/Cluster 级别覆盖），新配置只支持全局级别。根据 doc 描述（"不暴露"、debug 场景），这应该是有意设计。如果确实如此，建议加一行注释说明原因（安全策略考虑）。

Verdict: APPROVED

变更逻辑正确，skip 条件设计合理，下游 flow 联动无误。Warning 项建议确认 agent 端兼容性，但不阻塞合并。

---

🤖 Robot Reviewer

[coderabbitai]

Actionable comments posted: 1

🤖 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 175-180: 当前的 shouldSkip 判定以任一为空（context.providerUuid 或
context.providerName）就跳过 create-dek，导致当 providerUuid 已存在但 providerName
临时为空时误放行；请将判定收敛为只有 context.providerUuid 为空才跳过（保留 providerName 不参与此 skip
决策），以便与启动前 run 的校验逻辑一致，修改 KvmTpmExtensions 中计算 shouldSkip 的逻辑（参见 shouldSkip 变量与
context.providerUuid / context.providerName）以只基于 providerUuid 为空决定返回 true。

🪄 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: 2647c130-dc97-48c7-8563-0fd1c9ac70e9

📥 Commits

Reviewing files that changed from the base of the PR and between 22763e0 and 1d7126c.

📒 Files selected for processing (2)

• compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java
• plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java

✅ Files skipped from review due to trivial changes (1)

• compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java

[coderabbitai]

⚠️ Potential issue | 🟠 Major

skip 条件过宽，可能误把“已绑定 KMS”当成“无 KMS”放行。

Line 175 当前用 providerUuid/providerName 任一为空就跳过。这样在 providerUuid 已存在但 providerName 暂时为空时，会直接跳过 create-dek，导致 VM 在本应走 KMS 的情况下被放行为无 KMS 启动。

建议修复（收敛为“仅 providerUuid 缺失才跳过”，并与 run 前置校验保持一致）

@@
-                boolean shouldSkip = VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS.value(Boolean.class) &&
-                        (StringUtils.isBlank(context.providerUuid) || StringUtils.isBlank(context.providerName));
+                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");
                 }
                 return shouldSkip;
@@
-                if (StringUtils.isBlank(context.providerUuid) || StringUtils.isBlank(context.providerName)) {
+                if (StringUtils.isBlank(context.providerUuid)) {
                     trigger.fail(operr("missing TPM resource key binding for tpm[uuid:%s], attachKeyProviderToTpm must run before create-dek", context.tpmUuid));
                     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 175 - 180, 当前的 shouldSkip 判定以任一为空（context.providerUuid 或
context.providerName）就跳过 create-dek，导致当 providerUuid 已存在但 providerName
临时为空时误放行；请将判定收敛为只有 context.providerUuid 为空才跳过（保留 providerName 不参与此 skip
决策），以便与启动前 run 的校验逻辑一致，修改 KvmTpmExtensions 中计算 shouldSkip 的逻辑（参见 shouldSkip 变量与
context.providerUuid / context.providerName）以只基于 providerUuid 为空决定返回 true。