This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
     new 5690cd6  server: fix the subnet overlap checking logic for tagged and 
untagged vlans when adding ipranges (#3430)
5690cd6 is described below

commit 5690cd636427eb70ea09a732bf75163ec77af15e
Author: Anurag Awasthi <[email protected]>
AuthorDate: Tue Jul 23 00:27:50 2019 +0530

    server: fix the subnet overlap checking logic for tagged and untagged vlans 
when adding ipranges (#3430)
    
    Fixes: #3114
    
    When adding iprange for VLANs there are 3 cases -
    
    VLAN under consideration has a tag (like 101)
    VLAN under consideration has a tag but as a range (like 101-124)
    VLAN is untagged (i.e. id is "untagged")
    Before adding iprange we have to check for possible overlaps and throw 
exception. This needs to be done as follows -
    
    If VLAN Tag ID is numeric or a range we need to call 
UriUtils.checkVlanUriOverlapmethod which internally tries to expand the range 
as verifies if there are overlaps. If URI overlaps (i.e. there are overlapping 
VLAN tags) we then need to verify if the iprange being added overlaps with 
previously added ranges.
    If there are no overlapping tags we simply need to test for public networks 
being present in the VLAN.
    A Regression was introduced in 
41fdb88#diff-6e2b61984e8fa2823bb47da3caafa4eeR3174 which caused comparing 
'untagged' string as a numeric VLAN Tag range and and attempted expanding it to 
test overlap in UriUtils.checkVlanUriOverlap.
    
    To fix the bug in the issue, we need to handle the untagged case separately 
as it's non-numeric tag in code. For untagged VLANs and overlapping VLAN URIs 
we need to check for ipranges and gateways which happens naturally after this 
change. For tagged VLANs with non-overlapping URIs we need to check if there is 
a public network.
---
 .../configuration/ConfigurationManagerImpl.java    | 45 +++++++++++-----------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git 
a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java 
b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index bf238b0..2c71f0d 100755
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -3815,29 +3815,10 @@ public class ConfigurationManagerImpl extends 
ManagerBase implements Configurati
                     continue;
                 }
                 // from here, subnet overlaps
-                if (!UriUtils.checkVlanUriOverlap(
+                if (vlanId.toLowerCase().contains(Vlan.UNTAGGED) || 
UriUtils.checkVlanUriOverlap(
                         
BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId)),
                         
BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlan.getVlanTag()))))
 {
-                    boolean overlapped = false;
-                    if( network.getTrafficType() == TrafficType.Public ) {
-                        overlapped = true;
-                    } else {
-                        final Long nwId = vlan.getNetworkId();
-                        if ( nwId != null ) {
-                            final Network nw = _networkModel.getNetwork(nwId);
-                            if ( nw != null && nw.getTrafficType() == 
TrafficType.Public ) {
-                                overlapped = true;
-                            }
-                        }
-
-                    }
-                    if ( overlapped ) {
-                        throw new InvalidParameterValueException("The IP range 
with tag: " + vlan.getVlanTag()
-                                + " in zone " + zone.getName()
-                                + " has overlapped with the subnet. Please 
specify a different gateway/netmask.");
-                    }
-                } else {
-
+                    // For untagged VLAN Id and overlapping URIs we need to 
expand and verify IP ranges
                     final String[] otherVlanIpRange = 
vlan.getIpRange().split("\\-");
                     final String otherVlanStartIP = otherVlanIpRange[0];
                     String otherVlanEndIP = null;
@@ -3854,9 +3835,29 @@ public class ConfigurationManagerImpl extends 
ManagerBase implements Configurati
                     if (!NetUtils.is31PrefixCidr(newCidr)) {
                         if (NetUtils.ipRangesOverlap(startIP, endIP, 
otherVlanStartIP, otherVlanEndIP)) {
                             throw new InvalidParameterValueException("The IP 
range already has IPs that overlap with the new range." +
-                                " Please specify a different start IP/end 
IP.");
+                                    " Please specify a different start IP/end 
IP.");
                         }
                     }
+                } else {
+                    // For tagged or non-overlapping URIs we need to ensure 
there is no Public traffic type
+                    boolean overlapped = false;
+                    if (network.getTrafficType() == TrafficType.Public) {
+                        overlapped = true;
+                    } else {
+                        final Long nwId = vlan.getNetworkId();
+                        if (nwId != null) {
+                            final Network nw = _networkModel.getNetwork(nwId);
+                            if (nw != null && nw.getTrafficType() == 
TrafficType.Public) {
+                                overlapped = true;
+                            }
+                        }
+
+                    }
+                    if (overlapped) {
+                        throw new InvalidParameterValueException("The IP range 
with tag: " + vlan.getVlanTag()
+                                + " in zone " + zone.getName()
+                                + " has overlapped with the subnet. Please 
specify a different gateway/netmask.");
+                    }
                 }
             }
         }

Reply via email to