server: Broadcast URI not set to vxlan, but vlan (Fix #3040) (#4190)

This PR sets properly Broadcast URI to vxlan://vxlan_id when the physical network is of VXLAN.

Fixes: #3040

Author: GabrielBrascher

api/src/main/java/com/cloud/network/Networks.java

@@ -20,6 +20,7 @@
 import java.net.URISyntaxException;
 
 import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.commons.lang3.StringUtils;
 
 /**
  * Network includes all of the enums used within networking.
@@ -253,20 +254,42 @@ public static URI fromString(String candidate) {
                 Long.parseLong(candidate);
                 return Vlan.toUri(candidate);
             } catch (NumberFormatException nfe) {
-                if (com.cloud.dc.Vlan.UNTAGGED.equalsIgnoreCase(candidate)) {
-                    return Native.toUri(candidate);
-                }
-                try {
-                    URI uri = new URI(candidate);
-                    BroadcastDomainType tiep = getSchemeValue(uri);
-                    if (tiep.scheme != null && tiep.scheme.equals(uri.getScheme())) {
-                        return uri;
-                    } else {
-                        throw new CloudRuntimeException("string '" + candidate + "' has an unknown BroadcastDomainType.");
-                    }
-                } catch (URISyntaxException e) {
-                    throw new CloudRuntimeException("string is not a broadcast URI: " + candidate);
+                return getVlanUriWhenNumberFormatException(candidate);
+            }
+        }
+
+        /**
+         * This method is called in case of NumberFormatException is thrown when parsing the String into long
+         */
+        private static URI getVlanUriWhenNumberFormatException(String candidate) {
+            if(StringUtils.isBlank(candidate)) {
+                throw new CloudRuntimeException("Expected VLAN or VXLAN but got a null isolation method");
+            }
+            if (com.cloud.dc.Vlan.UNTAGGED.equalsIgnoreCase(candidate)) {
+                return Native.toUri(candidate);
+            }
+            try {
+                URI uri = new URI(candidate);
+                BroadcastDomainType tiep = getSchemeValue(uri);
+                if (tiep.scheme != null && tiep.scheme.equals(uri.getScheme())) {
+                    return uri;
+                } else {
+                    throw new CloudRuntimeException("string '" + candidate + "' has an unknown BroadcastDomainType.");
                 }
+            } catch (URISyntaxException e) {
+                throw new CloudRuntimeException("string is not a broadcast URI: " + candidate);
+            }
+        }
+
+        /**
+         * Encodes a string into a BroadcastUri, according to the given BroadcastDomainType
+         */
+        public static URI encodeStringIntoBroadcastUri(String candidate, BroadcastDomainType isolationMethod) {
+            try{
+                Long.parseLong(candidate);
+                return isolationMethod.toUri(candidate);
+            } catch (NumberFormatException nfe) {
+                return getVlanUriWhenNumberFormatException(candidate);
             }
         }
     };

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -2279,7 +2279,7 @@ public Network createGuestNetwork(final long networkOfferingId, final String nam
         }
 
         if (vlanSpecified) {
-            URI uri = BroadcastDomainType.fromString(vlanId);
+            URI uri = encodeVlanIdIntoBroadcastUri(vlanId, pNtwk);
             //don't allow to specify vlan tag used by physical network for dynamic vlan allocation
             if (!(bypassVlanOverlapCheck && ntwkOff.getGuestType() == GuestType.Shared) && _dcDao.findVnet(zoneId, pNtwk.getId(), BroadcastDomainType.getValue(uri)).size() > 0) {
                 throw new InvalidParameterValueException("The VLAN tag " + vlanId + " is already being used for dynamic vlan allocation for the guest network in zone "
@@ -2424,7 +2424,7 @@ public Network doInTransaction(final TransactionStatus status) {
                             //Logical router's UUID provided as VLAN_ID
                             userNetwork.setVlanIdAsUUID(vlanIdFinal); //Set transient field
                         } else {
-                            uri = BroadcastDomainType.fromString(vlanIdFinal);
+                            uri = encodeVlanIdIntoBroadcastUri(vlanIdFinal, pNtwk);
                         }
                         userNetwork.setBroadcastUri(uri);
                         if (!vlanIdFinal.equalsIgnoreCase(Vlan.UNTAGGED)) {
@@ -2475,6 +2475,25 @@ public Network doInTransaction(final TransactionStatus status) {
         return network;
     }
 
+    /**
+     * Encodes VLAN/VXLAN ID into a Broadcast URI according to the isolation method from the Physical Network.
+     * @return Broadcast URI, e.g. 'vlan://vlan_ID' or 'vxlan://vlxan_ID'
+     */
+    protected URI encodeVlanIdIntoBroadcastUri(String vlanId, PhysicalNetwork pNtwk) {
+        if (pNtwk == null) {
+            throw new InvalidParameterValueException(String.format("Failed to encode VLAN/VXLAN %s into a Broadcast URI. Physical Network cannot be null.", vlanId));
+        }
+
+        if(StringUtils.isNotBlank(pNtwk.getIsolationMethods().get(0))) {
+            String isolationMethod = pNtwk.getIsolationMethods().get(0).toLowerCase();
+            String vxlan = BroadcastDomainType.Vxlan.toString().toLowerCase();
+            if(isolationMethod.equals(vxlan)) {
+                return BroadcastDomainType.encodeStringIntoBroadcastUri(vlanId, BroadcastDomainType.Vxlan);
+            }
+        }
+        return BroadcastDomainType.fromString(vlanId);
+    }
+
   /**
    * Checks bypass VLAN id/range overlap check during network creation for guest networks
    * @param bypassVlanOverlapCheck bypass VLAN id/range overlap check

engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java

@@ -22,12 +22,15 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import com.cloud.network.dao.PhysicalNetworkVO;
+import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.log4j.Logger;
 import org.junit.Assert;
 import org.junit.Before;
@@ -463,4 +466,75 @@ public void validateLockedRequestedIpTestFreeAndNotNullIp() {
         testOrchastrator.validateLockedRequestedIp(ipVoSpy, lockedIp);
     }
 
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestVxlan() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("123", "VXLAN", "vxlan", "vxlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestVlan() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("123", "VLAN", "vlan", "vlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestEmpty() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("123", "", "vlan", "vlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestNull() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("123", null, "vlan", "vlan://123");
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void encodeVlanIdIntoBroadcastUriTestEmptyVlanId() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("", "vxlan", "vlan", "vlan://123");
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void encodeVlanIdIntoBroadcastUriTestNullVlanId() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest(null, "vlan", "vlan", "vlan://123");
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void encodeVlanIdIntoBroadcastUriTestBlankVlanId() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest(" ", "vlan", "vlan", "vlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestNullVlanIdWithSchema() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("vlan://123", "vlan", "vlan", "vlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestNullVlanIdWithSchemaIsolationVxlan() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("vlan://123", "vxlan", "vlan", "vlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestNullVxlanIdWithSchema() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("vxlan://123", "vxlan", "vxlan", "vxlan://123");
+    }
+
+    @Test
+    public void encodeVlanIdIntoBroadcastUriTestNullVxlanIdWithSchemaIsolationVlan() {
+        encodeVlanIdIntoBroadcastUriPrepareAndTest("vxlan://123", "vlan", "vxlan", "vxlan://123");
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void encodeVlanIdIntoBroadcastUriTestNullNetwork() {
+        URI resultUri = testOrchastrator.encodeVlanIdIntoBroadcastUri("vxlan://123", null);
+    }
+
+    private void encodeVlanIdIntoBroadcastUriPrepareAndTest(String vlanId, String isolationMethod, String expectedIsolation, String expectedUri) {
+        PhysicalNetworkVO physicalNetwork = new PhysicalNetworkVO();
+        List<String> isolationMethods = new ArrayList<>();
+        isolationMethods.add(isolationMethod);
+        physicalNetwork.setIsolationMethods(isolationMethods);
+
+        URI resultUri = testOrchastrator.encodeVlanIdIntoBroadcastUri(vlanId, physicalNetwork);
+
+        Assert.assertEquals(expectedIsolation, resultUri.getScheme());
+        Assert.assertEquals(expectedUri, resultUri.toString());
+    }
 }