This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new bb0c1f93afe Add volume encryption checks during the disk offering
change (#9209)
bb0c1f93afe is described below
commit bb0c1f93afea55a7e86240e5e5233205fb74abe9
Author: Harikrishna <[email protected]>
AuthorDate: Mon Jun 17 14:06:47 2024 +0530
Add volume encryption checks during the disk offering change (#9209)
---
.../subsystem/api/storage/VolumeService.java | 2 +
.../storage/volume/VolumeServiceImpl.java | 15 ++++++++
.../storage/volume/VolumeServiceTest.java | 43 ++++++++++++++++++++++
.../com/cloud/storage/VolumeApiServiceImpl.java | 4 +-
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 7 +---
.../java/com/cloud/vm/UserVmManagerImplTest.java | 42 ---------------------
6 files changed, 64 insertions(+), 49 deletions(-)
diff --git
a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java
b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java
index 7c4d56e12b9..682473ec94f 100644
---
a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java
+++
b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java
@@ -121,4 +121,6 @@ public interface VolumeService {
Pair<String, String> checkAndRepairVolume(VolumeInfo volume);
void checkAndRepairVolumeBasedOnConfig(DataObject dataObject, Host host);
+
+ void validateChangeDiskOfferingEncryptionType(long existingDiskOfferingId,
long newDiskOfferingId);
}
diff --git
a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
index 59d027e3c42..a47cb41a323 100644
---
a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
+++
b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
@@ -32,7 +32,10 @@ import java.util.concurrent.ExecutionException;
import javax.inject.Inject;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.VolumeApiServiceImpl;
+import com.cloud.storage.dao.DiskOfferingDao;
import org.apache.cloudstack.annotation.AnnotationService;
import org.apache.cloudstack.annotation.dao.AnnotationDao;
import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd;
@@ -212,6 +215,8 @@ public class VolumeServiceImpl implements VolumeService {
private SnapshotApiService snapshotApiService;
@Inject
private PassphraseDao passphraseDao;
+ @Inject
+ protected DiskOfferingDao diskOfferingDao;
public VolumeServiceImpl() {
}
@@ -2794,6 +2799,16 @@ public class VolumeServiceImpl implements VolumeService {
}
}
+ @Override
+ public void validateChangeDiskOfferingEncryptionType(long
existingDiskOfferingId, long newDiskOfferingId) {
+ DiskOfferingVO existingDiskOffering =
diskOfferingDao.findByIdIncludingRemoved(existingDiskOfferingId);
+ DiskOfferingVO newDiskOffering =
diskOfferingDao.findById(newDiskOfferingId);
+
+ if (existingDiskOffering.getEncrypt() != newDiskOffering.getEncrypt())
{
+ throw new InvalidParameterValueException("Cannot change the
encryption type of a volume, please check the selected offering");
+ }
+ }
+
@Override
public Pair<String, String> checkAndRepairVolume(VolumeInfo volume) {
Long poolId = volume.getPoolId();
diff --git
a/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceTest.java
b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceTest.java
index 55ff2f659af..97ccc37be13 100644
---
a/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceTest.java
+++
b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceTest.java
@@ -22,13 +22,16 @@ package org.apache.cloudstack.storage.volume;
import com.cloud.agent.api.storage.CheckAndRepairVolumeAnswer;
import com.cloud.agent.api.storage.CheckAndRepairVolumeCommand;
import com.cloud.agent.api.to.StorageFilerTO;
+import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.storage.CheckAndRepairVolumePayload;
+import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.snapshot.SnapshotManager;
import java.util.ArrayList;
@@ -88,6 +91,9 @@ public class VolumeServiceTest extends TestCase{
@Mock
HostDao hostDaoMock;
+ @Mock
+ DiskOfferingDao diskOfferingDaoMock;
+
@Before
public void setup(){
volumeServiceImplSpy = Mockito.spy(new VolumeServiceImpl());
@@ -96,6 +102,7 @@ public class VolumeServiceTest extends TestCase{
volumeServiceImplSpy.snapshotMgr = snapshotManagerMock;
volumeServiceImplSpy._storageMgr = storageManagerMock;
volumeServiceImplSpy._hostDao = hostDaoMock;
+ volumeServiceImplSpy.diskOfferingDao = diskOfferingDaoMock;
}
@Test(expected = InterruptedException.class)
@@ -303,4 +310,40 @@ public class VolumeServiceTest extends TestCase{
Assert.assertEquals(null, result);
}
+
+ @Test
+ public void validateDiskOfferingCheckForEncryption1Test() {
+ prepareOfferingsForEncryptionValidation(1L, true);
+ prepareOfferingsForEncryptionValidation(2L, true);
+ volumeServiceImplSpy.validateChangeDiskOfferingEncryptionType(1L, 2L);
+ }
+
+ @Test
+ public void validateDiskOfferingCheckForEncryption2Test() {
+ prepareOfferingsForEncryptionValidation(1L, false);
+ prepareOfferingsForEncryptionValidation(2L, false);
+ volumeServiceImplSpy.validateChangeDiskOfferingEncryptionType(1L, 2L);
+ }
+
+ @Test (expected = InvalidParameterValueException.class)
+ public void validateDiskOfferingCheckForEncryptionFail1Test() {
+ prepareOfferingsForEncryptionValidation(1L, false);
+ prepareOfferingsForEncryptionValidation(2L, true);
+ volumeServiceImplSpy.validateChangeDiskOfferingEncryptionType(1L, 2L);
+ }
+
+ @Test (expected = InvalidParameterValueException.class)
+ public void validateDiskOfferingCheckForEncryptionFail2Test() {
+ prepareOfferingsForEncryptionValidation(1L, true);
+ prepareOfferingsForEncryptionValidation(2L, false);
+ volumeServiceImplSpy.validateChangeDiskOfferingEncryptionType(1L, 2L);
+ }
+
+ private void prepareOfferingsForEncryptionValidation(long diskOfferingId,
boolean encryption) {
+ DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
+
+ Mockito.when(diskOffering.getEncrypt()).thenReturn(encryption);
+
Mockito.when(diskOfferingDaoMock.findByIdIncludingRemoved(diskOfferingId)).thenReturn(diskOffering);
+
Mockito.when(diskOfferingDaoMock.findById(diskOfferingId)).thenReturn(diskOffering);
+ }
}
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 1cf069feae8..31778b33b24 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -2027,7 +2027,8 @@ public class VolumeApiServiceImpl extends ManagerBase
implements VolumeApiServic
}
private Volume changeDiskOfferingForVolumeInternal(VolumeVO volume, Long
newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean
autoMigrateVolume, boolean shrinkOk) throws ResourceAllocationException {
- DiskOfferingVO existingDiskOffering =
_diskOfferingDao.findById(volume.getDiskOfferingId());
+ long existingDiskOfferingId = volume.getDiskOfferingId();
+ DiskOfferingVO existingDiskOffering =
_diskOfferingDao.findByIdIncludingRemoved(existingDiskOfferingId);
DiskOfferingVO newDiskOffering =
_diskOfferingDao.findById(newDiskOfferingId);
Integer newHypervisorSnapshotReserve = null;
@@ -2039,6 +2040,7 @@ public class VolumeApiServiceImpl extends ManagerBase
implements VolumeApiServic
Long[] updateNewMinIops = {newMinIops};
Long[] updateNewMaxIops = {newMaxIops};
Integer[] updateNewHypervisorSnapshotReserve =
{newHypervisorSnapshotReserve};
+
volService.validateChangeDiskOfferingEncryptionType(existingDiskOfferingId,
newDiskOfferingId);
validateVolumeResizeWithNewDiskOfferingAndLoad(volume,
existingDiskOffering, newDiskOffering, updateNewSize, updateNewMinIops,
updateNewMaxIops, updateNewHypervisorSnapshotReserve);
newSize = updateNewSize[0];
newMinIops = updateNewMinIops[0];
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 0c8ab1cd793..7fd074e13e8 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -2116,12 +2116,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
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");
- }
+
_volService.validateChangeDiskOfferingEncryptionType(currentServiceOffering.getDiskOfferingId(),
newServiceOffering.getDiskOfferingId());
}
private void changeDiskOfferingForRootVolume(Long vmId, DiskOfferingVO
newDiskOffering, Map<String, String> customParameters, Long zoneId) 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 303a9b08b1c..ba9e5ba6922 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -677,34 +677,6 @@ 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);
@@ -728,20 +700,6 @@ 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;
