You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/07/27 18:47:42 UTC

[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #5215: Support for multiple SSHKeys in resetsshkeyforvirtualmachine api

GutoVeronezi commented on a change in pull request #5215:
URL: https://github.com/apache/cloudstack/pull/5215#discussion_r677709227



##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -992,9 +992,9 @@
             sb.and("affinityGroupId", sb.entity().getAffinityGroupId(), SearchCriteria.Op.EQ);
         }
 
-        if (keyPairName != null) {
-            sb.and("keyPairName", sb.entity().getKeypairName(), SearchCriteria.Op.EQ);
-        }
+        //if (keyPairName != null) {
+        //    sb.and("keyPairName", sb.entity().getKeypairName(), SearchCriteria.Op.EQ);
+        //}

Review comment:
       Do we need this commented code?

##########
File path: server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java
##########
@@ -325,6 +325,10 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us
         if (vmDetails != null) {
             Map<String, String> resourceDetails = new HashMap<String, String>();
             for (UserVmDetailVO userVmDetailVO : vmDetails) {
+                if (userVmDetailVO.getName().startsWith(VmDetailConstants.KEY_PAIR_NAMES)) {
+                    s_logger.info(userVmDetailVO.getValue());

Review comment:
       We could improve this log with some context.

##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -862,17 +862,52 @@ public UserVm resetVMSSHKey(ResetVMSSHKeyCmd cmd) throws ResourceUnavailableExce
             throw new InvalidParameterValueException("Vm " + userVm + " should be stopped to do SSH Key reset");
         }
 
-        SSHKeyPairVO s = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
-        if (s == null) {
-            throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' does not exist for account " + owner.getAccountName()
-            + " in specified domain id");
+        if (cmd.getName() == null && cmd.getNames() == null) {

Review comment:
       `org.apache.commons.lang3.ObjectUtils` has a method to verify nulls `anyNull(...)`.

##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -862,17 +862,52 @@ public UserVm resetVMSSHKey(ResetVMSSHKeyCmd cmd) throws ResourceUnavailableExce
             throw new InvalidParameterValueException("Vm " + userVm + " should be stopped to do SSH Key reset");
         }
 
-        SSHKeyPairVO s = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
-        if (s == null) {
-            throw new InvalidParameterValueException("A key pair with name '" + cmd.getName() + "' does not exist for account " + owner.getAccountName()
-            + " in specified domain id");
+        if (cmd.getName() == null && cmd.getNames() == null) {
+            throw new InvalidParameterValueException("No keypair name given");
+        }
+
+        String keypairnames = "";
+        SSHKeyPairVO s = null;
+        if (cmd.getName() != null) {
+            s = _sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), cmd.getName());
+            keypairnames = keypairnames + cmd.getName();
+        }
+
+        List<SSHKeyPairVO> s_list = null;
+        if (cmd.getNames() != null) {
+            s_list = _sshKeyPairDao.findByNames(owner.getAccountId(), owner.getDomainId(), cmd.getNames());
+            for (String keypairname : cmd.getNames()) {
+                if (keypairnames != "") {
+                    keypairnames = keypairnames + ", ";
+                }
+                keypairnames = keypairnames + keypairname;
+            }
+        }
+        if (s_list == null && s == null) {

Review comment:
       `org.apache.commons.lang3.ObjectUtils` has a method to verify nulls `anyNull(...)`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org