diff --git a/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java b/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java index 5c78d6bedd64..c91f5b736514 100644 --- a/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java +++ b/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java @@ -79,6 +79,7 @@ public static class Param { public static final Param PreserveNics = new Param("PreserveNics"); public static final Param ConsiderLastHost = new Param("ConsiderLastHost"); public static final Param ReturnAfterVolumePrepare = new Param("ReturnAfterVolumePrepare"); + public static final Param ResetPasswordOnRestore = new Param("ResetPasswordOnRestore"); private String name; 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 03b73834a94e..762da4ab5ff8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -452,6 +452,7 @@ public class ApiConstants { public static final String CURRENT_PASSWORD = "currentpassword"; public static final String SHOULD_UPDATE_PASSWORD = "update_passwd_on_host"; public static final String PASSWORD_ENABLED = "passwordenabled"; + public static final String RESET_PASSWORD = "resetpassword"; public static final String SSHKEY_ENABLED = "sshkeyenabled"; public static final String PATH = "path"; public static final String PATH_READY = "pathready"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java index 6192bfb4540a..7390d4d2da71 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java @@ -70,6 +70,11 @@ public class CreateVMFromBackupCmd extends BaseDeployVMCmd { @Parameter(name = ApiConstants.PRESERVE_IP, type = CommandType.BOOLEAN, description = "Use the same IP/MAC addresses as stored in the backup metadata. Works only if the original Instance is deleted and the IP/MAC address is available.") private Boolean preserveIp; + @Parameter(name = ApiConstants.RESET_PASSWORD, type = CommandType.BOOLEAN, + description = "For a password enabled template, whether to generate a new password for the created Instance and return it in the response. " + + "If not specified, the zone setting `restore.vm.from.backup.reset.password` decides.", since = "4.22.1.0") + private Boolean resetPassword; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -90,6 +95,10 @@ public boolean getPreserveIp() { return (preserveIp != null) ? preserveIp : false; } + public Boolean getResetPassword() { + return resetPassword; + } + @Override public void create() { UserVm vm; diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java index 38cb6d2db46b..7882668ccbbd 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManager.java +++ b/server/src/main/java/com/cloud/vm/UserVmManager.java @@ -111,6 +111,10 @@ public interface UserVmManager extends UserVmService { ConfigKey AllowDifferentHostTagsOfferingsForVmScale = new ConfigKey<>("Advanced", Boolean.class, "allow.different.host.tags.offerings.for.vm.scale", "false", "Enables/Disable allowing to change a VM offering to offerings with different host tags", true); + ConfigKey ResetPasswordOnRestoreFromBackup = new ConfigKey("Advanced", Boolean.class, "restore.vm.from.backup.reset.password", "true", + "For a password enabled template, whether to generate a new password and expose it in the API response when creating/restoring an Instance from a backup. " + + "Can be overridden per call with the createVMFromBackup API's resetpassword parameter.", true, ConfigKey.Scope.Zone); + static final int MAX_USER_DATA_LENGTH_BYTES = 2048; public static final String CKS_NODE = "cksnode"; diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index fe18263fd3bf..d6348e1ca5ac 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -5801,7 +5801,8 @@ private Pair> startVirtualMac } // Set parameters - Map params = null; + Map params = new HashMap<>(); + params.putAll(additionalParams); if (vm.isUpdateParameters()) { _vmDao.loadDetails(vm); String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template); @@ -5811,18 +5812,19 @@ private Pair> startVirtualMac // Check if an SSH key pair was selected for the instance and if so // use it to encrypt & save the vm password encryptAndStorePassword(vm, password); - params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.VmPassword, password); + // overwrite VmPassword + params = createParameterInParameterMap(params, VirtualMachineProfile.Param.VmPassword, password); } if (additionalParams.containsKey(VirtualMachineProfile.Param.BootIntoSetup)) { if (!HypervisorType.VMware.equals(vm.getHypervisorType())) { throw new InvalidParameterValueException(ApiConstants.BOOT_INTO_SETUP + " makes no sense for " + vm.getHypervisorType()); } + + //overwrite BootIntoSetup Object paramValue = additionalParams.get(VirtualMachineProfile.Param.BootIntoSetup); - if (logger.isTraceEnabled()) { - logger.trace("It was specified whether to enter setup mode: " + paramValue.toString()); - } - params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.BootIntoSetup, paramValue); + logger.trace("It was specified whether to enter setup mode: {}", paramValue.toString()); + params = createParameterInParameterMap(params, VirtualMachineProfile.Param.BootIntoSetup, paramValue); } VirtualMachineEntity vmEntity = _orchSrvc.getVirtualMachine(vm.getUuid()); @@ -5843,7 +5845,7 @@ private Pair> startVirtualMac vmEntity.deploy(reservationId, Long.toString(callerUser.getId()), params, deployOnGivenHost); Pair> vmParamPair = new Pair(vm, params); - if (vm.isUpdateParameters()) { + if (shouldClearUpdateParametersFlag(vm, additionalParams)) { // this value is not being sent to the backend; need only for api // display purposes if (template.isEnablePassword()) { @@ -5907,6 +5909,16 @@ public Pair> startVirtualMach } } + /** + * False for a volume-prepare-only start that should still reset the password (isUpdateParameters must stay + * set for the real start that follows). + */ + boolean shouldClearUpdateParametersFlag(UserVmVO vm, Map additionalParams) { + boolean isVolumePrepareOnly = Boolean.TRUE.equals(additionalParams.get(VirtualMachineProfile.Param.ReturnAfterVolumePrepare)); + boolean resetPasswordOnRestore = Boolean.TRUE.equals(additionalParams.get(VirtualMachineProfile.Param.ResetPasswordOnRestore)); + return vm.isUpdateParameters() && !(isVolumePrepareOnly && resetPasswordOnRestore); + } + /** * If the template is password enabled and the VM already has a password, returns it. * If the template is password enabled and the VM does not have a password, sets the password to the password defined by the user and returns it. If no password is informed, @@ -5940,20 +5952,18 @@ protected String getCurrentVmPasswordOrDefineNewPassword(String newPassword, Use return password; } - private Map createParameterInParameterMap(Map params, Map parameterMap, VirtualMachineProfile.Param parameter, + /** + * Create or overwrite a parameter in the list + * @param params the list of parameters + * @param parameter the parameter to create/overwrite + * @param parameterValue the value to give to the parameter + * @return the resulting updated list of parameters + */ + private Map createParameterInParameterMap( + Map params, + VirtualMachineProfile.Param parameter, Object parameterValue) { - if (logger.isTraceEnabled()) { - logger.trace(String.format("createParameterInParameterMap(%s, %s)", parameter, parameterValue)); - } - if (params == null) { - if (logger.isTraceEnabled()) { - logger.trace("creating new Parameter map"); - } - params = new HashMap<>(); - if (parameterMap != null) { - params.putAll(parameterMap); - } - } + logger.trace("createParameterInParameterMap({}, {})", parameter, parameterValue); params.put(parameter, parameterValue); return params; } @@ -9420,7 +9430,8 @@ public ConfigKey[] getConfigKeys() { VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties, KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList, DestroyRootVolumeOnVmDestruction, EnforceStrictResourceLimitHostTagCheck, StrictHostTags, AllowUserForceStopVm, VmDistinctHostNameScope, - VmwareAdditionalDetailsFromOvaEnabled, VmwareAllowedAdditionalDetailsFromOva, AllowDifferentHostTagsOfferingsForVmScale}; + VmwareAdditionalDetailsFromOvaEnabled, VmwareAllowedAdditionalDetailsFromOva, AllowDifferentHostTagsOfferingsForVmScale, + ResetPasswordOnRestoreFromBackup}; } @Override @@ -9805,6 +9816,17 @@ public UserVm allocateVMFromBackup(CreateVMFromBackupCmd cmd) throws Insufficien return vm; } + /** + * The cmd's resetpassword parameter, if set; otherwise the zone's ResetPasswordOnRestoreFromBackup setting. + */ + boolean isResetPasswordOnRestoreFromBackup(CreateVMFromBackupCmd cmd) { + if (cmd.getResetPassword() != null) { + return cmd.getResetPassword(); + } + UserVmVO vm = _vmDao.findById(cmd.getEntityId()); + return ResetPasswordOnRestoreFromBackup.valueIn(vm.getDataCenterId()); + } + @Override public UserVm restoreVMFromBackup(CreateVMFromBackupCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { long vmId = cmd.getEntityId(); @@ -9812,6 +9834,7 @@ public UserVm restoreVMFromBackup(CreateVMFromBackupCmd cmd) throws ResourceUnav Map diskOfferingMap = cmd.getDataDiskTemplateToDiskOfferingMap(); Map additonalParams = new HashMap<>(); additonalParams.put(VirtualMachineProfile.Param.ReturnAfterVolumePrepare, true); + additonalParams.put(VirtualMachineProfile.Param.ResetPasswordOnRestore, isResetPasswordOnRestoreFromBackup(cmd)); try { Pair> vmParamPair = null; diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index cb15298fbf33..6cc1ef2337c3 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -42,6 +42,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.lang.reflect.Field; import java.text.SimpleDateFormat; import java.time.LocalDateTime; import java.time.ZoneOffset; @@ -88,6 +89,7 @@ import org.apache.cloudstack.backup.dao.BackupScheduleDao; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.resourcelimit.Reserver; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.Scope; @@ -1400,6 +1402,105 @@ public void getCurrentVmPasswordOrDefineNewPasswordTestUserDefinedPasswordReturn Assert.assertEquals(expected, userVmVoMock.getPassword()); } + private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException { + final Field f = ConfigKey.class.getDeclaredField("_defaultValue"); + f.setAccessible(true); + f.set(configKey, value); + } + + @Test + public void shouldClearUpdateParametersFlagTestVmDoesNotHaveParametersToUpdateReturnFalse() { + Mockito.doReturn(false).when(userVmVoMock).isUpdateParameters(); + + boolean result = userVmManagerImpl.shouldClearUpdateParametersFlag(userVmVoMock, new HashMap<>()); + + Assert.assertFalse(result); + } + + @Test + public void shouldClearUpdateParametersFlagTestRegularStartReturnTrue() { + Mockito.doReturn(true).when(userVmVoMock).isUpdateParameters(); + + boolean result = userVmManagerImpl.shouldClearUpdateParametersFlag(userVmVoMock, new HashMap<>()); + + Assert.assertTrue(result); + } + + @Test + public void shouldClearUpdateParametersFlagTestVolumePrepareOnlyWithoutPasswordResetReturnTrue() { + Mockito.doReturn(true).when(userVmVoMock).isUpdateParameters(); + Map additionalParams = new HashMap<>(); + additionalParams.put(VirtualMachineProfile.Param.ReturnAfterVolumePrepare, true); + + boolean result = userVmManagerImpl.shouldClearUpdateParametersFlag(userVmVoMock, additionalParams); + + Assert.assertTrue(result); + } + + @Test + public void shouldClearUpdateParametersFlagTestVolumePrepareOnlyWithPasswordResetReturnFalse() { + Mockito.doReturn(true).when(userVmVoMock).isUpdateParameters(); + Map additionalParams = new HashMap<>(); + additionalParams.put(VirtualMachineProfile.Param.ReturnAfterVolumePrepare, true); + additionalParams.put(VirtualMachineProfile.Param.ResetPasswordOnRestore, true); + + boolean result = userVmManagerImpl.shouldClearUpdateParametersFlag(userVmVoMock, additionalParams); + + Assert.assertFalse(result); + } + + @Test + public void isResetPasswordOnRestoreFromBackupTestCmdOverrideTrueIgnoresZoneSetting() throws IllegalAccessException, NoSuchFieldException { + overrideDefaultConfigValue(UserVmManager.ResetPasswordOnRestoreFromBackup, "false"); + CreateVMFromBackupCmd cmd = mock(CreateVMFromBackupCmd.class); + when(cmd.getResetPassword()).thenReturn(true); + + boolean result = userVmManagerImpl.isResetPasswordOnRestoreFromBackup(cmd); + + Assert.assertTrue(result); + overrideDefaultConfigValue(UserVmManager.ResetPasswordOnRestoreFromBackup, "true"); + } + + @Test + public void isResetPasswordOnRestoreFromBackupTestCmdOverrideFalseIgnoresZoneSetting() throws IllegalAccessException, NoSuchFieldException { + overrideDefaultConfigValue(UserVmManager.ResetPasswordOnRestoreFromBackup, "true"); + CreateVMFromBackupCmd cmd = mock(CreateVMFromBackupCmd.class); + when(cmd.getResetPassword()).thenReturn(false); + + boolean result = userVmManagerImpl.isResetPasswordOnRestoreFromBackup(cmd); + + Assert.assertFalse(result); + } + + @Test + public void isResetPasswordOnRestoreFromBackupTestNoCmdOverrideFallsBackToZoneSettingTrue() throws IllegalAccessException, NoSuchFieldException { + overrideDefaultConfigValue(UserVmManager.ResetPasswordOnRestoreFromBackup, "true"); + CreateVMFromBackupCmd cmd = mock(CreateVMFromBackupCmd.class); + when(cmd.getResetPassword()).thenReturn(null); + when(cmd.getEntityId()).thenReturn(vmId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + Mockito.doReturn(1L).when(userVmVoMock).getDataCenterId(); + + boolean result = userVmManagerImpl.isResetPasswordOnRestoreFromBackup(cmd); + + Assert.assertTrue(result); + } + + @Test + public void isResetPasswordOnRestoreFromBackupTestNoCmdOverrideFallsBackToZoneSettingFalse() throws IllegalAccessException, NoSuchFieldException { + overrideDefaultConfigValue(UserVmManager.ResetPasswordOnRestoreFromBackup, "false"); + CreateVMFromBackupCmd cmd = mock(CreateVMFromBackupCmd.class); + when(cmd.getResetPassword()).thenReturn(null); + when(cmd.getEntityId()).thenReturn(vmId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + Mockito.doReturn(1L).when(userVmVoMock).getDataCenterId(); + + boolean result = userVmManagerImpl.isResetPasswordOnRestoreFromBackup(cmd); + + Assert.assertFalse(result); + overrideDefaultConfigValue(UserVmManager.ResetPasswordOnRestoreFromBackup, "true"); + } + @Test public void testSetVmRequiredFieldsForImportNotImport() { userVmManagerImpl.setVmRequiredFieldsForImport(false, userVmVoMock, _dcMock,