You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2015/06/17 22:04:05 UTC

[1/4] git commit: updated refs/heads/4.4 to f0cc5c5

Repository: cloudstack
Updated Branches:
  refs/heads/4.4 e6f81dbbb -> f0cc5c510


CLOUDSTACK-8545 make reboot on out of band migration configurable


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/9ced5755
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/9ced5755
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/9ced5755

Branch: refs/heads/4.4
Commit: 9ced57551de5e1d62f2f7eb286440931bb6de262
Parents: e6f81db
Author: Daan Hoogland <da...@gmail.com>
Authored: Tue Jun 16 17:12:43 2015 +0200
Committer: Daan Hoogland <da...@gmail.com>
Committed: Wed Jun 17 09:46:46 2015 +0200

----------------------------------------------------------------------
 .../router/VirtualNetworkApplianceManager.java  |  9 +++-
 .../VirtualNetworkApplianceManagerImpl.java     | 50 ++++++++++----------
 2 files changed, 33 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9ced5755/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
index 5cff679..ac3406d 100644
--- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
+++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
@@ -52,6 +52,7 @@ public interface VirtualNetworkApplianceManager extends Manager, VirtualNetworkA
     static final String RouterTemplateLxcCK = "router.template.lxc";
     static final String SetServiceMonitorCK = "network.router.EnableServiceMonitoring";
     static final String RouterAlertsCheckIntervalCK = "router.alerts.check.interval";
+    static final String RouterReprovisionOnOutOfBandMigrationCK = "router.reboot.when.outofband.migrated";
 
     static final ConfigKey<String> RouterTemplateXen = new ConfigKey<String>(String.class, RouterTemplateXenCK, "Advanced", "SystemVM Template (XenServer)",
         "Name of the default router template on Xenserver.", true, ConfigKey.Scope.Zone, null);
@@ -63,12 +64,16 @@ public interface VirtualNetworkApplianceManager extends Manager, VirtualNetworkA
         "Name of the default router template on Hyperv.", true, ConfigKey.Scope.Zone, null);
     static final ConfigKey<String> RouterTemplateLxc = new ConfigKey<String>(String.class, RouterTemplateLxcCK, "Advanced", "SystemVM Template (LXC)",
         "Name of the default router template on LXC.", true, ConfigKey.Scope.Zone, null);
-
     static final ConfigKey<String> SetServiceMonitor = new ConfigKey<String>(String.class, SetServiceMonitorCK, "Advanced", "true",
             "service monitoring in router enable/disable option, default true", true, ConfigKey.Scope.Zone, null);
-
     static final ConfigKey<Integer> RouterAlertsCheckInterval = new ConfigKey<Integer>(Integer.class, RouterAlertsCheckIntervalCK, "Advanced", "1800",
             "Interval (in seconds) to check for alerts in Virtual Router.", false, ConfigKey.Scope.Global, null);
+    static final ConfigKey<Boolean> routerVersionCheckEnabled = new ConfigKey<Boolean>("Advanced", Boolean.class, "router.version.check", "true",
+            "If true, router minimum required version is checked before sending command", false);
+    static final ConfigKey<Boolean> UseExternalDnsServers = new ConfigKey<Boolean>(Boolean.class, "use.external.dns", "Advanced", "false",
+            "Bypass internal dns, use external dns1 and dns2", true, ConfigKey.Scope.Zone, null);
+    static final ConfigKey<Boolean> RouterReprovisionOnOutOfBandMigration = new ConfigKey<Boolean>(Boolean.class, RouterReprovisionOnOutOfBandMigrationCK, "Advanced", "false",
+            "Reboot routers when they are migrated out of band in order to reprovision", true, ConfigKey.Scope.Zone, null);
 
     public static final int DEFAULT_ROUTER_VM_RAMSIZE = 256;             // 256M
     public static final int DEFAULT_ROUTER_CPU_MHZ = 500;                // 500 MHz

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9ced5755/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
index 59bd4c6..4400a62 100755
--- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
+++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
@@ -673,13 +673,6 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V
         }
     }
 
-    static final ConfigKey<Boolean> UseExternalDnsServers = new ConfigKey<Boolean>(Boolean.class, "use.external.dns", "Advanced", "false",
-            "Bypass internal dns, use external dns1 and dns2", true, ConfigKey.Scope.Zone, null);
-
-    static final ConfigKey<Boolean> routerVersionCheckEnabled = new ConfigKey<Boolean>("Advanced", Boolean.class, "router.version.check", "true",
-            "If true, router minimum required version is checked before sending command", false);
-
-
     @Override
     public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
 
@@ -4394,7 +4387,7 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {UseExternalDnsServers, routerVersionCheckEnabled, SetServiceMonitor, RouterAlertsCheckInterval};
+        return new ConfigKey<?>[] {UseExternalDnsServers, routerVersionCheckEnabled, SetServiceMonitor, RouterAlertsCheckInterval, RouterReprovisionOnOutOfBandMigration};
     }
 
     @Override
@@ -4404,25 +4397,34 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V
 
     @Override
     public boolean postStateTransitionEvent(State oldState, VirtualMachine.Event event, State newState, VirtualMachine vo, boolean status, Object opaque) {
-        if (event == VirtualMachine.Event.FollowAgentPowerOnReport && newState == State.Running) {
-            if (vo.getType() == VirtualMachine.Type.DomainRouter) {
-                if (opaque != null && opaque instanceof Pair<?, ?>) {
-                    Pair<?, ?> pair = (Pair<?, ?>)opaque;
-                    Object first = pair.first();
-                    Object second = pair.second();
-                    if (first != null && second != null && first instanceof Long && second instanceof Long) {
-                        Long hostId = (Long)first;
-                        Long powerHostId = (Long)second;
-                        // If VM host known to CS is different from 'PowerOn' report host, then it is out-of-band movement
-                        if (hostId.longValue() != powerHostId.longValue()) {
-                            s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
-                            _executor.schedule(new RebootTask(vo.getId()), 1000, TimeUnit.MICROSECONDS);
-                        }
-                    }
+        // if (vm is a router) and ((vm was stopped before) or (reprovision on out of band migration is true)) and (vm is running)
+        boolean reprovision_out_of_band = RouterReprovisionOnOutOfBandMigration.value();
+        if (
+            (vo.getType() == VirtualMachine.Type.DomainRouter) &&
+            ((oldState == State.Stopped) || (reprovision_out_of_band && isOutOfBandMigrated(opaque))) &&
+            (event == VirtualMachine.Event.FollowAgentPowerOnReport) &&
+            (newState == State.Running)) {
+                s_logger.info("Schedule a router reboot task as router " + vo.getId() + " is powered-on out-of-band. we need to reboot to refresh network rules");
+                _executor.schedule(new RebootTask(vo.getId()), 1000, TimeUnit.MICROSECONDS);
+        }
+        return true;
+    }
+
+    private boolean isOutOfBandMigrated(Object opaque) {
+        if (opaque != null && opaque instanceof Pair<?, ?>) {
+            Pair<?, ?> pair = (Pair<?, ?>)opaque;
+            Object first = pair.first();
+            Object second = pair.second();
+            if (first != null && second != null && first instanceof Long && second instanceof Long) {
+                Long hostId = (Long)first;
+                Long powerHostId = (Long)second;
+                // If VM host known to CS is different from 'PowerOn' report host, then it is out-of-band movement
+                if (hostId.longValue() != powerHostId.longValue()) {
+                    return true;
                 }
             }
         }
-        return true;
+        return false;
     }
 
     protected class RebootTask extends ManagedContextRunnable {


[2/4] git commit: updated refs/heads/4.4 to f0cc5c5

Posted by da...@apache.org.
CLOUDSTACK-8537 add check for unique public key and account on ssh keypair registration

Signed-off-by: Daan Hoogland <da...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/f294d319
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/f294d319
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/f294d319

Branch: refs/heads/4.4
Commit: f294d319c606d27935487a8222f3b24bdfb637ee
Parents: 9ced575
Author: Daan Hoogland <da...@gmail.com>
Authored: Thu Jun 4 16:48:14 2015 +0200
Committer: Daan Hoogland <da...@gmail.com>
Committed: Wed Jun 17 21:14:27 2015 +0200

----------------------------------------------------------------------
 .../src/com/cloud/user/dao/SSHKeyPairDao.java     |  2 ++
 .../src/com/cloud/user/dao/SSHKeyPairDaoImpl.java |  9 +++++++++
 .../com/cloud/server/ManagementServerImpl.java    | 18 +++++++++++-------
 3 files changed, 22 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f294d319/engine/schema/src/com/cloud/user/dao/SSHKeyPairDao.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/user/dao/SSHKeyPairDao.java b/engine/schema/src/com/cloud/user/dao/SSHKeyPairDao.java
index 7a4ac40..e035e96 100644
--- a/engine/schema/src/com/cloud/user/dao/SSHKeyPairDao.java
+++ b/engine/schema/src/com/cloud/user/dao/SSHKeyPairDao.java
@@ -35,4 +35,6 @@ public interface SSHKeyPairDao extends GenericDao<SSHKeyPairVO, Long> {
 
     public boolean deleteByName(long accountId, long domainId, String name);
 
+    public SSHKeyPairVO findByPublicKey(long accountId, long domainId, String publicKey);
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f294d319/engine/schema/src/com/cloud/user/dao/SSHKeyPairDaoImpl.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/user/dao/SSHKeyPairDaoImpl.java b/engine/schema/src/com/cloud/user/dao/SSHKeyPairDaoImpl.java
index cfaa993..51e5fbe 100644
--- a/engine/schema/src/com/cloud/user/dao/SSHKeyPairDaoImpl.java
+++ b/engine/schema/src/com/cloud/user/dao/SSHKeyPairDaoImpl.java
@@ -73,6 +73,15 @@ public class SSHKeyPairDaoImpl extends GenericDaoBase<SSHKeyPairVO, Long> implem
     }
 
     @Override
+    public SSHKeyPairVO findByPublicKey(long accountId, long domainId, String publicKey) {
+        SearchCriteria<SSHKeyPairVO> sc = createSearchCriteria();
+        sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
+        sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
+        sc.addAnd("publicKey", SearchCriteria.Op.EQ, publicKey);
+        return findOneBy(sc);
+    }
+
+    @Override
     public boolean deleteByName(long accountId, long domainId, String name) {
         SSHKeyPairVO pair = findByName(accountId, domainId, name);
         if (pair == null)

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f294d319/server/src/com/cloud/server/ManagementServerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java
index 07ea4b4..74319a3 100755
--- a/server/src/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/com/cloud/server/ManagementServerImpl.java
@@ -37,10 +37,6 @@ import javax.crypto.spec.SecretKeySpec;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd;
-import org.apache.commons.codec.binary.Base64;
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.affinity.AffinityGroupProcessor;
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
@@ -407,6 +403,7 @@ import org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotPoliciesCmd
 import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotPoliciesCmd;
 import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotsCmd;
 import org.apache.cloudstack.api.command.user.snapshot.RevertSnapshotCmd;
+import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd;
 import org.apache.cloudstack.api.command.user.ssh.CreateSSHKeyPairCmd;
 import org.apache.cloudstack.api.command.user.ssh.DeleteSSHKeyPairCmd;
 import org.apache.cloudstack.api.command.user.ssh.ListSSHKeyPairsCmd;
@@ -510,6 +507,8 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.commons.codec.binary.Base64;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.GetVncPortAnswer;
@@ -3595,9 +3594,14 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
         Account owner = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId());
 
-        SSHKeyPairVO s = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
-        if (s != null) {
-            throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' already exists.");
+        SSHKeyPairVO existingPair = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
+        if (existingPair != null) {
+            throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' already exists for this account.");
+        }
+
+        existingPair = _sshKeyPairDao.findByPublicKey(owner.getAccountId(), owner.getDomainId(), cmd.getPublicKey());
+        if (existingPair != null) {
+            throw new InvalidParameterValueException("A key pair with name '" + cmd.getPublicKey() + "' already exists for this account.");
         }
 
         String name = cmd.getName();


[4/4] git commit: updated refs/heads/4.4 to f0cc5c5

Posted by da...@apache.org.
CLOUDSTACK-8537 refactor registerSSHKeyPair() for legibility and testability reasons

Signed-off-by: Daan Hoogland <da...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/c8211312
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/c8211312
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/c8211312

Branch: refs/heads/4.4
Commit: c8211312ab4041b1300d35926f5efa35ad765f9d
Parents: f294d31
Author: Daan Hoogland <da...@gmail.com>
Authored: Tue Jun 9 23:57:00 2015 +0200
Committer: Daan Hoogland <da...@gmail.com>
Committed: Wed Jun 17 21:14:27 2015 +0200

----------------------------------------------------------------------
 .../com/cloud/server/ManagementServerImpl.java  | 110 ++++++++++++++-----
 1 file changed, 83 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c8211312/server/src/com/cloud/server/ManagementServerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java
index 74319a3..050381a 100755
--- a/server/src/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/com/cloud/server/ManagementServerImpl.java
@@ -16,7 +16,9 @@
 // under the License.
 package com.cloud.server;
 
+import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Field;
+import java.net.URLDecoder;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
@@ -938,7 +940,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public boolean archiveEvents(ArchiveEventsCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         List<Long> ids = cmd.getIds();
         boolean result = true;
         List<Long> permittedAccountIds = new ArrayList<Long>();
@@ -965,7 +967,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public boolean deleteEvents(DeleteEventsCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         List<Long> ids = cmd.getIds();
         boolean result = true;
         List<Long> permittedAccountIds = new ArrayList<Long>();
@@ -1089,8 +1091,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>> listHostsForMigrationOfVM(Long vmId, Long startIndex, Long pageSize) {
-        // access check - only root admin can migrate VM
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         if (!_accountMgr.isRootAdmin(caller.getId())) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Caller is not a root admin, permission denied to migrate the VM");
@@ -1263,8 +1264,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForMigrationOfVolume(Long volumeId) {
-        // Access check - only root administrator can migrate volumes.
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         if (!_accountMgr.isRootAdmin(caller.getId())) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Caller is not a root admin, permission denied to migrate the volume");
@@ -1746,7 +1746,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         List<Long> permittedAccounts = new ArrayList<Long>();
         ListProjectResourcesCriteria listProjectResourcesCriteria = null;
         if (isAllocated) {
-            Account caller = CallContext.current().getCallingAccount();
+            Account caller = getCaller();
 
             Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(
                     cmd.getDomainId(), cmd.isRecursive(), null);
@@ -2234,8 +2234,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             throw new InvalidParameterValueException("ROOT domain can not be edited with a new name");
         }
 
-        // check permissions
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         _accountMgr.checkAccess(caller, domain);
 
         // domain name is unique under the parent domain
@@ -3282,7 +3281,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public ArrayList<String> getCloudIdentifierResponse(long userId) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
 
         // verify that user exists
         User user = _accountMgr.getUserIncludingRemoved(userId);
@@ -3322,7 +3321,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     public Map<String, Object> listCapabilities(ListCapabilitiesCmd cmd) {
         Map<String, Object> capabilities = new HashMap<String, Object>();
 
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         boolean securityGroupsEnabled = false;
         boolean elasticLoadBalancerEnabled = false;
         boolean KVMSnapshotEnabled = false;
@@ -3388,7 +3387,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public InstanceGroupVO updateVmGroup(UpdateVMGroupCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         Long groupId = cmd.getId();
         String groupName = cmd.getGroupName();
 
@@ -3506,7 +3505,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public SSHKeyPair createSSHKeyPair(CreateSSHKeyPairCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         String accountName = cmd.getAccountName();
         Long domainId = cmd.getDomainId();
         Long projectId = cmd.getProjectId();
@@ -3530,7 +3529,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public boolean deleteSSHKeyPair(DeleteSSHKeyPairCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         String accountName = cmd.getAccountName();
         Long domainId = cmd.getDomainId();
         Long projectId = cmd.getProjectId();
@@ -3558,7 +3557,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         String name = cmd.getName();
         String fingerPrint = cmd.getFingerprint();
 
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
         List<Long> permittedAccounts = new ArrayList<Long>();
 
         Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(
@@ -3590,30 +3589,87 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_REGISTER_SSH_KEYPAIR, eventDescription = "registering ssh keypair", async = true)
     public SSHKeyPair registerSSHKeyPair(RegisterSSHKeyPairCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account owner = getOwner(cmd);
+        checkForKeyByName(cmd, owner);
+        checkForKeyByPublicKey(cmd, owner);
 
-        Account owner = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId());
+        String name = cmd.getName();
+        String key = cmd.getPublicKey();
+        try {
+            key = URLDecoder.decode(key, "UTF-8");
+        } catch (UnsupportedEncodingException e) {
+            s_logger.warn("key decoding tried invain: " + e.getLocalizedMessage());
+        }
+        String publicKey = getPublicKeyFromKeyKeyMaterial(key);
+        String fingerprint = getFingerprint(publicKey);
 
-        SSHKeyPairVO existingPair = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
+        return createAndSaveSSHKeyPair(name, fingerprint, publicKey, null, owner);
+    }
+
+    /**
+     * @param cmd
+     * @param owner
+     * @throws InvalidParameterValueException
+     */
+    private void checkForKeyByPublicKey(RegisterSSHKeyPairCmd cmd, Account owner) throws InvalidParameterValueException {
+        SSHKeyPairVO existingPair = _sshKeyPairDao.findByPublicKey(owner.getAccountId(), owner.getDomainId(), cmd.getPublicKey());
         if (existingPair != null) {
-            throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' already exists for this account.");
+            throw new InvalidParameterValueException("A key pair with name '" + cmd.getPublicKey() + "' already exists for this account.");
         }
+    }
 
-        existingPair = _sshKeyPairDao.findByPublicKey(owner.getAccountId(), owner.getDomainId(), cmd.getPublicKey());
+    /**
+     * @param cmd
+     * @param owner
+     * @throws InvalidParameterValueException
+     */
+    private void checkForKeyByName(RegisterSSHKeyPairCmd cmd, Account owner) throws InvalidParameterValueException {
+        SSHKeyPairVO existingPair = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
         if (existingPair != null) {
-            throw new InvalidParameterValueException("A key pair with name '" + cmd.getPublicKey() + "' already exists for this account.");
+            throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' already exists for this account.");
         }
+    }
 
-        String name = cmd.getName();
-        String publicKey = SSHKeysHelper.getPublicKeyFromKeyMaterial(cmd.getPublicKey());
+    /**
+     * @param publicKey
+     * @return
+     */
+    private String getFingerprint(String publicKey) {
+        String fingerprint = SSHKeysHelper.getPublicKeyFingerprint(publicKey);
+        return fingerprint;
+    }
+
+    /**
+     * @param key
+     * @return
+     * @throws InvalidParameterValueException
+     */
+    private String getPublicKeyFromKeyKeyMaterial(String key) throws InvalidParameterValueException {
+        String publicKey = SSHKeysHelper.getPublicKeyFromKeyMaterial(key);
 
         if (publicKey == null) {
             throw new InvalidParameterValueException("Public key is invalid");
         }
+        return publicKey;
+    }
 
-        String fingerprint = SSHKeysHelper.getPublicKeyFingerprint(publicKey);
+    /**
+     * @param cmd
+     * @return
+     */
+    private Account getOwner(RegisterSSHKeyPairCmd cmd) {
+        Account caller = getCaller();
 
-        return createAndSaveSSHKeyPair(name, fingerprint, publicKey, null, owner);
+        Account owner = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId());
+        return owner;
+    }
+
+    /**
+     * @return
+     */
+    private Account getCaller() {
+        Account caller = CallContext.current().getCallingAccount();
+        return caller;
     }
 
     private SSHKeyPair createAndSaveSSHKeyPair(String name, String fingerprint, String publicKey, String privateKey, Account owner) {
@@ -3633,7 +3689,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public String getVMPassword(GetVMPasswordCmd cmd) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
 
         UserVmVO vm = _userVmDao.findById(cmd.getId());
         if (vm == null) {
@@ -3809,7 +3865,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     }
 
     private VirtualMachine upgradeStoppedSystemVm(Long systemVmId, Long serviceOfferingId, Map<String, String> customparameters) {
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCaller();
 
         VMInstanceVO systemVm = _vmInstanceDao.findByIdTypes(systemVmId, VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm);
         if (systemVm == null) {


[3/4] git commit: updated refs/heads/4.4 to f0cc5c5

Posted by da...@apache.org.
CLOUDSTACK-8537 test for the sake of testing the fix seems so trivial but no testing is available for it at all. when bugs arise test extension should be the start point here.

Signed-off-by: Daan Hoogland <da...@gmail.com>

This closes #357


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/f0cc5c51
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/f0cc5c51
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/f0cc5c51

Branch: refs/heads/4.4
Commit: f0cc5c510ccaf83241b9a2ef750aad00ee355f10
Parents: c821131
Author: Daan Hoogland <da...@gmail.com>
Authored: Mon Jun 15 16:11:36 2015 +0200
Committer: Daan Hoogland <da...@gmail.com>
Committed: Wed Jun 17 21:14:27 2015 +0200

----------------------------------------------------------------------
 .../com/cloud/server/ManagementServerImpl.java  |  8 +--
 .../cloud/server/ManagementServerImplTest.java  | 67 ++++++++++++++++++++
 2 files changed, 71 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f0cc5c51/server/src/com/cloud/server/ManagementServerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java
index 050381a..05b089a 100755
--- a/server/src/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/com/cloud/server/ManagementServerImpl.java
@@ -743,7 +743,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Inject
     private InstanceGroupDao _vmGroupDao;
     @Inject
-    private SSHKeyPairDao _sshKeyPairDao;
+    protected SSHKeyPairDao _sshKeyPairDao;
     @Inject
     private LoadBalancerDao _loadbalancerDao;
     @Inject
@@ -3623,7 +3623,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
      * @param owner
      * @throws InvalidParameterValueException
      */
-    private void checkForKeyByName(RegisterSSHKeyPairCmd cmd, Account owner) throws InvalidParameterValueException {
+    protected void checkForKeyByName(RegisterSSHKeyPairCmd cmd, Account owner) throws InvalidParameterValueException {
         SSHKeyPairVO existingPair = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
         if (existingPair != null) {
             throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' already exists for this account.");
@@ -3657,7 +3657,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
      * @param cmd
      * @return
      */
-    private Account getOwner(RegisterSSHKeyPairCmd cmd) {
+    protected Account getOwner(RegisterSSHKeyPairCmd cmd) {
         Account caller = getCaller();
 
         Account owner = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId());
@@ -3667,7 +3667,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     /**
      * @return
      */
-    private Account getCaller() {
+    protected Account getCaller() {
         Account caller = CallContext.current().getCallingAccount();
         return caller;
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f0cc5c51/server/test/com/cloud/server/ManagementServerImplTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/server/ManagementServerImplTest.java b/server/test/com/cloud/server/ManagementServerImplTest.java
new file mode 100644
index 0000000..1e530e6
--- /dev/null
+++ b/server/test/com/cloud/server/ManagementServerImplTest.java
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.server;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import org.apache.cloudstack.api.command.user.ssh.RegisterSSHKeyPairCmd;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.user.Account;
+import com.cloud.user.SSHKeyPairVO;
+import com.cloud.user.dao.SSHKeyPairDao;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ManagementServerImplTest {
+
+    @Mock
+    RegisterSSHKeyPairCmd regCmd;
+    @Mock
+    SSHKeyPairVO existingPair;
+    @Mock
+    Account account;
+    @Mock
+    SSHKeyPairDao sshKeyPairDao;
+    ManagementServerImpl ms = new ManagementServerImpl();
+    @Spy
+    ManagementServerImpl spy;
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testExistingPairRegistration() {
+        String accountName = "account";
+        String publicKeyString = "very public";
+        // setup owner with domainid
+        Mockito.doReturn(account).when(spy).getCaller();
+        Mockito.doReturn(account).when(spy).getOwner(regCmd);
+        // mock _sshKeyPairDao.findByName to return null
+        Mockito.doNothing().when(spy).checkForKeyByName(regCmd, account);
+        // mock _sshKeyPairDao.findByPublicKey to return a known object
+        Mockito.doReturn(accountName).when(regCmd).getAccountName();
+        Mockito.doReturn(publicKeyString).when(regCmd).getPublicKey();
+        Mockito.doReturn("name").when(regCmd).getName();
+        spy._sshKeyPairDao = sshKeyPairDao;
+        Mockito.doReturn(1L).when(account).getAccountId();
+        Mockito.doReturn(1L).when(account).getDomainId();
+        Mockito.doReturn(existingPair).when(sshKeyPairDao).findByPublicKey(1L, 1L, publicKeyString);
+        spy.registerSSHKeyPair(regCmd);
+    }
+}