Skip to content

<fix>[storage]: fix encrypted data volume host selection #4299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ZStack-Robot wants to merge 1 commit into from
Closed

<fix>[storage]: fix encrypted data volume host selection #4299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions header/src/main/java/org/zstack/header/storage/primary/PSCapacityExtensionPoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
public interface PSCapacityExtensionPoint {
String buildAllocatedInstallUrl(AllocatePrimaryStorageSpaceMsg msg, PrimaryStorageInventory psInv);

default String getHostUuidFromAllocatedInstallUrl(String allocatedInstallUrl) {
return null;
}

@Transactional(propagation = Propagation.MANDATORY)
long reserveCapacity(AllocatePrimaryStorageSpaceMsg msg, String allocatedInstallUrl, long size, String psUuid);

Expand Down
27 changes: 23 additions & 4 deletions plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1794,9 +1794,25 @@ private void createEmptyVolume(final InstantiateVolumeOnPrimaryStorageMsg msg) {
// Encrypted variant: forward to the dest KVM host so qemu-img can open
// a one-shot key-agent channel from encryptedDek.
if (Boolean.TRUE.equals(msg.getVolume().getEncrypted())) {
if (msg.getDestHost() == null || StringUtils.isBlank(msg.getDestHost().getUuid())) {
HostInventory destHost = msg.getDestHost();
String destHostUuid = destHost == null ? null : destHost.getUuid();
if (StringUtils.isBlank(destHostUuid)) {
try {
destHostUuid = findConnectedHostForCephLuks();
HostVO selectedHost = dbf.findByUuid(destHostUuid, HostVO.class);
if (selectedHost != null) {
msg.setDestHost(HostInventory.valueOf(selectedHost));
}
} catch (OperationFailureException e) {
reply.setError(e.getErrorCode());
bus.reply(msg, reply);
return;
}
}
if (StringUtils.isBlank(destHostUuid)) {
reply.setError(operr(
"ceph LUKS createempty requires destHost; volume[uuid:%s] has none", volumeUuid));
"ceph LUKS createempty requires a connected KVM host attached to primary storage[uuid:%s]; volume[uuid:%s] has none",
self.getUuid(), volumeUuid));
bus.reply(msg, reply);
return;
}
Expand All @@ -1805,8 +1821,8 @@ private void createEmptyVolume(final InstantiateVolumeOnPrimaryStorageMsg msg) {
kcmd.installPath = installPath;
kcmd.size = volumeSize;
kcmd.encryptedDek = volumeEncryptedSecretHelper.prepareLuksEnvelopeDekOnHost(
msg.getDestHost().getUuid(), volumeUuid);
httpCallToKvmHost(msg.getDestHost().getUuid(),
destHostUuid, volumeUuid);
httpCallToKvmHost(destHostUuid,
KVM_HOST_LUKS_CREATE_EMPTY_PATH, kcmd, KVMHostLuksRsp.class,
new ReturnValueCompletion<KVMHostLuksRsp>(msg) {
@Override
Expand Down Expand Up @@ -5572,8 +5588,11 @@ private String findConnectedHostForCephLuks() {
.listValues();
String hostUuid = connectedHostUuids.isEmpty() ? null : Q.New(HostVO.class)
.eq(HostVO_.hypervisorType, KVMConstant.KVM_HYPERVISOR_TYPE)
.eq(HostVO_.status, HostStatus.Connected)
.eq(HostVO_.state, HostState.Enabled)
.in(HostVO_.uuid, connectedHostUuids)
.select(HostVO_.uuid)
.orderBy(HostVO_.uuid, SimpleQuery.Od.ASC)
.limit(1)
.findValue();
if (StringUtils.isBlank(hostUuid)) {
Expand Down
5 changes: 5 additions & 0 deletions ...lstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageAllocatorFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ public String buildAllocatedInstallUrl(AllocatePrimaryStorageSpaceMsg msg, Prima
return path.makeFullPath();
}

@Override
public String getHostUuidFromAllocatedInstallUrl(String allocatedInstallUrl) {
return LocalStorageUtils.getHostUuidFromInstallUrl(allocatedInstallUrl);
}

public static String getHostUuidFromURIScheme(String uri) {
String protocol;
try {
Expand Down
5 changes: 0 additions & 5 deletions storage/src/main/java/org/zstack/storage/encrypt/VolumeEncryptedInitialExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ public class VolumeEncryptedInitialExtension implements PreInstantiateVolumeExte

@Override
public void preInstantiateVolume(InstantiateVolumeMsg msg) {
String hostUuid = msg.getHostUuid();
if (StringUtils.isBlank(hostUuid)) {
return;
}

String volUuid = msg.getVolumeUuid();
VolumeVO volume = dbf.findByUuid(volUuid, VolumeVO.class);

Expand Down
95 changes: 87 additions & 8 deletions storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.zstack.header.storage.snapshot.*;
import org.zstack.header.storage.snapshot.group.VolumeSnapshotGroupVO;
import org.zstack.header.storage.snapshot.group.VolumeSnapshotGroupVO_;
import org.zstack.storage.encrypt.VolumeEncryptedResourceKeyBackend;
import org.zstack.storage.encrypt.VolumeSnapshotEncryptionHelper;
import org.zstack.header.vm.*;
import org.zstack.header.vm.devices.VmInstanceResourceMetadataManager;
Expand Down Expand Up @@ -97,6 +98,8 @@ public class VolumeManagerImpl extends AbstractService implements VolumeManager,
private VolumeInPlaceEncryptor volumeInPlaceEncryptor;
@Autowired
private VolumeSnapshotEncryptionHelper snapshotEncryptionHelper;
@Autowired
private VolumeEncryptedResourceKeyBackend volumeEncryptedResourceKeyBackend;

private Future<Void> volumeExpungeTask;

Expand Down Expand Up @@ -292,6 +295,8 @@ protected VolumeVO scripts() {
String volumeFormat;
String volumeProtocol;
String allocatedInstallUrl;
String selectedHostUuid;
boolean encryptInPlaceRequired;

@Override
public void setup() {
Expand Down Expand Up @@ -367,6 +372,23 @@ public void run(MessageReply reply) {
AllocatePrimaryStorageSpaceReply ar = (AllocatePrimaryStorageSpaceReply) reply;
allocatedInstallUrl = ar.getAllocatedInstallUrl();
targetPrimaryStorage = ar.getPrimaryStorageInventory();
encryptInPlaceRequired = requiresEncryptInPlace(msg);
selectedHostUuid = msg.getHostUuid();
String allocatedHostUuid = getHostUuidFromAllocatedInstallUrl(
targetPrimaryStorage.getType(), allocatedInstallUrl);
if (StringUtils.isNotBlank(allocatedHostUuid)) {
selectedHostUuid = allocatedHostUuid;
}
if (encryptInPlaceRequired && StringUtils.isBlank(selectedHostUuid)) {
HostInventory host = selectHostForEncryptInPlace(targetPrimaryStorage.getUuid());
if (host == null) {
trigger.fail(operr(
"cannot encrypt volume[uuid:%s] in place: no connected KVM host found for primary storage[uuid:%s]",
vol.getUuid(), targetPrimaryStorage.getUuid()));
return;
}
selectedHostUuid = host.getUuid();
}
trigger.next();
}
});
Expand Down Expand Up @@ -398,7 +420,7 @@ public void run(FlowTrigger trigger, Map data) {
gmsg.setVolumeUuid(vol.getUuid());
gmsg.setBackupStorageRef(ImageBackupStorageRefInventory.valueOf(targetBackupStorageRef));
gmsg.setImage(ImageInventory.valueOf(template));
gmsg.setHostUuid(msg.getHostUuid());
gmsg.setHostUuid(selectedHostUuid);
gmsg.setAllocatedInstallUrl(allocatedInstallUrl);
bus.makeTargetServiceIdByResourceUuid(gmsg, PrimaryStorageConstant.SERVICE_ID, targetPrimaryStorage.getUuid());
bus.send(gmsg, new CloudBusCallBack(trigger) {
Expand Down Expand Up @@ -428,7 +450,7 @@ public void run(final FlowTrigger trigger, Map data) {
dmsg.setVolumeUuid(vol.getUuid());
dmsg.setBackupStorageRef(ImageBackupStorageRefInventory.valueOf(targetBackupStorageRef));
dmsg.setImage(ImageInventory.valueOf(template));
dmsg.setHostUuid(msg.getHostUuid());
dmsg.setHostUuid(selectedHostUuid);
dmsg.setAllocatedInstallUrl(allocatedInstallUrl);
bus.makeTargetServiceIdByResourceUuid(dmsg, PrimaryStorageConstant.SERVICE_ID, targetPrimaryStorage.getUuid());
bus.send(dmsg, new CloudBusCallBack(trigger) {
Expand Down Expand Up @@ -478,17 +500,13 @@ public void rollback(FlowRollback trigger, Map data) {

@Override
public boolean skip(Map data) {
// Template bits cloned from an encrypted source are already LUKS.
if (isTemplateFromEncryptedSource(msg.getImageUuid())) {
return true;
}
return !Boolean.TRUE.equals(msg.getEncrypted());
return !encryptInPlaceRequired;
}

@Override
public void run(FlowTrigger trigger, Map data) {
VolumeInPlaceEncryptor.Context ctx = new VolumeInPlaceEncryptor.Context()
.setHostUuid(msg.getHostUuid())
.setHostUuid(selectedHostUuid)
.setPrimaryStorageUuid(targetPrimaryStorage.getUuid())
.setInstallPath(primaryStorageInstallPath)
.setPurpose("create-data-volume-from-template");
Expand Down Expand Up @@ -701,6 +719,53 @@ protected VolumeVO scripts() {
return inv;
}

private boolean requiresEncryptInPlace(CreateDataVolumeFromVolumeTemplateMsg msg) {
return Boolean.TRUE.equals(msg.getEncrypted()) && !isTemplateFromEncryptedSource(msg.getImageUuid());
}

private String getHostUuidFromAllocatedInstallUrl(String primaryStorageType, String allocatedInstallUrl) {
if (StringUtils.isBlank(primaryStorageType) || StringUtils.isBlank(allocatedInstallUrl)) {
return null;
}

PSCapacityExtensionPoint ext = pluginRgty.getExtensionFromMap(primaryStorageType, PSCapacityExtensionPoint.class);
return ext == null ? null : ext.getHostUuidFromAllocatedInstallUrl(allocatedInstallUrl);
}

private HostInventory selectHostForEncryptInPlace(String primaryStorageUuid) {
SimpleQuery<PrimaryStorageClusterRefVO> clusterRefQuery = dbf.createQuery(PrimaryStorageClusterRefVO.class);
clusterRefQuery.select(PrimaryStorageClusterRefVO_.clusterUuid);
clusterRefQuery.add(PrimaryStorageClusterRefVO_.primaryStorageUuid, Op.EQ, primaryStorageUuid);
List<String> clusterUuids = clusterRefQuery.listValue();
if (clusterUuids.isEmpty()) {
return null;
}

SimpleQuery<PrimaryStorageHostRefVO> hostRefQuery = dbf.createQuery(PrimaryStorageHostRefVO.class);
hostRefQuery.add(PrimaryStorageHostRefVO_.primaryStorageUuid, Op.EQ, primaryStorageUuid);
List<PrimaryStorageHostRefVO> hostRefs = hostRefQuery.list();
List<String> connectedHostUuids = hostRefs.stream()
.filter(ref -> PrimaryStorageHostStatus.Connected == ref.getStatus())
.map(PrimaryStorageHostRefVO::getHostUuid)
.collect(Collectors.toList());
if (!hostRefs.isEmpty() && connectedHostUuids.isEmpty()) {
return null;
}

SimpleQuery<HostVO> hostQuery = dbf.createQuery(HostVO.class);
hostQuery.add(HostVO_.clusterUuid, Op.IN, clusterUuids);
if (!connectedHostUuids.isEmpty()) {
hostQuery.add(HostVO_.uuid, Op.IN, connectedHostUuids);
}
hostQuery.add(HostVO_.hypervisorType, Op.EQ, VmInstanceConstant.KVM_HYPERVISOR_TYPE);
hostQuery.add(HostVO_.status, Op.EQ, HostStatus.Connected);
hostQuery.add(HostVO_.state, Op.EQ, HostState.Enabled);
hostQuery.orderBy(HostVO_.uuid, SimpleQuery.Od.ASC);
hostQuery.setLimit(1);
HostVO host = hostQuery.find();
return host == null ? null : HostInventory.valueOf(host);
}

private boolean isTemplateFromEncryptedSource(String imageUuid) {
if (StringUtils.isBlank(imageUuid)) {
return false;
Expand Down Expand Up @@ -1220,6 +1285,7 @@ protected VolumeVO scripts() {
public void run(MessageReply r) {
InstantiateVolumeReply cr = r.castReply();
if (!cr.isSuccess()) {
detachVolumeKeyProviderOnCreateFailure(finalVo);
dbf.remove(finalVo);
reply.setError(cr.getError());
} else {
Expand All @@ -1230,6 +1296,19 @@ public void run(MessageReply r) {
});
}

private void detachVolumeKeyProviderOnCreateFailure(VolumeVO volume) {
if (!volume.isEncrypted()) {
return;
}

try {
volumeEncryptedResourceKeyBackend.detachKeyProviderFromVolume(volume.getUuid());
} catch (Exception e) {
logger.warn(String.format("failed to detach key provider from volume[uuid:%s] after create failure",
volume.getUuid()), e);
}
}

private void handle(APICreateDataVolumeMsg msg) {
APICreateDataVolumeEvent evt = new APICreateDataVolumeEvent(msg.getId());
CreateDataVolumeMsg cmsg = new CreateDataVolumeMsg();
Expand Down
98 changes: 98 additions & 0 deletions test/src/test/groovy/org/zstack/storage/volume/VolumeManagerImplHostSelectionTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package org.zstack.storage.volume

import org.junit.Before
import org.junit.Test
import org.mockito.Mockito
import org.zstack.core.db.DatabaseFacade
import org.zstack.core.db.SimpleQuery
import org.zstack.header.host.HostInventory
import org.zstack.header.host.HostState
import org.zstack.header.host.HostStatus
import org.zstack.header.host.HostVO
import org.zstack.header.host.HostVO_
import org.zstack.header.storage.primary.PrimaryStorageClusterRefVO
import org.zstack.header.storage.primary.PrimaryStorageClusterRefVO_
import org.zstack.header.storage.primary.PrimaryStorageHostRefVO
import org.zstack.header.storage.primary.PrimaryStorageHostRefVO_
import org.zstack.header.storage.primary.PrimaryStorageHostStatus
import org.zstack.header.vm.VmInstanceConstant

import java.lang.reflect.Field
import java.lang.reflect.Method

class VolumeManagerImplHostSelectionTest {
private static final String PS_UUID = "ps-uuid"

private VolumeManagerImpl manager
private DatabaseFacade dbf
private SimpleQuery<PrimaryStorageClusterRefVO> clusterRefQuery
private SimpleQuery<PrimaryStorageHostRefVO> hostRefQuery
private SimpleQuery<HostVO> hostQuery

@Before
void setUp() {
manager = new VolumeManagerImpl()
dbf = Mockito.mock(DatabaseFacade.class)
clusterRefQuery = Mockito.mock(SimpleQuery.class)
hostRefQuery = Mockito.mock(SimpleQuery.class)
hostQuery = Mockito.mock(SimpleQuery.class)
setField("dbf", dbf)

Mockito.when(dbf.createQuery(PrimaryStorageClusterRefVO.class)).thenReturn(clusterRefQuery)
Mockito.when(dbf.createQuery(PrimaryStorageHostRefVO.class)).thenReturn(hostRefQuery)
Mockito.when(dbf.createQuery(HostVO.class)).thenReturn(hostQuery)
Mockito.when(clusterRefQuery.select(PrimaryStorageClusterRefVO_.clusterUuid)).thenReturn(clusterRefQuery)
Mockito.when(clusterRefQuery.add(PrimaryStorageClusterRefVO_.primaryStorageUuid, SimpleQuery.Op.EQ, PS_UUID)).thenReturn(clusterRefQuery)
Mockito.when(hostRefQuery.add(PrimaryStorageHostRefVO_.primaryStorageUuid, SimpleQuery.Op.EQ, PS_UUID)).thenReturn(hostRefQuery)
Mockito.when(hostQuery.add(HostVO_.clusterUuid, SimpleQuery.Op.IN, Arrays.asList("cluster-uuid"))).thenReturn(hostQuery)
Mockito.when(hostQuery.add(HostVO_.uuid, SimpleQuery.Op.IN, Arrays.asList("host-connected"))).thenReturn(hostQuery)
Mockito.when(hostQuery.add(HostVO_.hypervisorType, SimpleQuery.Op.EQ, VmInstanceConstant.KVM_HYPERVISOR_TYPE)).thenReturn(hostQuery)
Mockito.when(hostQuery.add(HostVO_.status, SimpleQuery.Op.EQ, HostStatus.Connected)).thenReturn(hostQuery)
Mockito.when(hostQuery.add(HostVO_.state, SimpleQuery.Op.EQ, HostState.Enabled)).thenReturn(hostQuery)
Mockito.when(hostQuery.orderBy(HostVO_.uuid, SimpleQuery.Od.ASC)).thenReturn(hostQuery)
Mockito.when(hostQuery.setLimit(1)).thenReturn(hostQuery)
}

@Test
void testSelectHostForEncryptInPlaceUsesConnectedPrimaryStorageHostRef() {
Mockito.when(clusterRefQuery.listValue()).thenReturn(Arrays.asList("cluster-uuid"))
Mockito.when(hostRefQuery.list()).thenReturn(Arrays.asList(
primaryStorageHostRef("host-disconnected", PrimaryStorageHostStatus.Disconnected),
primaryStorageHostRef("host-connected", PrimaryStorageHostStatus.Connected)
))
HostVO host = new HostVO()
host.uuid = "host-connected"
host.clusterUuid = "cluster-uuid"
host.name = "host"
host.managementIp = "172.20.0.10"
host.hypervisorType = VmInstanceConstant.KVM_HYPERVISOR_TYPE
host.status = HostStatus.Connected
host.state = HostState.Enabled
Mockito.when(hostQuery.find()).thenReturn(host)

HostInventory inventory = invokeSelectHostForEncryptInPlace()

assert inventory.uuid == "host-connected" : "create-from-template encrypt-in-place should stage the LUKS secret on a connected PS host ref: expected=host-connected actual=${inventory?.uuid}"
Mockito.verify(hostQuery).add(HostVO_.uuid, SimpleQuery.Op.IN, Arrays.asList("host-connected"))
}

private HostInventory invokeSelectHostForEncryptInPlace() {
Method method = VolumeManagerImpl.class.getDeclaredMethod("selectHostForEncryptInPlace", String.class)
method.accessible = true
return method.invoke(manager, PS_UUID) as HostInventory
}

private PrimaryStorageHostRefVO primaryStorageHostRef(String hostUuid, PrimaryStorageHostStatus status) {
PrimaryStorageHostRefVO ref = new PrimaryStorageHostRefVO()
ref.primaryStorageUuid = PS_UUID
ref.hostUuid = hostUuid
ref.status = status
return ref
}

private void setField(String name, Object value) {
Field field = VolumeManagerImpl.class.getDeclaredField(name)
field.accessible = true
field.set(manager, value)
}
}
Loading