This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new a3f39db server: Remove meaningless password regeneration on
resetSSHKeyForVirtualMachine (#4819)
a3f39db is described below
commit a3f39db62b6e48e0e9677839f6bc6807be5b32be
Author: Daniel Augusto Veronezi Salvador
<[email protected]>
AuthorDate: Thu Jun 17 03:27:52 2021 -0300
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 <[email protected]>
---
.../com/cloud/server/ManagementServerImpl.java | 23 ++++++++---------
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 29 ++++++++++------------
.../java/com/cloud/vm/UserVmManagerImplTest.java | 13 ++++++++++
3 files changed, 36 insertions(+), 29 deletions(-)
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 98937ca..9ae7ddf 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -4233,26 +4233,23 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
}
@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;
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index fb32dc5..c1743e7 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -862,22 +862,28 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
}
_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 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
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 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
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) {
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index da59f07..ae647f3 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -558,4 +558,17 @@ public class UserVmManagerImplTest {
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);
+ }
}
