From 93f09265c3ad4916ef06c1278fa117d9fa91f00f Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Fri, 7 Oct 2022 00:10:44 -0600 Subject: [PATCH] server: Don't allow service offering change if encryption value would change (#6776) This PR blocks change of service offering if the offering root volume encryption values don't match. We don't support dynamically removing or adding encryption to a VM. Signed-off-by: Marcus Sorensen Co-authored-by: Marcus Sorensen --- .../java/com/cloud/vm/UserVmManagerImpl.java | 9 +++- .../com/cloud/vm/UserVmManagerImplTest.java | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 9829124f2e5..43fcd39c37b 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2092,7 +2092,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return success; } - private void validateDiskOfferingChecks(ServiceOfferingVO currentServiceOffering, ServiceOfferingVO newServiceOffering) { + protected void validateDiskOfferingChecks(ServiceOfferingVO currentServiceOffering, ServiceOfferingVO newServiceOffering) { if (currentServiceOffering.getDiskOfferingStrictness() != newServiceOffering.getDiskOfferingStrictness()) { throw new InvalidParameterValueException("Unable to Scale VM, since disk offering strictness flag is not same for new service offering and old service offering"); } @@ -2100,6 +2100,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (currentServiceOffering.getDiskOfferingStrictness() && currentServiceOffering.getDiskOfferingId() != newServiceOffering.getDiskOfferingId()) { throw new InvalidParameterValueException("Unable to Scale VM, since disk offering id associated with the old service offering is not same for new service offering"); } + + DiskOfferingVO currentRootDiskOffering = _diskOfferingDao.findByIdIncludingRemoved(currentServiceOffering.getDiskOfferingId()); + DiskOfferingVO newRootDiskOffering = _diskOfferingDao.findById(newServiceOffering.getDiskOfferingId()); + + if (currentRootDiskOffering.getEncrypt() != newRootDiskOffering.getEncrypt()) { + throw new InvalidParameterValueException("Cannot change volume encryption type via service offering change"); + } } private void changeDiskOfferingForRootVolume(Long vmId, DiskOfferingVO newDiskOffering, Map customParameters) throws ResourceAllocationException { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index d8219611253..2e6f72e67b3 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -565,6 +565,34 @@ public class UserVmManagerImplTest { prepareAndRunResizeVolumeTest(2L, 10L, 20L, largerDisdkOffering, smallerDisdkOffering); } + @Test + public void validateDiskOfferingCheckForEncryption1Test() { + ServiceOfferingVO currentOffering = prepareOfferingsForEncryptionValidation(1L, true); + ServiceOfferingVO newOffering = prepareOfferingsForEncryptionValidation(2L, true); + userVmManagerImpl.validateDiskOfferingChecks(currentOffering, newOffering); + } + + @Test + public void validateDiskOfferingCheckForEncryption2Test() { + ServiceOfferingVO currentOffering = prepareOfferingsForEncryptionValidation(1L, false); + ServiceOfferingVO newOffering = prepareOfferingsForEncryptionValidation(2L, false); + userVmManagerImpl.validateDiskOfferingChecks(currentOffering, newOffering); + } + + @Test (expected = InvalidParameterValueException.class) + public void validateDiskOfferingCheckForEncryptionFail1Test() { + ServiceOfferingVO currentOffering = prepareOfferingsForEncryptionValidation(1L, false); + ServiceOfferingVO newOffering = prepareOfferingsForEncryptionValidation(2L, true); + userVmManagerImpl.validateDiskOfferingChecks(currentOffering, newOffering); + } + + @Test (expected = InvalidParameterValueException.class) + public void validateDiskOfferingCheckForEncryptionFail2Test() { + ServiceOfferingVO currentOffering = prepareOfferingsForEncryptionValidation(1L, true); + ServiceOfferingVO newOffering = prepareOfferingsForEncryptionValidation(2L, false); + userVmManagerImpl.validateDiskOfferingChecks(currentOffering, newOffering); + } + private void prepareAndRunResizeVolumeTest(Long expectedOfferingId, long expectedMinIops, long expectedMaxIops, DiskOfferingVO currentRootDiskOffering, DiskOfferingVO newRootDiskOffering) { long rootVolumeId = 1l; VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class); @@ -588,6 +616,20 @@ public class UserVmManagerImplTest { return newRootDiskOffering; } + private ServiceOfferingVO prepareOfferingsForEncryptionValidation(long diskOfferingId, boolean encryption) { + ServiceOfferingVO svcOffering = Mockito.mock(ServiceOfferingVO.class); + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + + Mockito.when(svcOffering.getDiskOfferingId()).thenReturn(diskOfferingId); + Mockito.when(diskOffering.getEncrypt()).thenReturn(encryption); + + // Be aware - Multiple calls with the same disk offering ID could conflict + Mockito.when(diskOfferingDao.findByIdIncludingRemoved(diskOfferingId)).thenReturn(diskOffering); + Mockito.when(diskOfferingDao.findById(diskOfferingId)).thenReturn(diskOffering); + + return svcOffering; + } + @Test (expected = CloudRuntimeException.class) public void testUserDataDenyOverride() { Long userDataId = 1L;