From 1303a4f323dc21f23cff60a3ed83694f7d76d89c Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 9 Sep 2024 18:14:50 +0530 Subject: [PATCH] Feature: Allow adding delete protection for VMs & volumes (#9633) Co-authored-by: Suresh Kumar Anaparti --- .../main/java/com/cloud/storage/Volume.java | 10 +++-- .../com/cloud/storage/VolumeApiService.java | 4 +- .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/command/user/vm/UpdateVMCmd.java | 12 ++++++ .../command/user/volume/UpdateVolumeCmd.java | 14 ++++++- .../api/response/UserVmResponse.java | 12 ++++++ .../api/response/VolumeResponse.java | 12 ++++++ .../main/java/com/cloud/storage/VolumeVO.java | 12 ++++++ .../main/java/com/cloud/vm/VMInstanceVO.java | 14 +++++-- .../main/java/com/cloud/vm/dao/UserVmDao.java | 6 ++- .../java/com/cloud/vm/dao/UserVmDaoImpl.java | 8 +++- .../META-INF/db/schema-41910to42000.sql | 3 ++ .../META-INF/db/views/cloud.user_vm_view.sql | 1 + .../META-INF/db/views/cloud.volume_view.sql | 1 + .../storage/volume/VolumeObject.java | 5 +++ .../api/query/dao/UserVmJoinDaoImpl.java | 6 +++ .../api/query/dao/VolumeJoinDaoImpl.java | 6 +++ .../com/cloud/api/query/vo/UserVmJoinVO.java | 6 +++ .../com/cloud/api/query/vo/VolumeJoinVO.java | 7 ++++ .../cloud/storage/VolumeApiServiceImpl.java | 16 +++++++- .../main/java/com/cloud/vm/UserVmManager.java | 10 ++++- .../java/com/cloud/vm/UserVmManagerImpl.java | 37 ++++++++++++++++--- .../com/cloud/vm/UserVmManagerImplTest.java | 4 +- test/integration/smoke/test_vm_life_cycle.py | 28 ++++++++++++++ test/integration/smoke/test_volumes.py | 27 ++++++++++++++ tools/marvin/marvin/lib/base.py | 8 ++++ ui/public/locales/en.json | 1 + ui/src/config/section/compute.js | 3 +- ui/src/config/section/storage.js | 4 +- ui/src/views/compute/EditVM.vue | 11 ++++++ 30 files changed, 261 insertions(+), 28 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/Volume.java b/api/src/main/java/com/cloud/storage/Volume.java index 40c5660b2df2..c7fbdb0a5445 100644 --- a/api/src/main/java/com/cloud/storage/Volume.java +++ b/api/src/main/java/com/cloud/storage/Volume.java @@ -271,11 +271,13 @@ enum Event { void setExternalUuid(String externalUuid); - public Long getPassphraseId(); + Long getPassphraseId(); - public void setPassphraseId(Long id); + void setPassphraseId(Long id); - public String getEncryptFormat(); + String getEncryptFormat(); - public void setEncryptFormat(String encryptFormat); + void setEncryptFormat(String encryptFormat); + + boolean isDeleteProtection(); } diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java index f9cba14679e0..6f4c7aa09e26 100644 --- a/api/src/main/java/com/cloud/storage/VolumeApiService.java +++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java @@ -117,7 +117,9 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List zoneIds) throws ResourceAllocationException; - Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, String customId, long owner, String chainInfo, String name); + Volume updateVolume(long volumeId, String path, String state, Long storageId, + Boolean displayVolume, Boolean deleteProtection, + String customId, long owner, String chainInfo, String name); /** * Extracts the volume to a particular location. diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index ad953e978678..bb16b0ff90de 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -138,6 +138,7 @@ public class ApiConstants { public static final String DATACENTER_NAME = "datacentername"; public static final String DATADISK_OFFERING_LIST = "datadiskofferinglist"; public static final String DEFAULT_VALUE = "defaultvalue"; + public static final String DELETE_PROTECTION = "deleteprotection"; public static final String DESCRIPTION = "description"; public static final String DESTINATION = "destination"; public static final String DESTINATION_ZONE_ID = "destzoneid"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index e5d625fe70d5..0f5dade96d25 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -146,6 +146,14 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction, @Parameter(name = ApiConstants.EXTRA_CONFIG, type = CommandType.STRING, since = "4.12", description = "an optional URL encoded string that can be passed to the virtual machine upon successful deployment", length = 5120) private String extraConfig; + @Parameter(name = ApiConstants.DELETE_PROTECTION, + type = CommandType.BOOLEAN, since = "4.20.0", + description = "Set delete protection for the virtual machine. If " + + "true, the instance will be protected from deletion. " + + "Note: If the instance is managed by another service like" + + " autoscaling groups or CKS, delete protection will be ignored.") + private Boolean deleteProtection; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -215,6 +223,10 @@ public boolean isCleanupDetails(){ return cleanupDetails == null ? false : cleanupDetails.booleanValue(); } + public Boolean getDeleteProtection() { + return deleteProtection; + } + public Map> getDhcpOptionsMap() { Map> dhcpOptionsMap = new HashMap<>(); if (dhcpOptionsNetworkList != null && !dhcpOptionsNetworkList.isEmpty()) { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java index 467c587cc731..22b819c8cba2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java @@ -77,6 +77,14 @@ public class UpdateVolumeCmd extends BaseAsyncCustomIdCmd implements UserCmd { @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "new name of the volume", since = "4.16") private String name; + @Parameter(name = ApiConstants.DELETE_PROTECTION, + type = CommandType.BOOLEAN, since = "4.20.0", + description = "Set delete protection for the volume. If true, The volume " + + "will be protected from deletion. Note: If the volume is managed by " + + "another service like autoscaling groups or CKS, delete protection will be " + + "ignored.") + private Boolean deleteProtection; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -109,6 +117,10 @@ public String getName() { return name; } + public Boolean getDeleteProtection() { + return deleteProtection; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -168,7 +180,7 @@ public String getEventDescription() { public void execute() { CallContext.current().setEventDetails("Volume Id: " + this._uuidMgr.getUuid(Volume.class, getId())); Volume result = _volumeService.updateVolume(getId(), getPath(), getState(), getStorageId(), getDisplayVolume(), - getCustomId(), getEntityOwnerId(), getChainInfo(), getName()); + getDeleteProtection(), getCustomId(), getEntityOwnerId(), getChainInfo(), getName()); if (result != null) { VolumeResponse response = _responseGenerator.createVolumeResponse(getResponseView(), result); response.setResponseName(getCommandName()); diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java index df9a474213b4..1f4b493fba2f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java @@ -320,6 +320,10 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co @Param(description = "true if vm contains XS/VMWare tools inorder to support dynamic scaling of VM cpu/memory.") private Boolean isDynamicallyScalable; + @SerializedName(ApiConstants.DELETE_PROTECTION) + @Param(description = "true if vm has delete protection.", since = "4.20.0") + private boolean deleteProtection; + @SerializedName(ApiConstants.SERVICE_STATE) @Param(description = "State of the Service from LB rule") private String serviceState; @@ -995,6 +999,14 @@ public void setDynamicallyScalable(Boolean dynamicallyScalable) { isDynamicallyScalable = dynamicallyScalable; } + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } + public String getOsTypeId() { return osTypeId; } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java index 4ac17e9832b9..209ca57c50d2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java @@ -261,6 +261,10 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co @Param(description = "true if storage snapshot is supported for the volume, false otherwise", since = "4.16") private boolean supportsStorageSnapshot; + @SerializedName(ApiConstants.DELETE_PROTECTION) + @Param(description = "true if volume has delete protection.", since = "4.20.0") + private boolean deleteProtection; + @SerializedName(ApiConstants.PHYSICAL_SIZE) @Param(description = "the bytes actually consumed on disk") private Long physicalsize; @@ -584,6 +588,14 @@ public boolean getSupportsStorageSnapshot() { return this.supportsStorageSnapshot; } + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } + public String getIsoId() { return isoId; } diff --git a/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java b/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java index e12859ea8d6c..c105acf40b8d 100644 --- a/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java @@ -182,6 +182,9 @@ public class VolumeVO implements Volume { @Column(name = "encrypt_format") private String encryptFormat; + @Column(name = "delete_protection") + private boolean deleteProtection; + // Real Constructor public VolumeVO(Type type, String name, long dcId, long domainId, @@ -678,4 +681,13 @@ public void setExternalUuid(String externalUuid) { public String getEncryptFormat() { return encryptFormat; } public void setEncryptFormat(String encryptFormat) { this.encryptFormat = encryptFormat; } + + @Override + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } } diff --git a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java index a1600e04350e..a1d9f4a8089a 100644 --- a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java +++ b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java @@ -167,10 +167,8 @@ public class VMInstanceVO implements VirtualMachine, FiniteStateObject details; @@ -542,6 +540,14 @@ public boolean isDynamicallyScalable() { return dynamicallyScalable; } + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } + @Override public Class getEntityType() { return VirtualMachine.class; diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java index 39c65866658f..7de543e69d31 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java @@ -53,7 +53,11 @@ public interface UserVmDao extends GenericDao { * @param hostName TODO * @param instanceName */ - void updateVM(long id, String displayName, boolean enable, Long osTypeId, String userData, Long userDataId, String userDataDetails, boolean displayVm, boolean isDynamicallyScalable, String customId, String hostName, String instanceName); + void updateVM(long id, String displayName, boolean enable, Long osTypeId, + String userData, Long userDataId, String userDataDetails, + boolean displayVm, boolean isDynamicallyScalable, + boolean deleteProtection, String customId, String hostName, + String instanceName); List findDestroyedVms(Date date); diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java index 536779125e24..cc8b9fc59a8d 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java @@ -274,8 +274,11 @@ public List listByAccountAndDataCenter(long accountId, long dcId) { } @Override - public void updateVM(long id, String displayName, boolean enable, Long osTypeId, String userData, Long userDataId, String userDataDetails, boolean displayVm, - boolean isDynamicallyScalable, String customId, String hostName, String instanceName) { + public void updateVM(long id, String displayName, boolean enable, Long osTypeId, + String userData, Long userDataId, String userDataDetails, + boolean displayVm, boolean isDynamicallyScalable, + boolean deleteProtection, String customId, String hostName, + String instanceName) { UserVmVO vo = createForUpdate(); vo.setDisplayName(displayName); vo.setHaEnabled(enable); @@ -285,6 +288,7 @@ public void updateVM(long id, String displayName, boolean enable, Long osTypeId, vo.setUserDataDetails(userDataDetails); vo.setDisplayVm(displayVm); vo.setDynamicallyScalable(isDynamicallyScalable); + vo.setDeleteProtection(deleteProtection); if (hostName != null) { vo.setHostName(hostName); } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql b/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql index ac7d79368333..8f0076ad46ba 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql @@ -620,3 +620,6 @@ INSERT IGNORE INTO `cloud`.`hypervisor_capabilities` (uuid, hypervisor_type, hyp INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) SELECT UUID(),'VMware', '8.0.2', guest_os_name, guest_os_id, utc_timestamp(), 0 FROM `cloud`.`guest_os_hypervisor` WHERE hypervisor_type='VMware' AND hypervisor_version='8.0'; INSERT IGNORE INTO `cloud`.`hypervisor_capabilities` (uuid, hypervisor_type, hypervisor_version, max_guests_limit, security_group_enabled, max_data_volumes_limit, max_hosts_per_cluster, storage_motion_supported, vm_snapshot_enabled) values (UUID(), 'VMware', '8.0.3', 1024, 0, 59, 64, 1, 1); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) SELECT UUID(),'VMware', '8.0.3', guest_os_name, guest_os_id, utc_timestamp(), 0 FROM `cloud`.`guest_os_hypervisor` WHERE hypervisor_type='VMware' AND hypervisor_version='8.0'; + +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vm_instance', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for vm" '); +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for volumes" '); diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql index de48272006dc..97cb7b735cfc 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql @@ -54,6 +54,7 @@ SELECT `vm_instance`.`instance_name` AS `instance_name`, `vm_instance`.`guest_os_id` AS `guest_os_id`, `vm_instance`.`display_vm` AS `display_vm`, + `vm_instance`.`delete_protection` AS `delete_protection`, `guest_os`.`uuid` AS `guest_os_uuid`, `vm_instance`.`pod_id` AS `pod_id`, `host_pod_ref`.`uuid` AS `pod_uuid`, diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql index 950dcddf4c71..ffeb93e8fa7a 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql @@ -40,6 +40,7 @@ SELECT `volumes`.`chain_info` AS `chain_info`, `volumes`.`external_uuid` AS `external_uuid`, `volumes`.`encrypt_format` AS `encrypt_format`, + `volumes`.`delete_protection` AS `delete_protection`, `account`.`id` AS `account_id`, `account`.`uuid` AS `account_uuid`, `account`.`account_name` AS `account_name`, diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 1b3bec0e9075..825a8cbd941c 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -935,6 +935,11 @@ public void setEncryptFormat(String encryptFormat) { volumeVO.setEncryptFormat(encryptFormat); } + @Override + public boolean isDeleteProtection() { + return volumeVO.isDeleteProtection(); + } + @Override public boolean isFollowRedirects() { return followRedirects; diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index bd97178e8a85..af26a242db47 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -426,6 +426,12 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us userVmResponse.setDynamicallyScalable(userVm.isDynamicallyScalable()); } + if (userVm.getDeleteProtection() == null) { + userVmResponse.setDeleteProtection(false); + } else { + userVmResponse.setDeleteProtection(userVm.getDeleteProtection()); + } + if (userVm.getAutoScaleVmGroupName() != null) { userVmResponse.setAutoScaleVmGroupName(userVm.getAutoScaleVmGroupName()); } diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 2becc05eaa01..4f5d984c969a 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -138,6 +138,12 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume) volResponse.setMinIops(volume.getMinIops()); volResponse.setMaxIops(volume.getMaxIops()); + if (volume.getDeleteProtection() == null) { + volResponse.setDeleteProtection(false); + } else { + volResponse.setDeleteProtection(volume.getDeleteProtection()); + } + volResponse.setCreated(volume.getCreated()); if (volume.getState() != null) { volResponse.setState(volume.getState().toString()); diff --git a/server/src/main/java/com/cloud/api/query/vo/UserVmJoinVO.java b/server/src/main/java/com/cloud/api/query/vo/UserVmJoinVO.java index accf8e688d9d..701fa7d4f826 100644 --- a/server/src/main/java/com/cloud/api/query/vo/UserVmJoinVO.java +++ b/server/src/main/java/com/cloud/api/query/vo/UserVmJoinVO.java @@ -436,6 +436,9 @@ public class UserVmJoinVO extends BaseViewWithTagInformationVO implements Contro @Column(name = "dynamically_scalable") private boolean isDynamicallyScalable; + @Column(name = "delete_protection") + protected Boolean deleteProtection; + public UserVmJoinVO() { // Empty constructor @@ -946,6 +949,9 @@ public Boolean isDynamicallyScalable() { return isDynamicallyScalable; } + public Boolean getDeleteProtection() { + return deleteProtection; + } @Override public Class getEntityType() { diff --git a/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java b/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java index 79f558a3ef50..2ae720fa8524 100644 --- a/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java +++ b/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java @@ -280,6 +280,9 @@ public class VolumeJoinVO extends BaseViewWithTagInformationVO implements Contro @Column(name = "encrypt_format") private String encryptionFormat = null; + @Column(name = "delete_protection") + protected Boolean deleteProtection; + public VolumeJoinVO() { } @@ -619,6 +622,10 @@ public String getEncryptionFormat() { return encryptionFormat; } + public Boolean getDeleteProtection() { + return deleteProtection; + } + @Override public Class getEntityType() { return Volume.class; diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index a392cecbeb4f..0f215874c3ae 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1699,6 +1699,12 @@ public boolean stateTransitTo(Volume vol, Volume.Event event) throws NoTransitio } public void validateDestroyVolume(Volume volume, Account caller, boolean expunge, boolean forceExpunge) { + if (volume.isDeleteProtection()) { + throw new InvalidParameterValueException(String.format( + "Volume [id = %s, name = %s] has delete protection enabled and cannot be deleted.", + volume.getUuid(), volume.getName())); + } + if (expunge) { // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVolume is false for the caller. final Long userId = caller.getAccountId(); @@ -2757,13 +2763,15 @@ protected String createVolumeInfoFromVolumes(List vmVolumes) { @Override @ActionEvent(eventType = EventTypes.EVENT_VOLUME_UPDATE, eventDescription = "updating volume", async = true) - public Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, + public Volume updateVolume(long volumeId, String path, String state, Long storageId, + Boolean displayVolume, Boolean deleteProtection, String customId, long entityOwnerId, String chainInfo, String name) { Account caller = CallContext.current().getCallingAccount(); if (!_accountMgr.isRootAdmin(caller.getId())) { if (path != null || state != null || storageId != null || displayVolume != null || customId != null || chainInfo != null) { - throw new InvalidParameterValueException("The domain admin and normal user are not allowed to update volume except volume name"); + throw new InvalidParameterValueException("The domain admin and normal user are " + + "not allowed to update volume except volume name & delete protection"); } } @@ -2815,6 +2823,10 @@ public Volume updateVolume(long volumeId, String path, String state, Long storag volume.setName(name); } + if (deleteProtection != null) { + volume.setDeleteProtection(deleteProtection); + } + updateDisplay(volume, displayVolume); _volsDao.update(volumeId, volume); diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java index 31b0bc405973..f2a8a672d42a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManager.java +++ b/server/src/main/java/com/cloud/vm/UserVmManager.java @@ -141,8 +141,14 @@ boolean upgradeVirtualMachine(Long id, Long serviceOfferingId, Map securityGroupIdList, Map> extraDhcpOptionsMap) throws ResourceUnavailableException, InsufficientCapacityException; + UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, + Boolean isDisplayVmEnabled, Boolean deleteProtection, + Long osTypeId, String userData, Long userDataId, + String userDataDetails, Boolean isDynamicallyScalable, + HTTPMethod httpMethod, String customId, String hostName, + String instanceName, List securityGroupIdList, + Map> extraDhcpOptionsMap + ) throws ResourceUnavailableException, InsufficientCapacityException; //the validateCustomParameters, save and remove CustomOfferingDetils functions can be removed from the interface once we can //find a common place for all the scaling and upgrading code of both user and systemvms. diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 17596163c372..cf81a78070f5 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2921,8 +2921,11 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx } } } - return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, osTypeId, userData, userDataId, userDataDetails, isDynamicallyScalable, - cmd.getHttpMethod(), cmd.getCustomId(), hostName, cmd.getInstanceName(), securityGroupIdList, cmd.getDhcpOptionsMap()); + return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, + cmd.getDeleteProtection(), osTypeId, userData, + userDataId, userDataDetails, isDynamicallyScalable, cmd.getHttpMethod(), + cmd.getCustomId(), hostName, cmd.getInstanceName(), securityGroupIdList, + cmd.getDhcpOptionsMap()); } private boolean isExtraConfig(String detailName) { @@ -3023,9 +3026,14 @@ private void generateNetworkUsageForVm(VirtualMachine vm, boolean isDisplay, Str } @Override - public UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, Boolean isDisplayVmEnabled, Long osTypeId, String userData, - Long userDataId, String userDataDetails, Boolean isDynamicallyScalable, HTTPMethod httpMethod, String customId, String hostName, String instanceName, List securityGroupIdList, Map> extraDhcpOptionsMap) - throws ResourceUnavailableException, InsufficientCapacityException { + public UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, + Boolean isDisplayVmEnabled, Boolean deleteProtection, + Long osTypeId, String userData, Long userDataId, + String userDataDetails, Boolean isDynamicallyScalable, + HTTPMethod httpMethod, String customId, String hostName, + String instanceName, List securityGroupIdList, + Map> extraDhcpOptionsMap + ) throws ResourceUnavailableException, InsufficientCapacityException { UserVmVO vm = _vmDao.findById(id); if (vm == null) { throw new CloudRuntimeException("Unable to find virtual machine with id " + id); @@ -3060,6 +3068,10 @@ public UserVm updateVirtualMachine(long id, String displayName, String group, Bo isDisplayVmEnabled = vm.isDisplayVm(); } + if (deleteProtection == null) { + deleteProtection = vm.isDeleteProtection(); + } + boolean updateUserdata = false; if (userData != null) { // check and replace newlines @@ -3174,7 +3186,9 @@ public UserVm updateVirtualMachine(long id, String displayName, String group, Bo .getUuid(), nic.getId(), extraDhcpOptionsMap); } - _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId, userDataDetails, isDisplayVmEnabled, isDynamicallyScalable, customId, hostName, instanceName); + _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId, + userDataDetails, isDisplayVmEnabled, isDynamicallyScalable, + deleteProtection, customId, hostName, instanceName); if (updateUserdata) { updateUserData(vm); @@ -3411,6 +3425,12 @@ public UserVm destroyVm(DestroyVMCmd cmd) throws ResourceUnavailableException, C return vm; } + if (vm.isDeleteProtection()) { + throw new InvalidParameterValueException(String.format( + "Instance [id = %s, name = %s] has delete protection enabled and cannot be deleted.", + vm.getUuid(), vm.getName())); + } + // check if vm belongs to AutoScale vm group in Disabled state autoScaleManager.checkIfVmActionAllowed(vmId); @@ -8586,6 +8606,11 @@ private void validateVolumes(List volumes) { if (!(volume.getVolumeType() == Volume.Type.ROOT || volume.getVolumeType() == Volume.Type.DATADISK)) { throw new InvalidParameterValueException("Please specify volume of type " + Volume.Type.DATADISK.toString() + " or " + Volume.Type.ROOT.toString()); } + if (volume.isDeleteProtection()) { + throw new InvalidParameterValueException(String.format( + "Volume [id = %s, name = %s] has delete protection enabled and cannot be deleted", + volume.getUuid(), volume.getName())); + } } } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 90d857b58162..8316c57d67dd 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -487,7 +487,7 @@ private void verifyMethodsThatAreAlwaysExecuted() throws ResourceUnavailableExce Mockito.verify(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand); Mockito.verify(userVmManagerImpl).updateVirtualMachine(nullable(Long.class), nullable(String.class), nullable(String.class), nullable(Boolean.class), - nullable(Boolean.class), nullable(Long.class), + nullable(Boolean.class), nullable(Boolean.class), nullable(Long.class), nullable(String.class), nullable(Long.class), nullable(String.class), nullable(Boolean.class), nullable(HTTPMethod.class), nullable(String.class), nullable(String.class), nullable(String.class), nullable(List.class), nullable(Map.class)); @@ -498,7 +498,7 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource Mockito.doNothing().when(userVmManagerImpl).validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand); Mockito.doReturn(new ArrayList()).when(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand); Mockito.lenient().doReturn(Mockito.mock(UserVm.class)).when(userVmManagerImpl).updateVirtualMachine(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), - Mockito.anyBoolean(), Mockito.anyLong(), + Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.any(HTTPMethod.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyList(), Mockito.anyMap()); } diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index c05ae2ad42ee..c7c9a01bd32c 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -1035,6 +1035,34 @@ def test_13_destroy_and_expunge_vm(self): return + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") + def test_14_destroy_vm_delete_protection(self): + """Test destroy Virtual Machine with delete protection + """ + + # Validate the following + # 1. Should not be able to delete the VM when delete protection is enabled + # 2. Should be able to delete the VM after disabling delete protection + + vm = VirtualMachine.create( + self.apiclient, + self.services["small"], + serviceofferingid=self.small_offering.id, + mode=self.services["mode"], + startvm=False + ) + + vm.update(self.apiclient, deleteprotection=True) + try: + vm.delete(self.apiclient) + self.fail("VM shouldn't get deleted with delete protection enabled") + except Exception as e: + self.debug("Expected exception: %s" % e) + + vm.update(self.apiclient, deleteprotection=False) + vm.delete(self.apiclient) + + return class TestSecuredVmMigration(cloudstackTestCase): diff --git a/test/integration/smoke/test_volumes.py b/test/integration/smoke/test_volumes.py index 7d64a27eaf2d..28a029adf70f 100644 --- a/test/integration/smoke/test_volumes.py +++ b/test/integration/smoke/test_volumes.py @@ -1038,6 +1038,33 @@ def test_13_migrate_volume_and_change_offering(self): ) return + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_14_delete_volume_delete_protection(self): + """Delete a Volume with delete protection + + # Validate the following + # 1. delete volume will fail when delete protection is enabled + # 2. delete volume is successful when delete protection is disabled + """ + + volume = Volume.create( + self.apiclient, + self.services, + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering.id + ) + volume.update(self.apiclient, deleteprotection=True) + try: + volume.delete(self.apiclient) + self.fail("Volume delete should have failed with delete protection enabled") + except Exception as e: + self.debug("Volume delete failed as expected with error: %s" % e) + + volume.update(self.apiclient, deleteprotection=False) + volume.destroy(self.apiclient, expunge=True) + class TestVolumeEncryption(cloudstackTestCase): diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index e0a57c399248..557434ea2ee3 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1160,6 +1160,14 @@ def create(cls, apiclient, services, zoneid=None, account=None, return Volume(apiclient.createVolume(cmd).__dict__) + def update(self, apiclient, **kwargs): + """Updates the volume""" + + cmd = updateVolume.updateVolumeCmd() + cmd.id = self.id + [setattr(cmd, k, v) for k, v in list(kwargs.items())] + return (apiclient.updateVolume(cmd)) + @classmethod def create_custom_disk(cls, apiclient, services, account=None, domainid=None, diskofferingid=None, projectid=None): diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index b011ddaf6eb3..6ca710d40956 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -740,6 +740,7 @@ "label.deleting.iso": "Deleting ISO", "label.deleting.snapshot": "Deleting Snapshot", "label.deleting.template": "Deleting Template", +"label.deleteprotection": "Delete protection", "label.demote.project.owner": "Demote Account to regular role", "label.demote.project.owner.user": "Demote User to regular role", "label.deny": "Deny", diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index 51bbbb1cff40..6d7875368f5c 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -81,7 +81,8 @@ export default { details: () => { var fields = ['name', 'displayname', 'id', 'state', 'ipaddress', 'ip6address', 'templatename', 'ostypename', 'serviceofferingname', 'isdynamicallyscalable', 'haenable', 'hypervisor', 'boottype', 'bootmode', 'account', - 'domain', 'zonename', 'userdataid', 'userdataname', 'userdataparams', 'userdatadetails', 'userdatapolicy', 'hostcontrolstate'] + 'domain', 'zonename', 'userdataid', 'userdataname', 'userdataparams', 'userdatadetails', 'userdatapolicy', + 'hostcontrolstate', 'deleteprotection'] const listZoneHaveSGEnabled = store.getters.zones.filter(zone => zone.securitygroupsenabled === true) if (!listZoneHaveSGEnabled || listZoneHaveSGEnabled.length === 0) { return fields diff --git a/ui/src/config/section/storage.js b/ui/src/config/section/storage.js index 66c6cd4d0d17..d979e7395e0d 100644 --- a/ui/src/config/section/storage.js +++ b/ui/src/config/section/storage.js @@ -62,7 +62,7 @@ export default { return fields }, - details: ['name', 'id', 'type', 'storagetype', 'diskofferingdisplaytext', 'deviceid', 'sizegb', 'physicalsize', 'provisioningtype', 'utilization', 'diskkbsread', 'diskkbswrite', 'diskioread', 'diskiowrite', 'diskiopstotal', 'miniops', 'maxiops', 'path'], + details: ['name', 'id', 'type', 'storagetype', 'diskofferingdisplaytext', 'deviceid', 'sizegb', 'physicalsize', 'provisioningtype', 'utilization', 'diskkbsread', 'diskkbswrite', 'diskioread', 'diskiowrite', 'diskiopstotal', 'miniops', 'maxiops', 'path', 'deleteprotection'], related: [{ name: 'snapshot', title: 'label.snapshots', @@ -148,7 +148,7 @@ export default { icon: 'edit-outlined', label: 'label.edit', dataView: true, - args: ['name'], + args: ['name', 'deleteprotection'], mapping: { account: { value: (record) => { return record.account } diff --git a/ui/src/views/compute/EditVM.vue b/ui/src/views/compute/EditVM.vue index 550c4645ed6f..87e6d96d6c6b 100644 --- a/ui/src/views/compute/EditVM.vue +++ b/ui/src/views/compute/EditVM.vue @@ -111,6 +111,13 @@ + + + + +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -175,6 +182,7 @@ export default { displayname: this.resource.displayname, ostypeid: this.resource.ostypeid, isdynamicallyscalable: this.resource.isdynamicallyscalable, + deleteprotection: this.resource.deleteprotection, group: this.resource.group, securitygroupids: this.resource.securitygroup.map(x => x.id), userdata: '', @@ -314,6 +322,9 @@ export default { if (values.isdynamicallyscalable !== undefined) { params.isdynamicallyscalable = values.isdynamicallyscalable } + if (values.deleteprotection !== undefined) { + params.deleteprotection = values.deleteprotection + } if (values.haenable !== undefined) { params.haenable = values.haenable }