network: Offerings do not have to have Security Grouping enabled (#3112)

Offerings can co-exist where on does provide Security Grouping in the
network, but other guest Networks have no Security Grouping.

In V(X)LAN isolation environments the L2 separation is handled by V(X)LAN
and protection between Instances is handled by Security Grouping.

There are multiple scenarios possible where one network has Security Grouping
enabled because that is required in that network.

In the other network, but in the same zone it could be a choice to have
Security Grouping disabled and allow all traffic to flow.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

Author: wido

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

@@ -2246,9 +2246,6 @@ public Network createGuestNetwork(final long networkOfferingId, final String nam
                 if (_networkModel.areServicesSupportedByNetworkOffering(ntwkOff.getId(), Service.SourceNat)) {
                     throw new InvalidParameterValueException("Service SourceNat is not allowed in security group enabled zone");
                 }
-                if (!_networkModel.areServicesSupportedByNetworkOffering(ntwkOff.getId(), Service.SecurityGroup)) {
-                    throw new InvalidParameterValueException("network must have SecurityGroup provider in security group enabled zone");
-                }
             }
 
             //don't allow eip/elb networks in Advance zone

server/src/main/java/com/cloud/network/guru/DirectNetworkGuru.java

@@ -122,10 +122,11 @@ public boolean isMyTrafficType(TrafficType type) {
      * Return true if the physical network isolation method contains the expected isolation method for this guru
      */
     protected boolean isMyIsolationMethod(PhysicalNetwork physicalNetwork) {
-        for (IsolationMethod m : _isolationMethods) {
+        for (IsolationMethod m : this.getIsolationMethods()) {
             List<String> isolationMethods = physicalNetwork.getIsolationMethods();
             if (CollectionUtils.isNotEmpty(isolationMethods)) {
                 for (String method : isolationMethods) {
+                    s_logger.debug(method + ": " + m.toString());
                     if (method.equalsIgnoreCase(m.toString())) {
                         return true;
                     }
@@ -140,20 +141,11 @@ public TrafficType[] getSupportedTrafficType() {
         return TrafficTypes;
     }
 
-    /**
-     * True for Advanced zones, with VXLAN isolation method and Security Groups enabled
-     */
-    private boolean isMyIsolationMethodVxlanWithSecurityGroups(NetworkOffering offering, DataCenter dc, PhysicalNetwork physnet) {
-        return dc.getNetworkType().equals(NetworkType.Advanced) &&
-                _networkModel.areServicesSupportedByNetworkOffering(offering.getId(), Service.SecurityGroup) &&
-                physnet.getIsolationMethods().contains("VXLAN");
-    }
-
     protected boolean canHandle(NetworkOffering offering, DataCenter dc, PhysicalNetwork physnet) {
-        // this guru handles only Guest networks in Advance zone with source nat service disabled
-        boolean vxlanWithSecurityGroups = isMyIsolationMethodVxlanWithSecurityGroups(offering, dc, physnet);
-        if (dc.getNetworkType() == NetworkType.Advanced && isMyTrafficType(offering.getTrafficType()) &&
-                (isMyIsolationMethod(physnet) || vxlanWithSecurityGroups) && offering.getGuestType() == GuestType.Shared
+        if (dc.getNetworkType() == NetworkType.Advanced
+                && isMyTrafficType(offering.getTrafficType())
+                && isMyIsolationMethod(physnet)
+                && offering.getGuestType() == GuestType.Shared
                 && !_ntwkOfferingSrvcDao.isProviderForNetworkOffering(offering.getId(), Network.Provider.NuageVsp)
                 && !_ntwkOfferingSrvcDao.isProviderForNetworkOffering(offering.getId(), Network.Provider.NiciraNvp)) {
             return true;
@@ -169,6 +161,7 @@ public Network design(NetworkOffering offering, DeploymentPlan plan, Network use
         PhysicalNetworkVO physnet = _physicalNetworkDao.findById(plan.getPhysicalNetworkId());
 
         if (!canHandle(offering, dc, physnet)) {
+            s_logger.info("Refusing to design this network");
             return null;
         }
 
@@ -222,7 +215,11 @@ public Network design(NetworkOffering offering, DeploymentPlan plan, Network use
 
     protected DirectNetworkGuru() {
         super();
-        _isolationMethods = new IsolationMethod[] { new IsolationMethod("VLAN") };
+        _isolationMethods = new IsolationMethod[] { new IsolationMethod("VLAN"), new IsolationMethod("VXLAN") };
+    }
+
+    public IsolationMethod[] getIsolationMethods() {
+        return _isolationMethods;
     }
 
     @Override

server/src/test/java/com/cloud/network/guru/DirectNetworkGuruTest.java

@@ -16,38 +16,117 @@
 // under the License.
 package com.cloud.network.guru;
 
-import com.cloud.network.PhysicalNetwork;
+import com.cloud.dc.DataCenter.NetworkType;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.network.Network;
+import com.cloud.network.NetworkModel;
+import com.cloud.network.Networks.TrafficType;
+import com.cloud.network.Network.GuestType;
+import com.cloud.network.PhysicalNetwork.IsolationMethod;
+import com.cloud.network.dao.PhysicalNetworkDao;
+import com.cloud.network.dao.PhysicalNetworkVO;
+import com.cloud.offering.NetworkOffering;
+import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
+import com.cloud.user.Account;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
 import java.util.Arrays;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.when;
 
 public class DirectNetworkGuruTest {
 
-    DirectNetworkGuru guru = new DirectNetworkGuru();
+    protected DirectNetworkGuru guru = new DirectNetworkGuru();
 
     @Mock
-    PhysicalNetwork physicalNetwork;
+    PhysicalNetworkVO physicalNetwork;
+
+    @Mock
+    DataCenterVO dc;
+
+    @Mock
+    NetworkOffering offering;
+
+    @Mock
+    NetworkOfferingServiceMapDao ntwkOfferingSrvcDao;
+
+    @Mock
+    DataCenterDao dcDao;
+
+    @Mock
+    PhysicalNetworkDao physicalNetworkDao;
+
+    @Mock
+    Network network;
+
+    @Mock
+    NetworkModel networkModel;
+
+    @Mock
+    DeploymentPlan plan;
+
+    @Mock
+    Account owner;
 
     @Before
-    public void setup() throws Exception {
+    public void setup() {
         MockitoAnnotations.initMocks(this);
+        guru._ntwkOfferingSrvcDao = ntwkOfferingSrvcDao;
+        guru._dcDao = dcDao;
+        guru._physicalNetworkDao = physicalNetworkDao;
+        guru._networkModel = networkModel;
+
+        when(physicalNetwork.getId()).thenReturn(1l);
         when(physicalNetwork.getIsolationMethods()).thenReturn(Arrays.asList("VXLAN", "VLAN"));
+
+        when(dc.getNetworkType()).thenReturn(NetworkType.Advanced);
+        when(dc.getId()).thenReturn(1l);
+        when(offering.getGuestType()).thenReturn(GuestType.Shared);
+        when(offering.getTrafficType()).thenReturn(TrafficType.Guest);
+        when(offering.getId()).thenReturn(42l);
+        when(ntwkOfferingSrvcDao.isProviderForNetworkOffering(offering.getId(), Network.Provider.NuageVsp)).thenReturn(false);
+        when(ntwkOfferingSrvcDao.isProviderForNetworkOffering(offering.getId(), Network.Provider.NiciraNvp)).thenReturn(false);
     }
 
     @Test
-    public void testIsMyIsolationMethodUppercaseMethods() {
+    public void testIsMyIsolationMethod() {
         assertTrue(guru.isMyIsolationMethod(physicalNetwork));
     }
 
     @Test
-    public void testIsMyIsolationMethodLowercaseMethods() {
-        when(physicalNetwork.getIsolationMethods()).thenReturn(Arrays.asList("vxlan", "vlan"));
-        assertTrue(guru.isMyIsolationMethod(physicalNetwork));
+    public void testIsolationMethods() {
+        IsolationMethod[] expected = new IsolationMethod[] { new IsolationMethod("VLAN"), new IsolationMethod("VXLAN") };
+        assertEquals(expected, guru.getIsolationMethods());
+    }
+
+    @Test
+    public void testTrafficTypes() {
+        assertTrue(guru.isMyTrafficType(TrafficType.Guest));
+    }
+
+    @Test
+    public void testCanHandle() {
+        assertTrue(guru.canHandle(offering, dc, physicalNetwork));
+    }
+
+    @Test
+    public void testCanDesign() {
+        when(dcDao.findById(dc.getId())).thenReturn(dc);
+        when(plan.getDataCenterId()).thenReturn(1l);
+        when(plan.getPhysicalNetworkId()).thenReturn(1l);
+        when(physicalNetworkDao.findById(physicalNetwork.getId())).thenReturn(physicalNetwork);
+        when(offering.isRedundantRouter()).thenReturn(false);
+
+        when(networkModel.areServicesSupportedByNetworkOffering(offering.getId(), Network.Service.SecurityGroup)).thenReturn(true);
+
+        assertNotNull(guru.design(offering, plan, network, owner));
     }
 }