Fixing vlan allowed to be created with existing pvlan id

davidjumani · davidjumani · commit 2cc200d07f94 · 2020-08-17T14:06:41.000+05:30
diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@ -2498,6 +2498,12 @@ public Network doInTransaction(final TransactionStatus status) {
                         } else {
                             uri = BroadcastDomainType.fromString(vlanIdFinal);
                         }
+
+                        if (_networksDao.listByPhysicalNetworkPvlan(physicalNetworkId, uri.toString()).size() > 0) {
+                            throw new InvalidParameterValueException("Network with vlan " + vlanIdFinal +
+                                " already exists or overlaps with other network pvlans in zone " + zoneId);
+                        }
+
                         userNetwork.setBroadcastUri(uri);
                         if (!vlanIdFinal.equalsIgnoreCase(Vlan.UNTAGGED)) {
                             userNetwork.setBroadcastDomainType(BroadcastDomainType.Vlan);
diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java
@@ -126,4 +126,6 @@ public interface NetworkDao extends GenericDao<NetworkVO, Long>, StateDao<State,
     List<NetworkVO> listByAccountIdNetworkName(long accountId, String name);
 
     List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType);
+
+    List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri);
 }
diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java
@@ -761,6 +761,11 @@ protected boolean isNetworkOverlappingRequestedPvlan(Integer existingPrimaryVlan
         return exactMatch || secondaryVlanUsed || primaryVlanUsed || isolatedMax || promiscuousMax;
     }
 
+    // True when a VLAN ID overlaps with an existing PVLAN primary or secondary ID
+    protected boolean isNetworkOverlappingRequestedPvlan(Integer existingPrimaryVlan, Integer existingSecondaryVlan, Integer requestedVlan) {
+        return requestedVlan.equals(existingPrimaryVlan) || requestedVlan.equals(existingSecondaryVlan);
+    }
+
     protected Network.PVlanType getNetworkPvlanType(long networkId, List<Integer> existingPvlan) {
         Network.PVlanType existingPvlanType = null;
         NetworkDetailVO pvlanTypeDetail = networkDetailsDao.findDetail(networkId, ApiConstants.ISOLATED_PVLAN_TYPE);
@@ -772,6 +777,38 @@ protected Network.PVlanType getNetworkPvlanType(long networkId, List<Integer> ex
         return existingPvlanType;
     }
 
+    @Override
+    public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri) {
+        final URI searchUri = BroadcastDomainType.fromString(broadcastUri);
+        if (!searchUri.getScheme().equalsIgnoreCase("vlan")) {
+            throw new CloudRuntimeException("VLAN requested but URI is not in the expected format: " + searchUri.toString());
+        }
+        final String searchRange = BroadcastDomainType.getValue(searchUri);
+        final List<Integer> searchVlans = UriUtils.expandVlanUri(searchRange);
+        final List<NetworkVO> overlappingNetworks = new ArrayList<>();
+
+        final SearchCriteria<NetworkVO> sc = PhysicalNetworkSearch.create();
+        sc.setParameters("physicalNetworkId", physicalNetworkId);
+
+        for (final NetworkVO network : listBy(sc)) {
+            if (network.getBroadcastUri() == null || !network.getBroadcastUri().getScheme().equalsIgnoreCase("pvlan")) {
+                continue;
+            }
+            // Ensure existing and proposed VLAN don't overlap
+            final String networkVlanRange = BroadcastDomainType.getValue(network.getBroadcastUri());
+            if (networkVlanRange == null || networkVlanRange.isEmpty()) {
+                continue;
+            }
+            List<Integer> existingPvlan = UriUtils.expandPvlanUri(networkVlanRange);
+            if (isNetworkOverlappingRequestedPvlan(existingPvlan.get(0), existingPvlan.get(1), searchVlans.get(0))) {
+                overlappingNetworks.add(network);
+                break;
+            }
+        }
+
+        return overlappingNetworks;
+    }
+
     @Override
     public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType) {
         final URI searchUri = BroadcastDomainType.fromString(broadcastUri);
diff --git a/server/src/test/java/com/cloud/network/dao/NetworkDaoTest.java b/server/src/test/java/com/cloud/network/dao/NetworkDaoTest.java
@@ -52,4 +52,14 @@ public void testNetworkOverlappingMultipleCommunityAllowed() {
         Assert.assertFalse(dao.isNetworkOverlappingRequestedPvlan(existingPrimaryVlan, existingSecondaryVlan, Network.PVlanType.Community,
                 existingPrimaryVlan, requestedVlan, Network.PVlanType.Community));
     }
+
+    public void testNetworkOverlappingVlanPvlanTrue() {
+        Assert.assertTrue(dao.isNetworkOverlappingRequestedPvlan(existingPrimaryVlan, existingSecondaryVlan, existingPrimaryVlan));
+        Assert.assertTrue(dao.isNetworkOverlappingRequestedPvlan(existingPrimaryVlan, existingSecondaryVlan, existingSecondaryVlan));
+    }
+
+    public void testNetworkOverlappingVlanPvlanFalse() {
+        Assert.assertFalse(dao.isNetworkOverlappingRequestedPvlan(existingPrimaryVlan, existingSecondaryVlan, requestedVlan));
+    }
+
 }
diff --git a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java
@@ -250,4 +250,9 @@ public List<NetworkVO> listByAccountIdNetworkName(final long accountId, final St
     public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType) {
         return null;
     }
+
+    @Override
+    public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri) {
+        return null;
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -126,4 +126,6 @@ public interface NetworkDao extends GenericDao<NetworkVO, Long>, StateDao<State,`
`126`	`126`	`List<NetworkVO> listByAccountIdNetworkName(long accountId, String name);`
`127`	`127`
`128`	`128`	`List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType);`
	`129`	`+`
	`130`	`+ List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri);`
`129`	`131`	`}`
Original file line number	Diff line number	Diff line change
`@@ -250,4 +250,9 @@ public List<NetworkVO> listByAccountIdNetworkName(final long accountId, final St`
`250`	`250`	`public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType) {`
`251`	`251`	`return null;`
`252`	`252`	`}`
	`253`	`+`
	`254`	`+ @Override`
	`255`	`+ public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri) {`
	`256`	`+ return null;`
	`257`	`+ }`
`253`	`258`	`}`