From a9accd331a2af63e3ee493ef9bf2a1225c6ccd71 Mon Sep 17 00:00:00 2001
From: Jayapal <jayapal@apache.org>
Date: Wed, 9 Apr 2014 16:38:02 +0530
Subject: [PATCH] CLOUDSTACK-6364 Added ip address validation

Also updated to assign vm primary ip to lb rule when vmid is passed virtualmachineid
and vm id ip details in vmidipmap
---
 .../AssignToLoadBalancerRuleCmd.java          |  6 ++
 .../lb/LoadBalancingRulesManagerImpl.java     | 61 +++++++++++--------
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java
index a5199937f4e..db4d70e685a 100644
--- a/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java
@@ -23,6 +23,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import com.cloud.utils.net.NetUtils;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@@ -142,6 +143,11 @@ public class AssignToLoadBalancerRuleCmd extends BaseAsyncCmd {
                     throw new InvalidParameterValueException("Unable to find virtual machine ID: " + vmId);
                 }
 
+                //check wether the given ip is valid ip or not
+                if (vmIp == null || !NetUtils.isValidIp(vmIp)) {
+                    throw new InvalidParameterValueException("Invalid ip address "+ vmIp +" passed in vmidipmap for " +
+                            "vmid " + vmId);
+                }
                 Long longVmId = lbvm.getId();
 
                 List<String> ipsList = null;
diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
index e5791a90f03..7c1517133df 100755
--- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
+++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
@@ -986,20 +986,6 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements
         // check for conflict
         Set<Long> passedInstanceIds = vmIdIpMap.keySet();
         for (Long instanceId : passedInstanceIds) {
-            if (existingVmIdIps.containsKey(instanceId)) {
-                // now check for ip address
-                List<String> mappedIps = existingVmIdIps.get(instanceId);
-                List<String> newIps = vmIdIpMap.get(instanceId);
-
-                if (newIps !=  null) {
-                    for (String newIp: newIps) {
-                        if (mappedIps.contains(newIp)) {
-                            throw new InvalidParameterValueException("VM " + instanceId + " with " + newIp +" is already mapped to load balancer.");
-                        }
-                    }
-                }
-            }
-
             UserVm vm = _vmDao.findById(instanceId);
             if (vm == null || vm.getState() == State.Destroyed || vm.getState() == State.Expunging) {
                 InvalidParameterValueException ex = new InvalidParameterValueException("Invalid instance id specified");
@@ -1036,28 +1022,55 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements
             }
 
             String priIp = nicInSameNetwork.getIp4Address();
+
+            if (existingVmIdIps.containsKey(instanceId)) {
+                // now check for ip address
+                List<String> mappedIps = existingVmIdIps.get(instanceId);
+                List<String> newIps = vmIdIpMap.get(instanceId);
+
+                if (newIps == null) {
+                    newIps = new ArrayList<String>();
+                    newIps.add(priIp);
+                }
+
+                for (String newIp: newIps) {
+                    if (mappedIps.contains(newIp)) {
+                        throw new InvalidParameterValueException("VM " + instanceId + " with " + newIp +" is already mapped to load balancer.");
+                    }
+                }
+            }
+
             List<String> vmIpsList = vmIdIpMap.get(instanceId);
             String vmLbIp = null;
 
-            if (vmIpsList == null) {
-                vmIpsList = new ArrayList<String>();
-                vmIpsList.add(priIp);
-                vmIdIpMap.put(instanceId, vmIpsList);
-            } else {
-                // skip the primary ip from vm secondary ip comparisions
-                if (vmIpsList.contains(priIp)) {
-                    vmIpsList.remove(priIp);
-                }
+            if (vmIpsList != null) {
 
                 //check if the ips belongs to nic secondary ip
                 for (String ip: vmIpsList) {
+                    // skip the primary ip from vm secondary ip comparisions
+                    if (ip.equals(priIp)) {
+                        continue;
+                    }
                     if(_nicSecondaryIpDao.findByIp4AddressAndNicId(ip,nicInSameNetwork.getId()) == null) {
                         throw new InvalidParameterValueException("VM ip "+ ip + " specified does not belong to " +
                                 "nic in network " + nicInSameNetwork.getNetworkId());
                     }
                 }
+            } else {
+                vmIpsList = new ArrayList<String>();
+                vmIpsList.add(priIp);
             }
 
+            // when vm id is passed in instance ids and in vmidipmap
+            // assign for primary ip and ip passed in vmidipmap
+            if (instanceIds != null ) {
+                if (instanceIds.contains(instanceId)) {
+                    vmIpsList.add(priIp);
+                }
+            }
+
+            vmIdIpMap.put(instanceId, vmIpsList);
+
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Adding " + vm + " to the load balancer pool");
             }
@@ -1072,7 +1085,7 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements
             public void doInTransactionWithoutResult(TransactionStatus status) {
 
                 for (Long vmId : vmIds) {
-                    final List<String> lbVmIps = newMap.get(vmId);
+                    final Set<String> lbVmIps = new HashSet<String>(newMap.get(vmId));
                     for (String vmIp: lbVmIps) {
                         LoadBalancerVMMapVO map = new LoadBalancerVMMapVO(loadBalancer.getId(), vmId, vmIp, false);
                         map = _lb2VmMapDao.persist(map);