Skip to content

Commit 82fbd80

Browse files
Kshitij Kansalkansal
Kshitij Kansal
authored and
kansal
committed
CLOUDSTACK-8727: API call listVirtualMachines returns same keypair: Corrected and test cases added
1 parent b400608 commit 82fbd80

3 files changed

Lines changed: 56 additions & 9 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3642,9 +3642,9 @@ public SSHKeyPair registerSSHKeyPair(final RegisterSSHKeyPairCmd cmd) {
36423642
* @throws InvalidParameterValueException
36433643
*/
36443644
private void checkForKeyByPublicKey(final RegisterSSHKeyPairCmd cmd, final Account owner) throws InvalidParameterValueException {
3645-
final SSHKeyPairVO existingPair = _sshKeyPairDao.findByPublicKey(owner.getAccountId(), owner.getDomainId(), cmd.getPublicKey());
3645+
final SSHKeyPairVO existingPair = _sshKeyPairDao.findByPublicKey(owner.getAccountId(), owner.getDomainId(), getPublicKeyFromKeyKeyMaterial(cmd.getPublicKey()));
36463646
if (existingPair != null) {
3647-
throw new InvalidParameterValueException("A key pair with name '" + cmd.getPublicKey() + "' already exists for this account.");
3647+
throw new InvalidParameterValueException("A key pair with key '" + cmd.getPublicKey() + "' already exists for this account.");
36483648
}
36493649
}
36503650

@@ -3674,7 +3674,7 @@ private String getFingerprint(final String publicKey) {
36743674
* @return
36753675
* @throws InvalidParameterValueException
36763676
*/
3677-
private String getPublicKeyFromKeyKeyMaterial(final String key) throws InvalidParameterValueException {
3677+
protected String getPublicKeyFromKeyKeyMaterial(final String key) throws InvalidParameterValueException {
36783678
final String publicKey = SSHKeysHelper.getPublicKeyFromKeyMaterial(key);
36793679

36803680
if (publicKey == null) {

‎server/test/com/cloud/server/ManagementServerImplTest.java‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,18 @@
1616
// under the License.
1717
package com.cloud.server;
1818

19+
import com.cloud.user.SSHKeyPair;
1920
import org.junit.Test;
2021
import org.junit.runner.RunWith;
2122
import org.mockito.Mock;
2223
import org.mockito.Mockito;
2324
import org.mockito.Spy;
2425
import org.mockito.runners.MockitoJUnitRunner;
2526

27+
import static org.mockito.Matchers.anyString;
28+
import static org.mockito.Mockito.when;
29+
import static org.mockito.Mockito.any;
30+
2631
import org.apache.cloudstack.api.command.user.ssh.RegisterSSHKeyPairCmd;
2732

2833
import com.cloud.exception.InvalidParameterValueException;
@@ -35,33 +40,71 @@ public class ManagementServerImplTest {
3540

3641
@Mock
3742
RegisterSSHKeyPairCmd regCmd;
43+
3844
@Mock
3945
SSHKeyPairVO existingPair;
46+
4047
@Mock
4148
Account account;
49+
4250
@Mock
4351
SSHKeyPairDao sshKeyPairDao;
4452
ManagementServerImpl ms = new ManagementServerImpl();
53+
54+
@Mock
55+
SSHKeyPair sshKeyPair;
56+
4557
@Spy
4658
ManagementServerImpl spy;
4759

4860
@Test(expected = InvalidParameterValueException.class)
49-
public void testExistingPairRegistration() {
61+
public void testDuplicateRegistraitons(){
5062
String accountName = "account";
51-
String publicKeyString = "very public";
52-
// setup owner with domainid
63+
String publicKeyString = "ssh-rsa very public";
64+
String publicKeyMaterial = spy.getPublicKeyFromKeyKeyMaterial(publicKeyString);
65+
5366
Mockito.doReturn(account).when(spy).getCaller();
5467
Mockito.doReturn(account).when(spy).getOwner(regCmd);
55-
// mock _sshKeyPairDao.findByName to return null
68+
5669
Mockito.doNothing().when(spy).checkForKeyByName(regCmd, account);
57-
// mock _sshKeyPairDao.findByPublicKey to return a known object
5870
Mockito.doReturn(accountName).when(regCmd).getAccountName();
71+
5972
Mockito.doReturn(publicKeyString).when(regCmd).getPublicKey();
6073
Mockito.doReturn("name").when(regCmd).getName();
74+
6175
spy._sshKeyPairDao = sshKeyPairDao;
6276
Mockito.doReturn(1L).when(account).getAccountId();
6377
Mockito.doReturn(1L).when(account).getDomainId();
64-
Mockito.doReturn(existingPair).when(sshKeyPairDao).findByPublicKey(1L, 1L, publicKeyString);
78+
Mockito.doReturn(Mockito.mock(SSHKeyPairVO.class)).when(sshKeyPairDao).persist(any(SSHKeyPairVO.class));
79+
80+
when(sshKeyPairDao.findByName(1L, 1L, "name")).thenReturn(null).thenReturn(null);
81+
when(sshKeyPairDao.findByPublicKey(1L, 1L, publicKeyMaterial)).thenReturn(null).thenReturn(existingPair);
82+
83+
spy.registerSSHKeyPair(regCmd);
84+
spy.registerSSHKeyPair(regCmd);
85+
}
86+
@Test
87+
public void testSuccess(){
88+
String accountName = "account";
89+
String publicKeyString = "ssh-rsa very public";
90+
String publicKeyMaterial = spy.getPublicKeyFromKeyKeyMaterial(publicKeyString);
91+
92+
Mockito.doReturn(1L).when(account).getAccountId();
93+
Mockito.doReturn(1L).when(account).getAccountId();
94+
spy._sshKeyPairDao = sshKeyPairDao;
95+
96+
97+
//Mocking the DAO object functions - NO object found in DB
98+
Mockito.doReturn(Mockito.mock(SSHKeyPairVO.class)).when(sshKeyPairDao).findByPublicKey(1L, 1L,publicKeyMaterial);
99+
Mockito.doReturn(Mockito.mock(SSHKeyPairVO.class)).when(sshKeyPairDao).findByName(1L, 1L, accountName);
100+
Mockito.doReturn(Mockito.mock(SSHKeyPairVO.class)).when(sshKeyPairDao).persist(any(SSHKeyPairVO.class));
101+
102+
//Mocking the User Params
103+
Mockito.doReturn(accountName).when(regCmd).getName();
104+
Mockito.doReturn(publicKeyString).when(regCmd).getPublicKey();
105+
Mockito.doReturn(account).when(spy).getOwner(regCmd);
106+
65107
spy.registerSSHKeyPair(regCmd);
108+
Mockito.verify(spy, Mockito.times(3)).getPublicKeyFromKeyKeyMaterial(anyString());
66109
}
67110
}

‎setup/db/db/schema-452to460.sql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,10 @@ CREATE VIEW `cloud`.`user_vm_view` AS
354354
left join
355355
`cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory')));
356356

357+
---Additional checks to ensure duplicate keys are not registered and remove the previously stored duplicate keys.
358+
DELETE `s1` FROM `ssh_keypairs` `s1`, `ssh_keypairs` `s2` WHERE `s1`.`id` > `s2`.`id` AND `s1`.`public_key` = `s2`.`public_key` AND `s1`.`account_id` = `s2`.`account_id`;
359+
ALTER TABLE `ssh_keypairs` ADD UNIQUE `unique_index`(`fingerprint`,`account_id`);
360+
357361
-- ovm3 stuff
358362
INSERT INTO `cloud`.`guest_os_hypervisor` (hypervisor_type, guest_os_name, guest_os_id) VALUES ("Ovm3", 'Sun Solaris 10(32-bit)', 79);
359363
INSERT INTO `cloud`.`guest_os_hypervisor` (hypervisor_type, guest_os_name, guest_os_id) VALUES ("Ovm3", 'Sun Solaris 10(64-bit)', 80);

0 commit comments

Comments
 (0)