Skip to content

Commit 74011f4

Browse files
author
zhijian.liu
committed
<fix>[kms]: add test case for delete secret
Resolves: ZSV-11630
1 parent f1c6817 commit 74011f4

10 files changed

Lines changed: 299 additions & 27 deletions

File tree

‎conf/springConfigXml/Kvm.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,9 @@
256256
<zstack:plugin>
257257
<zstack:extension interface="org.zstack.kvm.KVMStartVmExtensionPoint" />
258258
<zstack:extension interface="org.zstack.header.vm.PreVmInstantiateResourceExtensionPoint" />
259+
<zstack:extension interface="org.zstack.header.vm.VmInstanceMigrateExtensionPoint" />
259260
<zstack:extension interface="org.zstack.header.vm.VmAfterExpungeExtensionPoint" />
261+
<zstack:extension interface="org.zstack.header.vm.VmStateChangedExtensionPoint" />
260262
</zstack:plugin>
261263
</bean>
262264

‎header/src/main/java/org/zstack/header/secret/SecretHostDeleteMsg.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@
33
import org.zstack.header.host.HostMessage;
44
import org.zstack.header.message.NeedReplyMessage;
55

6-
/**
7-
* Request to delete existing secret on KVM host by vmUuid/purpose/keyVersion.
8-
* keyVersion can be null, then agent may remove all matched versions for the vm+purpose.
9-
*/
106
public class SecretHostDeleteMsg extends NeedReplyMessage implements HostMessage {
117
private String hostUuid;
128
private String vmUuid;

‎plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ public static class SecretHostDefineCmd extends AgentCommand {
412412
private String purpose;
413413
private Integer keyVersion;
414414
private String description;
415+
private String usageInstance;
415416

416417
public String getEncryptedDek() {
417418
return encryptedDek;
@@ -452,6 +453,14 @@ public String getDescription() {
452453
public void setDescription(String description) {
453454
this.description = description;
454455
}
456+
457+
public String getUsageInstance() {
458+
return usageInstance;
459+
}
460+
461+
public void setUsageInstance(String usageInstance) {
462+
this.usageInstance = usageInstance;
463+
}
455464
}
456465

457466
public static class SecretHostDefineResponse extends AgentResponse {
@@ -470,6 +479,7 @@ public static class SecretHostGetCmd extends AgentCommand {
470479
private String vmUuid;
471480
private String purpose;
472481
private Integer keyVersion;
482+
private String usageInstance;
473483

474484
public String getVmUuid() {
475485
return vmUuid;
@@ -494,6 +504,14 @@ public Integer getKeyVersion() {
494504
public void setKeyVersion(Integer keyVersion) {
495505
this.keyVersion = keyVersion;
496506
}
507+
508+
public String getUsageInstance() {
509+
return usageInstance;
510+
}
511+
512+
public void setUsageInstance(String usageInstance) {
513+
this.usageInstance = usageInstance;
514+
}
497515
}
498516

499517
public static class SecretHostGetResponse extends AgentResponse {
@@ -512,6 +530,7 @@ public static class SecretHostDeleteCmd extends AgentCommand {
512530
private String vmUuid;
513531
private String purpose;
514532
private Integer keyVersion;
533+
private String usageInstance;
515534

516535
public String getVmUuid() {
517536
return vmUuid;
@@ -536,6 +555,14 @@ public Integer getKeyVersion() {
536555
public void setKeyVersion(Integer keyVersion) {
537556
this.keyVersion = keyVersion;
538557
}
558+
559+
public String getUsageInstance() {
560+
return usageInstance;
561+
}
562+
563+
public void setUsageInstance(String usageInstance) {
564+
this.usageInstance = usageInstance;
565+
}
539566
}
540567

541568
public static class SecretHostDeleteResponse extends AgentResponse {

‎plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ public interface KVMConstant {
138138

139139
/** Max size in bytes for DEK payload in SecretHostDefine (decoded from dekBase64). */
140140
int MAX_DEK_BYTES = 1024;
141+
String HOST_SECRET_USAGE_INSTANCE_VTPM = "tpm0";
141142

142143
String KVM_HOST_FILE_DOWNLOAD_PATH = "/host/file/download";
143144
String KVM_HOST_FILE_UPLOAD_PATH = "/host/file/upload";

‎plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5327,11 +5327,14 @@ private void handle(SecretHostGetMsg msg) {
53275327
}
53285328

53295329
String url = buildUrl(KVMConstant.KVM_GET_SECRET_PATH);
5330+
Map<String, String> headers = new HashMap<>();
5331+
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
53305332
KVMAgentCommands.SecretHostGetCmd cmd = new KVMAgentCommands.SecretHostGetCmd();
53315333
cmd.setVmUuid(msg.getVmUuid());
53325334
cmd.setPurpose(msg.getPurpose());
53335335
cmd.setKeyVersion(msg.getKeyVersion());
5334-
restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) {
5336+
cmd.setUsageInstance(KVMConstant.HOST_SECRET_USAGE_INSTANCE_VTPM);
5337+
restf.asyncJsonPost(url, cmd, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) {
53355338
@Override
53365339
public void fail(ErrorCode err) {
53375340
reply.setError(err != null ? err : operr("get secret on agent failed"));
@@ -5437,13 +5440,16 @@ private void handle(SecretHostDefineMsg msg) {
54375440
}
54385441
String envelopeDekBase64 = java.util.Base64.getEncoder().encodeToString(envelope);
54395442
String url = buildUrl(KVMConstant.KVM_ENSURE_SECRET_PATH);
5443+
Map<String, String> headers = new HashMap<>();
5444+
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
54405445
KVMAgentCommands.SecretHostDefineCmd cmd = new KVMAgentCommands.SecretHostDefineCmd();
54415446
cmd.setEncryptedDek(envelopeDekBase64);
54425447
cmd.setVmUuid(msg.getVmUuid());
54435448
cmd.setPurpose(msg.getPurpose());
54445449
cmd.setKeyVersion(msg.getKeyVersion());
54455450
cmd.setDescription(msg.getDescription() != null ? msg.getDescription() : "");
5446-
restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) {
5451+
cmd.setUsageInstance(KVMConstant.HOST_SECRET_USAGE_INSTANCE_VTPM);
5452+
restf.asyncJsonPost(url, cmd, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) {
54475453
@Override
54485454
public void fail(ErrorCode err) {
54495455
reply.setError(err != null ? err : operr("ensure secret on agent failed"));
@@ -5476,24 +5482,41 @@ private void handle(SecretHostDeleteMsg msg) {
54765482
bus.reply(msg, reply);
54775483
return;
54785484
}
5485+
if (msg.getKeyVersion() == null) {
5486+
logger.debug(String.format(
5487+
"skip delete host secret on host[uuid:%s]: keyVersion is null (vm[uuid:%s], purpose:%s)",
5488+
getSelf().getUuid(), msg.getVmUuid(), msg.getPurpose()));
5489+
bus.reply(msg, reply);
5490+
return;
5491+
}
54795492

54805493
String url = buildUrl(KVMConstant.KVM_DELETE_SECRET_PATH);
5494+
Map<String, String> headers = new HashMap<>();
5495+
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
54815496
KVMAgentCommands.SecretHostDeleteCmd cmd = new KVMAgentCommands.SecretHostDeleteCmd();
54825497
cmd.setVmUuid(msg.getVmUuid());
54835498
cmd.setPurpose(msg.getPurpose());
54845499
cmd.setKeyVersion(msg.getKeyVersion());
5500+
cmd.setUsageInstance(KVMConstant.HOST_SECRET_USAGE_INSTANCE_VTPM);
54855501

5486-
restf.asyncJsonPost(url, cmd, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDeleteResponse>(msg, reply) {
5502+
restf.asyncJsonPost(url, cmd, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDeleteResponse>(msg, reply) {
54875503
@Override
54885504
public void fail(ErrorCode err) {
5489-
reply.setError(err != null ? err : operr("delete secret on agent failed"));
5505+
logger.warn(String.format(
5506+
"best-effort delete secret on agent failed on host[uuid:%s] for vm[uuid:%s]: %s",
5507+
getSelf().getUuid(), msg.getVmUuid(),
5508+
err != null ? err.getDetails() : "unknown error"));
54905509
bus.reply(msg, reply);
54915510
}
54925511

54935512
@Override
54945513
public void success(KVMAgentCommands.SecretHostDeleteResponse rsp) {
54955514
if (rsp == null || !rsp.isSuccess()) {
5496-
reply.setError(buildSecretAgentError(rsp, "delete secret failed"));
5515+
ErrorCode agentErr = buildSecretAgentError(rsp, "delete secret failed");
5516+
logger.warn(String.format(
5517+
"best-effort delete secret on agent failed on host[uuid:%s] for vm[uuid:%s]: %s",
5518+
getSelf().getUuid(), msg.getVmUuid(),
5519+
agentErr.getDetails()));
54975520
}
54985521
bus.reply(msg, reply);
54995522
}
@@ -5506,13 +5529,34 @@ public Class<KVMAgentCommands.SecretHostDeleteResponse> getReturnClass() {
55065529
}
55075530

55085531
private ErrorCode buildSecretAgentError(KVMAgentCommands.AgentResponse rsp, String defaultMessage) {
5509-
if (rsp != null && rsp.getError() != null) {
5532+
String raw = rsp != null ? StringUtils.trimToNull(rsp.getError()) : null;
5533+
if (raw == null) {
5534+
return operr(defaultMessage);
5535+
}
5536+
String stable = stableKeyAgentErrorCodeFromRawMessage(raw);
5537+
if (stable != null) {
55105538
ErrorCode err = new ErrorCode();
5511-
err.setCode(rsp.getError());
5512-
err.setDetails(rsp.getError());
5539+
err.setCode(stable);
5540+
err.setDetails(raw);
55135541
return err;
55145542
}
5515-
return operr(defaultMessage);
5543+
return operr("%s: %s", defaultMessage, raw);
5544+
}
5545+
5546+
private static String stableKeyAgentErrorCodeFromRawMessage(String raw) {
5547+
String t = raw.trim();
5548+
int colon = t.indexOf(':');
5549+
String head = (colon >= 0 ? t.substring(0, colon) : t).trim();
5550+
if (head.isEmpty()) {
5551+
return null;
5552+
}
5553+
if (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(head)) {
5554+
return head;
5555+
}
5556+
if (head.startsWith("KEY_AGENT_")) {
5557+
return head;
5558+
}
5559+
return null;
55165560
}
55175561

55185562
@Override

‎plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import org.apache.commons.lang.StringUtils;
44
import org.springframework.beans.factory.annotation.Autowired;
5+
import org.zstack.compute.vm.VmGlobalConfig;
56
import org.zstack.compute.vm.devices.TpmEncryptedResourceKeyBackend;
67
import org.zstack.compute.vm.devices.TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext;
78
import org.zstack.core.Platform;
@@ -42,7 +43,9 @@
4243
import org.zstack.header.vm.VmInstanceInventory;
4344
import org.zstack.header.vm.VmInstanceMigrateExtensionPoint;
4445
import org.zstack.header.vm.VmInstanceSpec;
46+
import org.zstack.header.vm.VmInstanceState;
4547
import org.zstack.header.vm.VmInstantiateResourceException;
48+
import org.zstack.header.vm.VmStateChangedExtensionPoint;
4649
import org.zstack.header.vm.additions.VmHostBackupFileVO;
4750
import org.zstack.header.vm.additions.VmHostBackupFileVO_;
4851
import org.zstack.header.vm.VmInstanceVO;
@@ -69,7 +72,8 @@
6972
public class KvmTpmExtensions implements KVMStartVmExtensionPoint,
7073
PreVmInstantiateResourceExtensionPoint,
7174
VmInstanceMigrateExtensionPoint,
72-
VmAfterExpungeExtensionPoint {
75+
VmAfterExpungeExtensionPoint,
76+
VmStateChangedExtensionPoint {
7377
private static final CLogger logger = Utils.getLogger(KvmTpmExtensions.class);
7478

7579
@Autowired
@@ -220,7 +224,6 @@ public void run(FlowTrigger trigger, Map data) {
220224
@Override
221225
public void success() {
222226
context.providerUuid = resourceKeyBackend.findKeyProviderUuidByTpm(context.tpmUuid);
223-
context.providerName = resourceKeyBackend.findKeyProviderNameByTpm(context.tpmUuid);
224227
trigger.next();
225228
}
226229

@@ -322,9 +325,7 @@ public void run(MessageReply reply) {
322325
}
323326

324327
ErrorCode errorCode = reply.getError();
325-
if (errorCode != null
326-
&& (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode())
327-
|| SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getDetails()))) {
328+
if (errorCode != null && isVtpmSecretNotFoundOnHost(errorCode)) {
328329
trigger.next();
329330
return;
330331
}
@@ -527,6 +528,27 @@ public void afterMigrateVm(VmInstanceInventory inv, String srcHostUuid, NoErrorC
527528
completion.done();
528529
}
529530

531+
@Override
532+
public void vmStateChanged(VmInstanceInventory vm, VmInstanceState oldState, VmInstanceState newState) {
533+
if (oldState != VmInstanceState.VolumeMigrating) {
534+
return;
535+
}
536+
537+
String vmUuid = vm == null ? null : vm.getUuid();
538+
String srcHostUuid = vm == null ? null : vm.getLastHostUuid();
539+
String destHostUuid = vm == null ? null : vm.getHostUuid();
540+
if (StringUtils.isBlank(vmUuid) || StringUtils.isBlank(srcHostUuid) || srcHostUuid.equals(destHostUuid)) {
541+
return;
542+
}
543+
544+
if (!isVmCurrentlyOnExpectedHost(vmUuid, destHostUuid)) {
545+
return;
546+
}
547+
548+
Integer keyVersion = findTpmKeyVersionByVmUuid(vmUuid);
549+
deleteHostSecretBestEffort(srcHostUuid, vmUuid, keyVersion, "volume-migrated-host-change");
550+
}
551+
530552
@Override
531553
public void vmAfterExpunge(VmInstanceInventory vm) {
532554
String vmUuid = vm.getUuid();
@@ -572,8 +594,16 @@ private boolean isVmCurrentlyOnExpectedHost(String vmUuid, String expectedHostUu
572594
return expectedHostUuid.equals(currentHostUuid);
573595
}
574596

597+
private static boolean isVtpmSecretNotFoundOnHost(ErrorCode errorCode) {
598+
if (SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(errorCode.getCode())) {
599+
return true;
600+
}
601+
String details = errorCode.getDetails();
602+
return details != null && details.contains(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND);
603+
}
604+
575605
private void deleteHostSecretBestEffort(String hostUuid, String vmUuid, Integer keyVersion, String reason) {
576-
if (StringUtils.isBlank(hostUuid) || StringUtils.isBlank(vmUuid)) {
606+
if (StringUtils.isBlank(hostUuid) || StringUtils.isBlank(vmUuid) || keyVersion == null) {
577607
return;
578608
}
579609

@@ -587,9 +617,11 @@ private void deleteHostSecretBestEffort(String hostUuid, String vmUuid, Integer
587617
@Override
588618
public void run(MessageReply reply) {
589619
if (!reply.isSuccess()) {
620+
ErrorCode err = reply.getError();
621+
String errMsg = err != null && err.getDetails() != null ? err.getDetails() : "unknown error";
590622
logger.warn(String.format(
591623
"best-effort delete host secret failed on %s for vm[uuid:%s], host[uuid:%s]: %s",
592-
reason, vmUuid, hostUuid, reply.getError().getDetails()));
624+
reason, vmUuid, hostUuid, errMsg));
593625
}
594626
}
595627
});

‎plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,10 @@ public void done(ErrorCodeList errorCodeList) {
420420
@Override
421421
public void run(MessageReply reply) {
422422
if (!reply.isSuccess()) {
423+
ErrorCode err = reply.getError();
424+
String errMsg = err != null && err.getDetails() != null ? err.getDetails() : "unknown error";
423425
logger.warn(String.format("failed to delete host secret on host[uuid:%s] for vm[uuid:%s], continue cleanup: %s",
424-
hostUuid, context.vmInstanceUuid, reply.getError().getDetails()));
426+
hostUuid, context.vmInstanceUuid, errMsg));
425427
}
426428
whileCompletion.done();
427429
}
@@ -717,6 +719,10 @@ public void done(ErrorCodeList errorCodeList) {
717719
.build())
718720
.then(Flow.of("delete-host-secret")
719721
.handle(trigger -> {
722+
if (context.keyVersion == null) {
723+
trigger.next();
724+
return;
725+
}
720726
Set<String> hostUuids = new HashSet<>();
721727
for (VmHostFileVO file : context.hostFiles) {
722728
hostUuids.add(file.getHostUuid());
@@ -740,8 +746,10 @@ public void done(ErrorCodeList errorCodeList) {
740746
@Override
741747
public void run(MessageReply reply) {
742748
if (!reply.isSuccess()) {
749+
ErrorCode err = reply.getError();
750+
String errMsg = err != null && err.getDetails() != null ? err.getDetails() : "unknown error";
743751
logger.warn(String.format("failed to delete host secret on host[uuid:%s] for vm[uuid:%s], continue reset: %s",
744-
hostUuid, vmUuid, reply.getError().getDetails()));
752+
hostUuid, vmUuid, errMsg));
745753
}
746754
whileCompletion.done();
747755
}

0 commit comments

Comments
 (0)