Skip to content

Commit ca13586

Browse files
bvbharatAbhinandan Prateek
bvbharat
authored and
Abhinandan Prateek
committed
Cloudstack-2511 Multiple_Ip_Ranges: Adding guest ip range in subset/superset to existing CIDR is allowed Cloudstack-2651 [Shared n/w]Add IP range should ask for gateway and netmask
Signed-off-by: Abhinandan Prateek <aprateek@apache.org>
1 parent 2ed17c7 commit ca13586

3 files changed

Lines changed: 116 additions & 45 deletions

File tree

‎server/src/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import javax.naming.directory.DirContext;
4040
import javax.naming.directory.InitialDirContext;
4141

42+
import com.cloud.utils.Pair;
4243
import org.apache.cloudstack.acl.SecurityChecker;
4344
import org.apache.cloudstack.api.ApiConstants.LDAPParams;
4445
import org.apache.cloudstack.api.command.admin.config.UpdateCfgCmd;
@@ -2473,7 +2474,7 @@ public Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws Insuffic
24732474
}
24742475
}
24752476

2476-
boolean sameSubnet=false;
2477+
Pair<Boolean,Pair<String,String>> sameSubnet= null;
24772478
// Can add vlan range only to the network which allows it
24782479
if (!network.getSpecifyIpRanges()) {
24792480
throw new InvalidParameterValueException("Network " + network + " doesn't support adding ip ranges");
@@ -2508,7 +2509,7 @@ else if (zone.getNetworkType() == NetworkType.Basic){
25082509
sameSubnet=validateIpRange(startIP,endIP,newVlanGateway, newVlanNetmask, vlans, ipv4, ipv6, ip6Gateway, ip6Cidr, startIPv6, endIPv6, network);
25092510
}
25102511

2511-
if (zoneId == null || (ipv4 && (newVlanGateway == null || newVlanNetmask == null)) || (ipv6 && (ip6Gateway == null || ip6Cidr == null))) {
2512+
if (zoneId == null || (ipv6 && (ip6Gateway == null || ip6Cidr == null))) {
25122513
throw new InvalidParameterValueException("Gateway, netmask and zoneId have to be passed in for virtual and direct untagged networks");
25132514
}
25142515

@@ -2527,54 +2528,80 @@ else if (zone.getNetworkType() == NetworkType.Basic){
25272528
}
25282529
Transaction txn = Transaction.currentTxn();
25292530
txn.start();
2530-
2531+
if (sameSubnet.first() == false) {
2532+
s_logger.info("adding a new subnet to the network "+network.getId());
2533+
}
2534+
else {
2535+
// if it is same subnet the user might not send the vlan and the netmask details. so we are
2536+
//figuring out while validation and setting them here.
2537+
newVlanGateway = sameSubnet.second().first();
2538+
newVlanNetmask = sameSubnet.second().second();
2539+
}
25312540
Vlan vlan = createVlanAndPublicIpRange(zoneId, networkId, physicalNetworkId, forVirtualNetwork, podId, startIP,
25322541
endIP, newVlanGateway, newVlanNetmask, vlanId, vlanOwner, startIPv6, endIPv6, ip6Gateway, ip6Cidr);
25332542
//create an entry in the nic_secondary table. This will be the new gateway that will be configured on the corresponding routervm.
2534-
if (sameSubnet == false) {
2535-
s_logger.info("adding a new subnet to the network "+network.getId());
2536-
}
2543+
25372544

25382545
txn.commit();
25392546

25402547
return vlan;
25412548
}
25422549

2543-
public boolean validateIpRange(String startIP, String endIP, String newVlanGateway, String newVlanNetmask, List<VlanVO> vlans, boolean ipv4, boolean ipv6, String ip6Gateway, String ip6Cidr, String startIPv6, String endIPv6, Network network) {
2544-
String vlanGateway;
2545-
String vlanNetmask;
2550+
public int checkIfSubsetOrSuperset(String newVlanGateway, String newVlanNetmask, VlanVO vlan, String startIP, String endIP) {
2551+
if (newVlanGateway == null && newVlanNetmask==null) {
2552+
newVlanGateway = vlan.getVlanGateway();
2553+
newVlanNetmask = vlan.getVlanNetmask();
2554+
//this means he is trying to add to the existing subnet.
2555+
if (NetUtils.sameSubnet(startIP, newVlanGateway, newVlanNetmask)) {
2556+
if (NetUtils.sameSubnet(endIP, newVlanGateway, newVlanNetmask)){
2557+
return 3;
2558+
}
2559+
}
2560+
return 0;
2561+
}
2562+
else if (newVlanGateway == null || newVlanGateway ==null){
2563+
throw new InvalidParameterValueException("either both netmask and gateway should be passed or both should me omited.");
2564+
}
2565+
else {
2566+
if (!NetUtils.sameSubnet(startIP, newVlanGateway, newVlanNetmask)) {
2567+
throw new InvalidParameterValueException("The start ip and gateway do not belong to the same subnet");
2568+
}
2569+
if (!NetUtils.sameSubnet(endIP, newVlanGateway, newVlanNetmask)) {
2570+
throw new InvalidParameterValueException("The end ip and gateway do not belong to the same subnet");
2571+
}
2572+
}
2573+
String cidrnew = NetUtils.getCidrFromGatewayAndNetmask(newVlanGateway, newVlanNetmask);
2574+
String existing_cidr = NetUtils.getCidrFromGatewayAndNetmask(vlan.getVlanGateway(), vlan.getVlanNetmask());
2575+
2576+
return (NetUtils.isNetowrkASubsetOrSupersetOfNetworkB(cidrnew, existing_cidr));
2577+
}
2578+
2579+
public Pair<Boolean,Pair<String,String>> validateIpRange(String startIP, String endIP, String newVlanGateway, String newVlanNetmask, List<VlanVO> vlans, boolean ipv4, boolean ipv6, String ip6Gateway, String ip6Cidr, String startIPv6, String endIPv6, Network network) {
2580+
String vlanGateway=null;
2581+
String vlanNetmask=null;
25462582
boolean sameSubnet = false;
25472583
if ( vlans != null && vlans.size() > 0 ) {
2548-
25492584
for (VlanVO vlan : vlans) {
25502585
if (ipv4) {
25512586
vlanGateway = vlan.getVlanGateway();
25522587
vlanNetmask = vlan.getVlanNetmask();
2553-
// Check if ip addresses are in network range
2554-
if (!NetUtils.sameSubnet(startIP, vlanGateway, vlanNetmask)) {
2555-
if (!NetUtils.sameSubnet(endIP, vlanGateway, vlanNetmask)) {
2556-
// check if the the new subnet is not a superset of the existing subnets.
2557-
if (NetUtils.isNetworkAWithinNetworkB(NetUtils.getCidrFromGatewayAndNetmask(vlanGateway,vlanNetmask), NetUtils.ipAndNetMaskToCidr(startIP, newVlanNetmask))){
2558-
throw new InvalidParameterValueException ("The new subnet is a superset of the existing subnet");
2559-
}
2560-
// check if the new subnet is not a subset of the existing subnet.
2561-
if (NetUtils.isNetworkAWithinNetworkB(NetUtils.ipAndNetMaskToCidr(startIP, newVlanNetmask), NetUtils.getCidrFromGatewayAndNetmask(vlanGateway,vlanNetmask))){
2562-
throw new InvalidParameterValueException("The new subnet is a subset of the existing subnet");
2563-
}
2564-
}
2565-
} else if (NetUtils.sameSubnet(endIP, vlanGateway, vlanNetmask)){
2566-
// trying to add to the same subnet.
2567-
sameSubnet = true;
2568-
if (newVlanGateway == null) {
2569-
newVlanGateway = vlanGateway;
2570-
}
2571-
if (!newVlanGateway.equals(vlanGateway)){
2572-
throw new InvalidParameterValueException("The gateway of the ip range is not same as the gateway of the subnet.");
2573-
}
2574-
break;
2588+
//check if subset or super set or neither.
2589+
int val = checkIfSubsetOrSuperset(newVlanGateway, newVlanNetmask, vlan, startIP, endIP);
2590+
if (val == 1) {
2591+
// this means that new cidr is a superset of the existing subnet.
2592+
throw new InvalidParameterValueException("The subnet you are trying to add is a superset of the existing subnet having gateway"+vlan.getVlanGateway()+" and netmask "+vlan.getVlanNetmask());
2593+
}
2594+
else if (val == 0) {
2595+
//this implies the user is trying to add a new subnet which is not a superset or subset of this subnet.
2596+
//checking with the other subnets.
2597+
continue;
2598+
}
2599+
else if (val == 2) {
2600+
//this means he is trying to add to the same subnet.
2601+
throw new InvalidParameterValueException("The subnet you are trying to add is a subset of the existing subnet having gateway"+vlan.getVlanGateway()+" and netmask "+vlan.getVlanNetmask());
25752602
}
2576-
else {
2577-
throw new InvalidParameterValueException("Start ip and End ip is not in vlan range!");
2603+
else if (val == 3) {
2604+
sameSubnet =true;
25782605
}
25792606
}
25802607
if (ipv6) {
@@ -2589,13 +2616,25 @@ public boolean validateIpRange(String startIP, String endIP, String newVlanGatew
25892616
_networkModel.checkIp6Parameters(startIPv6, endIPv6, ip6Gateway, ip6Cidr);
25902617
}
25912618
}
2592-
if (sameSubnet == false) {
2593-
if (newVlanGateway ==null) {
2594-
throw new MissingParameterValueException("The gateway for the new subnet is not specified.");
2595-
}
2596-
}
25972619
}
2598-
return sameSubnet;
2620+
if (newVlanGateway==null && newVlanNetmask ==null && sameSubnet == false) {
2621+
throw new InvalidParameterValueException("The ip range dose not belong to any of the existing subnets, Provide the netmask and gateway if you want to add new subnet");
2622+
}
2623+
Pair<String,String> vlanDetails=null;
2624+
2625+
if (sameSubnet){
2626+
vlanDetails = new Pair<String, String>(vlanGateway, vlanNetmask);
2627+
}
2628+
else {
2629+
vlanDetails = new Pair<String, String>(newVlanGateway, newVlanNetmask);
2630+
}
2631+
//check if the gatewayip is the part of the ip range being added.
2632+
if (NetUtils.ipRangesOverlap(startIP, endIP, vlanDetails.first(), vlanDetails.first())) {
2633+
throw new InvalidParameterValueException("The gateway ip should not be the part of the ip range being added.");
2634+
}
2635+
2636+
Pair<Boolean,Pair<String,String>> result = new Pair<Boolean,Pair<String,String>>(sameSubnet, vlanDetails);
2637+
return result;
25992638
}
26002639

26012640
@Override

‎server/test/com/cloud/configuration/ValidateIpRangeTest.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.cloud.dc.VlanVO;
2020
import com.cloud.network.Network;
2121
import com.cloud.network.NetworkModel;
22+
import com.cloud.utils.Pair;
2223
import org.junit.Assert;
2324
import org.junit.Before;
2425
import org.junit.Test;
@@ -49,20 +50,20 @@ public void setup() {
4950

5051
@Test
5152
public void SameSubnetTest() {
52-
boolean sameSubnet=configurationMgr.validateIpRange("10.147.33.104", "10.147.33.105", "10.147.33.1", "255.255.255.128", vlanVOList, true, false, null, null, null, null,network);
53-
Assert.assertTrue(sameSubnet);
53+
Pair<Boolean,Pair<String,String>> sameSubnet=configurationMgr.validateIpRange("10.147.33.104", "10.147.33.105", "10.147.33.1", "255.255.255.128", vlanVOList, true, false, null, null, null, null,network);
54+
Assert.assertTrue(sameSubnet.first());
5455
}
5556

5657
@Test
5758
public void NewSubnetTest() {
58-
boolean sameSubnet= configurationMgr.validateIpRange("10.147.33.140", "10.147.33.145", "10.147.33.129", "255.255.255.191", vlanVOList, true, false, null, null, null, null,network);
59-
Assert.assertTrue(!sameSubnet);
59+
Pair<Boolean,Pair<String,String>> sameSubnet= configurationMgr.validateIpRange("10.147.33.140", "10.147.33.145", "10.147.33.129", "255.255.255.191", vlanVOList, true, false, null, null, null, null,network);
60+
Assert.assertTrue(!sameSubnet.first());
6061
}
6162

6263
@Test
6364
public void SuperSetTest() {
6465
try {
65-
configurationMgr.validateIpRange("10.147.33.140", "10.147.33.143", "10.147.33.140", "255.255.255.191", vlanVOList, true, false, null, null, null, null,network);
66+
configurationMgr.validateIpRange("10.147.33.10", "10.147.33.20", "10.147.33.21", "255.255.255.127", vlanVOList, true, false, null, null, null, null,network);
6667
} catch (Exception e) {
6768
junit.framework.Assert.assertTrue(e.getMessage().contains("superset"));
6869
}

‎utils/src/com/cloud/utils/net/NetUtils.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,37 @@ public static Pair<String, Integer> getCidr(String cidr) {
795795
return new Pair<String, Integer>(tokens[0], Integer.parseInt(tokens[1]));
796796
}
797797

798+
public static int isNetowrkASubsetOrSupersetOfNetworkB (String cidrA, String cidrB) {
799+
Long[] cidrALong = cidrToLong(cidrA);
800+
Long[] cidrBLong = cidrToLong(cidrB);
801+
long shift =0;
802+
if (cidrALong == null || cidrBLong == null) {
803+
//implies error in the cidr format
804+
return -1;
805+
}
806+
if (cidrALong[1] >= cidrBLong[1]) {
807+
shift = 32 - cidrBLong[1];
808+
}
809+
else {
810+
shift = 32 - cidrALong[1];
811+
}
812+
long result = (cidrALong[0] >> shift) - (cidrBLong[0] >> shift);
813+
if (result == 0) {
814+
if (cidrALong[1] < cidrBLong[1]) {
815+
//this implies cidrA is super set of cidrB
816+
return 1;
817+
}
818+
else if (cidrALong[1] == cidrBLong[1]) {
819+
//this implies both the cidrs are equal
820+
return 3;
821+
}
822+
// implies cidrA is subset of cidrB
823+
return 2;
824+
}
825+
//this implies no overlap.
826+
return 0;
827+
}
828+
798829
public static boolean isNetworkAWithinNetworkB(String cidrA, String cidrB) {
799830
Long[] cidrALong = cidrToLong(cidrA);
800831
Long[] cidrBLong = cidrToLong(cidrB);

0 commit comments

Comments
 (0)