From fc56e176cb885a7ce598f1bb40b72c38a71f11ae Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 19 Sep 2022 14:20:28 -0600 Subject: [PATCH 1/2] Don't change service offering if encryption value would change Signed-off-by: Marcus Sorensen --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index e1397c4fbce6..3f4955d0ca3a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2052,6 +2052,13 @@ private void validateDiskOfferingChecks(ServiceOfferingVO currentServiceOffering 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 { From aa3e71bdcff54114b5ff3251468b07713776125a Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Wed, 28 Sep 2022 10:35:33 -0600 Subject: [PATCH 2/2] Add unit test for service offering encryption valudation Signed-off-by: Marcus Sorensen --- .../java/com/cloud/vm/UserVmManagerImpl.java | 2 +- .../com/cloud/vm/UserVmManagerImplTest.java | 42 +++++++++++++++++++ 2 files changed, 43 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 3f4955d0ca3a..0d4e2e983831 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2044,7 +2044,7 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI 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"); } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 22b7f223e6eb..ed210faa7aa7 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -551,6 +551,34 @@ public void prepareResizeVolumeCmdTestNewOfferingSmaller() { 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); @@ -573,4 +601,18 @@ private DiskOfferingVO prepareDiskOffering(long rootSize, long diskOfferingId, l Mockito.when(newRootDiskOffering.getName()).thenReturn("OfferingName"); 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; + } }