server: Remove meaningless password regeneration on resetSSHKeyForVirtualMachine (#4819)

On API `resetSSHKeyForVirtualMachine`, ACS also regenerates VM password when it uses a template with `Password Enabled` as true; there is already anAPI to reset VM password, therefore, the reset SSH keys API should not reset the VM SSH password as well.

Besides running a meaningless process, the VM's password regeneration slows down the main process and may cause a confusion in operations due to password change in the VM without being explicity requested.

Co-authored-by: Daniel Augusto Veronezi Salvador <daniel@scclouds.com.br>

Author: GutoVeronezi

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -4233,26 +4233,23 @@ private SSHKeyPair createAndSaveSSHKeyPair(final String name, final String finge
     }
 
     @Override
-    public String getVMPassword(final GetVMPasswordCmd cmd) {
-        final Account caller = getCaller();
+    public String getVMPassword(GetVMPasswordCmd cmd) {
+        Account caller = getCaller();
+        long vmId = cmd.getId();
+        UserVmVO vm = _userVmDao.findById(vmId);
 
-        final UserVmVO vm = _userVmDao.findById(cmd.getId());
         if (vm == null) {
-            final InvalidParameterValueException ex = new InvalidParameterValueException("No VM with specified id found.");
-            ex.addProxyObject(cmd.getId().toString(), "vmId");
-            throw ex;
+            throw new InvalidParameterValueException(String.format("No VM found with id [%s].", vmId));
         }
 
-        // make permission check
         _accountMgr.checkAccess(caller, null, true, vm);
 
         _userVmDao.loadDetails(vm);
-        final String password = vm.getDetail("Encrypted.Password");
-        if (password == null || password.equals("")) {
-            final InvalidParameterValueException ex = new InvalidParameterValueException(
-                    "No password for VM with specified id found. " + "If VM is created from password enabled template and SSH keypair is assigned to VM then only password can be retrieved.");
-            ex.addProxyObject(vm.getUuid(), "vmId");
-            throw ex;
+        String password = vm.getDetail("Encrypted.Password");
+
+        if (StringUtils.isEmpty(password)) {
+            throw new InvalidParameterValueException(String.format("No password found for VM with id [%s]. When the VM's SSH keypair is changed, the current encrypted password is "
+              + "removed due to incosistency in the encryptation, as the new SSH keypair is different from which the password was encrypted. To get a new password, it must be reseted.", vmId));
         }
 
         return password;

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -862,22 +862,28 @@ public UserVm resetVMSSHKey(ResetVMSSHKeyCmd cmd) throws ResourceUnavailableExce
         }
 
         _accountMgr.checkAccess(caller, null, true, userVm);
-        String password = null;
+
         String sshPublicKey = s.getPublicKey();
-        if (template != null && template.isEnablePassword()) {
-            password = _mgr.generateRandomPassword();
-        }
 
-        boolean result = resetVMSSHKeyInternal(vmId, sshPublicKey, password);
+        boolean result = resetVMSSHKeyInternal(vmId, sshPublicKey);
 
         if (!result) {
             throw new CloudRuntimeException("Failed to reset SSH Key for the virtual machine ");
         }
-        userVm.setPassword(password);
+
+        removeEncryptedPasswordFromUserVmVoDetails(userVm);
+
         return userVm;
     }
 
-    private boolean resetVMSSHKeyInternal(Long vmId, String sshPublicKey, String password) throws ResourceUnavailableException, InsufficientCapacityException {
+    protected void removeEncryptedPasswordFromUserVmVoDetails(UserVmVO userVmVo) {
+        Map<String, String> details = userVmVo.getDetails();
+        details.remove(VmDetailConstants.ENCRYPTED_PASSWORD);
+        userVmVo.setDetails(details);
+        _vmDao.saveDetails(userVmVo);
+    }
+
+    private boolean resetVMSSHKeyInternal(Long vmId, String sshPublicKey) throws ResourceUnavailableException, InsufficientCapacityException {
         Long userId = CallContext.current().getCallingUserId();
         VMInstanceVO vmInstance = _vmDao.findById(vmId);
 
@@ -894,10 +900,6 @@ private boolean resetVMSSHKeyInternal(Long vmId, String sshPublicKey, String pas
 
         VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vmInstance);
 
-        if (template.isEnablePassword()) {
-            vmProfile.setParameter(VirtualMachineProfile.Param.VmPassword, password);
-        }
-
         UserDataServiceProvider element = _networkMgr.getSSHKeyResetProvider(defaultNetwork);
         if (element == null) {
             throw new CloudRuntimeException("Can't find network element for " + Service.UserData.getName() + " provider needed for SSH Key reset");
@@ -912,11 +914,6 @@ private boolean resetVMSSHKeyInternal(Long vmId, String sshPublicKey, String pas
             final UserVmVO userVm = _vmDao.findById(vmId);
             _vmDao.loadDetails(userVm);
             userVm.setDetail(VmDetailConstants.SSH_PUBLIC_KEY, sshPublicKey);
-            if (template.isEnablePassword()) {
-                userVm.setPassword(password);
-                //update the encrypted password in vm_details table too
-                encryptAndStorePassword(userVm, password);
-            }
             _vmDao.saveDetails(userVm);
 
             if (vmInstance.getState() == State.Stopped) {

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -558,4 +558,17 @@ private DiskOfferingVO prepareDiskOffering(long rootSize, long diskOfferingId, l
         Mockito.when(newRootDiskOffering.getName()).thenReturn("OfferingName");
         return newRootDiskOffering;
     }
+
+    @Test
+    public void validateRemoveEncryptedPasswordFromUserVmVoDetails(){
+        Map<String, String> detailsMock = Mockito.mock(HashMap.class);
+
+        Mockito.doReturn(detailsMock).when(userVmVoMock).getDetails();
+        Mockito.doNothing().when(userVmDao).saveDetails(userVmVoMock);
+        userVmManagerImpl.removeEncryptedPasswordFromUserVmVoDetails(userVmVoMock);
+
+        Mockito.verify(detailsMock, Mockito.times(1)).remove(VmDetailConstants.ENCRYPTED_PASSWORD);
+        Mockito.verify(userVmVoMock, Mockito.times(1)).setDetails(detailsMock);
+        Mockito.verify(userVmDao, Mockito.times(1)).saveDetails(userVmVoMock);
+    }
 }