Add volume encryption checks during the disk offering change (#9209)

This commit is contained in:
Harikrishna 2024-06-17 14:06:47 +05:30 committed by GitHub
parent 96288ecf1f
commit bb0c1f93af
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 64 additions and 49 deletions

View File

@ -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);
}

View File

@ -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();

View File

@ -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);
}
}

View File

@ -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];

View File

@ -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 {

View File

@ -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;