Shared Network Firewall (Security groups) in Advanced zone without security groups (#9415)

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

@@ -356,4 +356,8 @@ public interface NetworkModel {
 
     void verifyIp6DnsPair(final String ip6Dns1, final String ip6Dns2);
 
+    boolean isSecurityGroupSupportedForZone(Long zoneId);
+
+    boolean checkSecurityGroupSupportForNetwork(DataCenter zone, List<Long> networkIds,
+                                                List<Long> securityGroupsIds);
 }

api/src/main/java/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java

@@ -79,7 +79,7 @@ public class AddIpToVmNicCmd extends BaseAsyncCreateCmd {
     private boolean isZoneSGEnabled() {
         Network ntwk = _entityMgr.findById(Network.class, getNetworkId());
         DataCenter dc = _entityMgr.findById(DataCenter.class, ntwk.getDataCenterId());
-        return dc.isSecurityGroupEnabled();
+        return dc.isSecurityGroupEnabled() || _ntwkModel.isSecurityGroupSupportedForZone(dc.getId());
     }
 
     @Override

api/src/main/java/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java

@@ -127,7 +127,7 @@ public class RemoveIpFromVmNicCmd extends BaseAsyncCmd {
     private boolean isZoneSGEnabled() {
         Network ntwk = _entityMgr.findById(Network.class, getNetworkId());
         DataCenter dc = _entityMgr.findById(DataCenter.class, ntwk.getDataCenterId());
-        return dc.isSecurityGroupEnabled();
+        return dc.isSecurityGroupEnabled() || _ntwkModel.isSecurityGroupSupportedForZone(dc.getId());
     }
 
     @Override

api/src/main/java/org/apache/cloudstack/api/response/ZoneResponse.java

@@ -312,10 +312,6 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso
         return networkType;
     }
 
-    public boolean isSecurityGroupsEnabled() {
-        return securityGroupsEnabled;
-    }
-
     public String getAllocationState() {
         return allocationState;
     }
@@ -332,10 +328,6 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso
         return capacities;
     }
 
-    public boolean isLocalStorageEnabled() {
-        return localStorageEnabled;
-    }
-
     public Set<ResourceTagResponse> getTags() {
         return tags;
     }
@@ -344,6 +336,14 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso
         return resourceDetails;
     }
 
+    public boolean isSecurityGroupsEnabled() {
+        return securityGroupsEnabled;
+    }
+
+    public boolean isLocalStorageEnabled() {
+        return localStorageEnabled;
+    }
+
     public Boolean getAllowUserSpecifyVRMtu() {
         return allowUserSpecifyVRMtu;
     }
@@ -356,6 +356,10 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso
         return routerPublicInterfaceMaxMtu;
     }
 
+    public boolean isNsxEnabled() {
+        return nsxEnabled;
+    }
+
     @Override
     public void setResourceIconResponse(ResourceIconResponse resourceIconResponse) {
         this.resourceIconResponse = resourceIconResponse;

plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java

@@ -346,7 +346,7 @@ public class XcpServerDiscoverer extends DiscovererBase implements Discoverer, L
                 }
 
                 DataCenterVO zone = _dcDao.findById(dcId);
-                boolean securityGroupEnabled = zone.isSecurityGroupEnabled();
+                boolean securityGroupEnabled = zone.isSecurityGroupEnabled() || _networkMgr.isSecurityGroupSupportedForZone(zone.getId());
                 params.put("securitygroupenabled", Boolean.toString(securityGroupEnabled));
 
                 params.put("router.aggregation.command.each.timeout", _configDao.getValue(Config.RouterAggregationCommandEachTimeout.toString()));
@@ -695,7 +695,7 @@ public class XcpServerDiscoverer extends DiscovererBase implements Discoverer, L
         HashMap<String, Object> params = super.buildConfigParams(host);
         DataCenterVO zone = _dcDao.findById(host.getDataCenterId());
         if (zone != null) {
-            boolean securityGroupEnabled = zone.isSecurityGroupEnabled();
+            boolean securityGroupEnabled = zone.isSecurityGroupEnabled() || _networkMgr.isSecurityGroupSupportedForZone(zone.getId());
             params.put("securitygroupenabled", Boolean.toString(securityGroupEnabled));
         }
         return params;

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java

@@ -401,7 +401,7 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
         if (StringUtils.isNotBlank(kubernetesCluster.getKeyPair())) {
             keypairs.add(kubernetesCluster.getKeyPair());
         }
-        if (zone.isSecurityGroupEnabled()) {
+        if (kubernetesCluster.getSecurityGroupId() != null && networkModel.checkSecurityGroupSupportForNetwork(zone, networkIds, List.of(kubernetesCluster.getSecurityGroupId()))) {
             List<Long> securityGroupIds = new ArrayList<>();
             securityGroupIds.add(kubernetesCluster.getSecurityGroupId());
             nodeVm = userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, clusterTemplate, networkIds, securityGroupIds, owner,

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java

@@ -215,7 +215,9 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
         if (StringUtils.isNotBlank(kubernetesCluster.getKeyPair())) {
             keypairs.add(kubernetesCluster.getKeyPair());
         }
-        if (zone.isSecurityGroupEnabled()) {
+        if (kubernetesCluster.getSecurityGroupId() != null &&
+                networkModel.checkSecurityGroupSupportForNetwork(zone, networkIds,
+                        List.of(kubernetesCluster.getSecurityGroupId()))) {
             List<Long> securityGroupIds = new ArrayList<>();
             securityGroupIds.add(kubernetesCluster.getSecurityGroupId());
             controlVm = userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, clusterTemplate, networkIds, securityGroupIds, owner,
@@ -289,7 +291,8 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif
         if (StringUtils.isNotBlank(kubernetesCluster.getKeyPair())) {
             keypairs.add(kubernetesCluster.getKeyPair());
         }
-        if (zone.isSecurityGroupEnabled()) {
+        if (kubernetesCluster.getSecurityGroupId() != null &&
+                networkModel.checkSecurityGroupSupportForNetwork(zone, networkIds, List.of(kubernetesCluster.getSecurityGroupId()))) {
             List<Long> securityGroupIds = new ArrayList<>();
             securityGroupIds.add(kubernetesCluster.getSecurityGroupId());
             additionalControlVm = userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, clusterTemplate, networkIds, securityGroupIds, owner,

server/src/main/java/com/cloud/network/NetworkModelImpl.java

@@ -145,6 +145,8 @@ import com.cloud.vm.dao.NicDao;
 import com.cloud.vm.dao.NicSecondaryIpDao;
 import com.cloud.vm.dao.VMInstanceDao;
 
+import static com.cloud.network.Network.Service.SecurityGroup;
+
 public class NetworkModelImpl extends ManagerBase implements NetworkModel, Configurable {
     public static final String UNABLE_TO_USE_NETWORK = "Unable to use network with id= %s, permission denied";
     @Inject
@@ -1262,7 +1264,7 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
             physicalNetworkId = findPhysicalNetworkId(network.getDataCenterId(), null, null);
         }
 
-        return isServiceEnabledInNetwork(physicalNetworkId, network.getId(), Service.SecurityGroup);
+        return isServiceEnabledInNetwork(physicalNetworkId, network.getId(), SecurityGroup);
     }
 
     @Override
@@ -2755,4 +2757,38 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
             throw new InvalidParameterValueException("Invalid IPv6 for IPv6 DNS2");
         }
     }
+
+    @Override
+    public boolean isSecurityGroupSupportedForZone(Long zoneId) {
+        List<? extends PhysicalNetwork> networks = getPhysicalNtwksSupportingTrafficType(zoneId, TrafficType.Guest);
+        for (PhysicalNetwork network : networks ) {
+            if (_pNSPDao.isServiceProviderEnabled(network.getId(), Provider.SecurityGroupProvider.getName(), Service.SecurityGroup.getName())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public boolean checkSecurityGroupSupportForNetwork(DataCenter zone, List<Long> networkIds,
+                                                       List<Long> securityGroupsIds) {
+        if (zone.isSecurityGroupEnabled()) {
+            return true;
+        }
+        if (CollectionUtils.isNotEmpty(networkIds)) {
+            for (Long networkId : networkIds) {
+                Network network = _networksDao.findById(networkId);
+                if (network == null) {
+                    throw new InvalidParameterValueException("Unable to find network by id " + networkId);
+                }
+                if (network.getGuestType() == Network.GuestType.Shared && isSecurityGroupSupportedInNetwork(network)) {
+                    return true;
+                }
+            }
+        } else if (CollectionUtils.isNotEmpty(securityGroupsIds)) {
+            Network networkWithSecurityGroup = getNetworkWithSGWithFreeIPs(zone.getId());
+            return networkWithSecurityGroup != null;
+        }
+        return false;
+    }
 }

server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java

@@ -19,6 +19,7 @@ package com.cloud.network.as;
 import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -37,6 +38,7 @@ import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
 
+import com.cloud.network.NetworkModel;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.affinity.AffinityGroupVO;
 import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
@@ -251,6 +253,8 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage
     @Inject
     NetworkOrchestrationService networkMgr;
     @Inject
+    NetworkModel networkModel;
+    @Inject
     private UserVmManager userVmMgr;
     @Inject
     private UserDataManager userDataMgr;
@@ -1808,7 +1812,8 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage
                         null, null, true, null, affinityGroupIdList, customParameters, null, null, null,
                         null, true, overrideDiskOfferingId);
             } else {
-                if (zone.isSecurityGroupEnabled()) {
+                if (networkModel.checkSecurityGroupSupportForNetwork(zone, networkIds,
+                        Collections.emptyList())) {
                     vm = userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, template, networkIds, null,
                             owner, vmHostName,vmHostName, diskOfferingId, dataDiskSize, null,
                             hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs,

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -27,6 +27,7 @@ import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -3094,7 +3095,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                 if (zone.getNetworkType() == NetworkType.Basic) {
                     // Get default guest network in Basic zone
                     defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId());
-                } else if (zone.isSecurityGroupEnabled()) {
+                } else if (_networkModel.checkSecurityGroupSupportForNetwork(zone, Collections.emptyList(), securityGroupIdList)) {
                     NicVO defaultNic = _nicDao.findDefaultNicForVM(vm.getId());
                     if (defaultNic != null) {
                         defaultNetwork = _networkDao.findById(defaultNic.getNetworkId());
@@ -6153,7 +6154,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                         dataDiskTemplateToDiskOfferingMap, userVmOVFProperties, dynamicScalingEnabled, overrideDiskOfferingId);
             }
         } else {
-            if (zone.isSecurityGroupEnabled())  {
+            if (_networkModel.checkSecurityGroupSupportForNetwork(zone, networkIds,
+                    cmd.getSecurityGroupIdList()))  {
                 vm = createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, template, networkIds, getSecurityGroupIdList(cmd, zone, template, owner), owner, name,
                         displayName, diskOfferingId, size, group, cmd.getHypervisor(), cmd.getHttpMethod(), userData, userDataId, userDataDetails, sshKeyPairNames, cmd.getIpToNetworkMap(), addrs, displayVm, keyboard,
                         cmd.getAffinityGroupIdList(), cmd.getDetails(), cmd.getCustomId(), cmd.getDhcpOptionsMap(),
@@ -7573,7 +7575,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             Set<NetworkVO> applicableNetworks = new LinkedHashSet<>();
             Map<Long, String> requestedIPv4ForNics = new HashMap<>();
             Map<Long, String> requestedIPv6ForNics = new HashMap<>();
-            if (zone.isSecurityGroupEnabled())  { // advanced zone with security groups
+            if (_networkModel.checkSecurityGroupSupportForNetwork(zone, networkIdList, securityGroupIdList))  { // advanced zone with security groups
                 // cleanup the old security groups
                 _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId());
                 // if networkIdList is null and the first network of vm is shared network, then keep it if possible
@@ -8794,7 +8796,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
     private Network getNetworkForOvfNetworkMapping(DataCenter zone, Account owner) throws InsufficientCapacityException, ResourceAllocationException {
         Network network = null;
-        if (zone.isSecurityGroupEnabled()) {
+        if (zone.isSecurityGroupEnabled() || _networkModel.isSecurityGroupSupportedForZone(zone.getId())) {
             network = _networkModel.getNetworkWithSGWithFreeIPs(zone.getId());
             if (network == null) {
                 throw new InvalidParameterValueException("No network with security enabled is found in zone ID: " + zone.getUuid());

server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java

@@ -291,7 +291,7 @@ public class VnfTemplateManagerImpl extends ManagerBase implements VnfTemplateMa
     @Override
     public SecurityGroup createSecurityGroupForVnfAppliance(DataCenter zone, VirtualMachineTemplate template, Account owner,
                                                             DeployVnfApplianceCmd cmd) {
-        if (zone == null || !zone.isSecurityGroupEnabled()) {
+        if (zone == null || !(zone.isSecurityGroupEnabled() || networkModel.isSecurityGroupSupportedForZone(zone.getId()))) {
             return null;
         }
         if (!cmd.getVnfConfigureManagement()) {

server/src/test/java/com/cloud/network/MockNetworkModelImpl.java

@@ -938,4 +938,14 @@ public class MockNetworkModelImpl extends ManagerBase implements NetworkModel {
 
     @Override
     public void verifyIp6DnsPair(String ip4Dns1, String ip4Dns2) {}
+
+    @Override
+    public boolean isSecurityGroupSupportedForZone(Long zoneId) {
+        return false;
+    }
+
+    @Override
+    public boolean checkSecurityGroupSupportForNetwork(DataCenter zone, List<Long> networkIds, List<Long> securityGroupsIds) {
+        return false;
+    }
 }

server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java

@@ -42,6 +42,7 @@ import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.host.HostVO;
 import com.cloud.host.dao.HostDao;
 import com.cloud.network.Network;
+import com.cloud.network.NetworkModel;
 import com.cloud.network.as.dao.AutoScalePolicyConditionMapDao;
 import com.cloud.network.as.dao.AutoScalePolicyDao;
 import com.cloud.network.as.dao.AutoScaleVmGroupDao;
@@ -139,6 +140,7 @@ import org.springframework.test.util.ReflectionTestUtils;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -355,6 +357,8 @@ public class AutoScaleManagerImplTest {
     @Mock
     NetworkVO networkMock;
     @Mock
+    NetworkModel networkModel;
+    @Mock
     NetworkOfferingVO networkOfferingMock;
     @Mock
     CounterVO counterMock;
@@ -1311,10 +1315,10 @@ public class AutoScaleManagerImplTest {
 
         when(userVmMock.getId()).thenReturn(virtualMachineId);
         when(zoneMock.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced);
-        when(zoneMock.isSecurityGroupEnabled()).thenReturn(true);
         when(userVmService.createAdvancedSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(),
                 any(), any(), any(), any(), any(), any(), eq(userData), eq(userDataId), eq(userDataDetails.toString()), any(), any(), any(), any(), any(), any(),
                 any(), any(), any(), any(), any(), eq(true), any(), any())).thenReturn(userVmMock);
+        when(networkModel.checkSecurityGroupSupportForNetwork(zoneMock, List.of(networkId), Collections.emptyList())).thenReturn(true);
 
         long result = autoScaleManagerImplSpy.createNewVM(asVmGroupMock);
 
@@ -1360,10 +1364,10 @@ public class AutoScaleManagerImplTest {
 
         when(userVmMock.getId()).thenReturn(virtualMachineId);
         when(zoneMock.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced);
-        when(zoneMock.isSecurityGroupEnabled()).thenReturn(false);
         when(userVmService.createAdvancedVirtualMachine(any(), any(), any(), any(), any(), any(), any(),
                 any(), any(), any(), any(), any(), eq(userData), eq(userDataId), eq(userDataDetails.toString()), any(), any(), any(), eq(true), any(), any(), any(),
                 any(), any(), any(), any(), eq(true), any(), any())).thenReturn(userVmMock);
+        when(networkModel.checkSecurityGroupSupportForNetwork(zoneMock, List.of(networkId), Collections.emptyList())).thenReturn(false);
 
         long result = autoScaleManagerImplSpy.createNewVM(asVmGroupMock);
 

server/src/test/java/com/cloud/vpc/MockNetworkModelImpl.java

@@ -954,4 +954,14 @@ public class MockNetworkModelImpl extends ManagerBase implements NetworkModel {
 
     @Override
     public void verifyIp6DnsPair(String ip4Dns1, String ip4Dns2) {}
+
+    @Override
+    public boolean isSecurityGroupSupportedForZone(Long zoneId) {
+        return false;
+    }
+
+    @Override
+    public boolean checkSecurityGroupSupportForNetwork(DataCenter zone, List<Long> networkIds, List<Long> securityGroupsIds) {
+        return false;
+    }
 }

services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerTest.java

@@ -29,6 +29,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import com.cloud.network.NetworkModel;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -56,6 +57,9 @@ public class SecondaryStorageManagerTest {
     @Mock
     NetworkDao _networkDao;
 
+    @Mock
+    NetworkModel _networkModel;
+
     @InjectMocks
     SecondaryStorageManagerImpl _ssMgr = new SecondaryStorageManagerImpl();
 
@@ -76,6 +80,7 @@ public class SecondaryStorageManagerTest {
         DataCenterVO dc = mock(DataCenterVO.class);
         when(dc.getNetworkType()).thenReturn(NetworkType.Advanced);
         when(dc.isSecurityGroupEnabled()).thenReturn(false);
+        when(_networkModel.isSecurityGroupSupportedForZone(anyLong())).thenReturn(false);
 
         when(_dcDao.findById(Mockito.anyLong())).thenReturn(dc);
 
@@ -102,6 +107,7 @@ public class SecondaryStorageManagerTest {
         DataCenterVO dc = Mockito.mock(DataCenterVO.class);
         when(dc.getNetworkType()).thenReturn(NetworkType.Advanced);
         when(dc.isSecurityGroupEnabled()).thenReturn(true);
+        when(_networkModel.isSecurityGroupSupportedForZone(anyLong())).thenReturn(true);
 
         when(_dcDao.findById(Mockito.anyLong())).thenReturn(dc);
 

test/integration/component/test_advancedsg_networks.py

@@ -31,7 +31,9 @@ from marvin.lib.base import (Zone,
                              VPC,
                              SecurityGroup,
                              Host,
-                                         )
+                             NetworkServiceProvider,
+                             PhysicalNetwork,
+                             TrafficType)
 
 from marvin.lib.common import (get_domain,
                                            get_zone,
@@ -158,7 +160,9 @@ class TestCreateZoneSG(cloudstackTestCase):
         return
 
 class TestNetworksInAdvancedSG(cloudstackTestCase):
-    """Test Creation of different types of networks in SG enabled advanced zone"""
+    """Test Creation of different types of networks in
+        - SG enabled advanced zone, or
+        - advanced zone without SG but with SG provider enabled"""
 
     @classmethod
     def setUpClass(cls):
@@ -169,6 +173,26 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
         # Get Zone, Domain and templates
         cls.domain = get_domain(cls.api_client)
         cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+        if cls.zone.securitygroupsenabled is False:
+            physical_networks = PhysicalNetwork.list(cls.api_client, zoneid=cls.zone.id)
+            selected_physical_network = None
+            for net in physical_networks:
+                traffic_types = TrafficType.list(cls.api_client, physicalnetworkid=net.id)
+                for traffic_type in traffic_types:
+                    if traffic_type.traffictype == 'Guest':
+                        selected_physical_network = net
+                        break
+                if selected_physical_network is not None:
+                    break
+            if selected_physical_network is None:
+                raise Exception("No physical network found with guest traffic type")
+
+            # Enable security group provider for physical network
+            nsps = NetworkServiceProvider.list(cls.api_client, physicalnetworkid=selected_physical_network.id, name='SecurityGroupProvider')
+            if len(nsps) == 0:
+                raise Exception("No security group provider found for physical network")
+            NetworkServiceProvider.update(cls.api_client, nsps[0].id, state='Enabled')
+
         cls.template = get_template(cls.api_client, cls.zone.id,
                                     cls.services["ostype"])
 
@@ -294,7 +318,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_03_createIsolatedNetwork(self):
         """ Test Isolated Network """
 
@@ -311,7 +335,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
         self.isolated_network_offering = NetworkOffering.create(self.api_client, self.services["isolated_network_offering"],
                                                                 conservemode=False)
 
-        self.cleanup.append(self.isolated_network_offering)
+        self.cleanup_nwOfferings.append(self.isolated_network_offering)
 
         self.debug("Isolated Network offering created: %s" % self.isolated_network_offering.id)
 
@@ -374,7 +398,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
                         Exception: %s" % e)
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_05_deployVM_SharedwithSG(self):
         """ Test VM deployment in shared networks with SecurityGroup Provider """
 
@@ -453,7 +477,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
         self.debug("Virtual Machine created: %s" % virtual_machine.id)
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_06_SharedNwSGAccountSpecific(self):
         """ Test Account specific shared network creation with SG"""
 
@@ -535,7 +559,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_07_SharedNwSG_DomainWide_SubdomainAcccess(self):
         """ Test Domain wide shared network with SG, with subdomain access set True"""
 
@@ -556,7 +580,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
         self.debug("Created domain %s" % self.parent_domain.id)
         self.debug("Creating child domain under this domain")
         self.child_domain = Domain.create(self.api_client,services=self.services["domain"],
-                                          parentdomainid=self.parent_domain)
+                                          parentdomainid=self.parent_domain.id)
 
         self.debug("Created child domain: %s" % self.child_domain.id)
 
@@ -753,7 +777,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_10_deleteSharedNwSGAccountSpecific_InUse(self):
         """ Test delete Account specific shared network creation with SG which is in use"""
 
@@ -806,7 +830,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_11_deleteSharedNwSG_DomainWide_InUse(self):
         """ Test delete Domain wide shared network with SG which is in use"""
 
@@ -861,7 +885,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_29_deleteSharedNwSG_ZoneWide_InUse(self):
         """ Test delete Zone wide shared network with SG which is in use"""
 
@@ -908,7 +932,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
         self.cleanup_vms.append(vm)
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_12_deleteSharedNwSGAccountSpecific_NotInUse(self):
         """ Test delete Account specific shared network creation with SG which is not in use"""
 
@@ -950,7 +974,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_13_deleteSharedNwSG_DomainWide_NotInUse(self):
         """ Test delete Domain wide shared network with SG which is not in use"""
 
@@ -994,7 +1018,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_30_deleteSharedNwSG_ZoneWide_NotInUse(self):
         """ Test delete zone wide shared network with SG which is not in use"""
 
@@ -1031,7 +1055,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test__14_createSharedNwWithSG_withoutParams(self):
         """ Test create shared network with SG without specifying necessary parameters"""
 
@@ -1076,7 +1100,7 @@ class TestNetworksInAdvancedSG(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test__15_createVPC(self):
         """ Test create VPC in advanced SG enabled zone"""
 
@@ -1116,6 +1140,26 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
         # Get Zone, Domain and templates
         cls.domain = get_domain(cls.api_client)
         cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+        if cls.zone.securitygroupsenabled is False:
+            physical_networks = PhysicalNetwork.list(cls.api_client, zoneid=cls.zone.id)
+            selected_physical_network = None
+            for net in physical_networks:
+                traffic_types = TrafficType.list(cls.api_client, physicalnetworkid=net.id)
+                for traffic_type in traffic_types:
+                    if traffic_type.traffictype == 'Guest':
+                        selected_physical_network = net
+                        break
+                if selected_physical_network is not None:
+                    break
+            if selected_physical_network is None:
+                raise Exception("No physical network found with guest traffic type")
+
+            # Enable security group provider for physical network
+            nsps = NetworkServiceProvider.list(cls.api_client, physicalnetworkid=selected_physical_network.id, name='SecurityGroupProvider')
+            if len(nsps) == 0:
+                raise Exception("No security group provider found for physical network")
+            NetworkServiceProvider.update(cls.api_client, nsps[0].id, state='Enabled')
+
         cls.template = get_template(cls.api_client,cls.zone.id,cls.services["ostype"])
 
         cls.services["virtual_machine"]["zoneid"] = cls.zone.id
@@ -1314,7 +1358,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
         status = os.system("%s -i %s" %(file2, config_file))
         return status
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test__16_AccountSpecificNwAccess(self):
         """ Test account specific network access of users"""
 
@@ -1409,7 +1453,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test__17_DomainSpecificNwAccess(self):
         """ Test domain specific network access of users"""
 
@@ -1520,7 +1564,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
         return
 
     @data("account", "domain")
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_18_DeployVM_NoFreeIPs(self, value):
         """ Test deploy VM in account/domain specific SG enabled shared network when no free IPs are available"""
 
@@ -1624,7 +1668,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
         return
 
     @data("default", "custom")
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_20_DeployVM_SecGrp_sharedNetwork(self, value):
         """ Test deploy VM in default/custom security group and shared network"""
 
@@ -1680,7 +1724,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_24_DeployVM_Multiple_Shared_Networks(self):
         """ Test deploy VM in multiple zone wide shared networks"""
 
@@ -1728,7 +1772,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_25_Deploy_Multiple_VM_Different_Shared_Networks_Same_SG(self):
         """ Test deploy Multiple VMs in different shared networks but same security group"""
 
@@ -1796,7 +1840,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_26_Destroy_Deploy_VM_NoFreeIPs(self):
         """ Test destroy VM in zone wide shared nw when IPs are full and then try to deploy vm"""
 
@@ -1903,7 +1947,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % vm.nic[0].ipaddress)
-            vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+            vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
 
             self.debug("SSH to VM successful, proceeding for %s operation" % value)
 
@@ -1914,7 +1958,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
                 vm.reboot(self.api_client)
 
             self.debug("SSH into VM: %s" % vm.nic[0].ipaddress)
-            vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+            vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
 
         except Exception as e:
             self.fail("SSH Access failed for %s: %s, failed at line %s" % \
@@ -1972,7 +2016,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % vm.nic[0].ipaddress)
-            vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+            vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
             self.debug("SSH to VM successful, proceeding for %s operation" % value)
             vm.delete(self.api_client, expunge=False)
             if value == "recover":
@@ -1989,7 +2033,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
                     retriesCount -= 1
                     time.sleep(10)
                 self.debug("SSH into VM: %s" % vm.nic[0].ipaddress)
-                vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+                vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
                 self.debug("SSH successful")
                 self.cleanup_vms.append(vm)
             elif value == "expunge":
@@ -2003,7 +2047,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
                 try:
                     self.debug("SSH into VM: %s, this should fail" % vm.nic[0].ipaddress)
-                    vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+                    vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
                     self.fail("SSH should have failed, instead it succeeded")
                 except Exception as e:
                     self.debug("SSH failed as expected with exception: %s" % e)
@@ -2019,7 +2063,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_31_Deploy_VM_multiple_shared_networks_sg(self):
         """ Test deploy VM in multiple SG enabled shared networks"""
 
@@ -2069,7 +2113,7 @@ class TestNetworksInAdvancedSG_VmOperations(cloudstackTestCase):
 
     @unittest.skip("Testing pending on multihost setup")
     @data("account","domain","zone")
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_33_VM_Migrate_SharedNwSG(self, value):
         """ Test migration of VM deployed in Account specific shared network"""
 
@@ -2280,6 +2324,26 @@ class TestSecurityGroups_BasicSanity(cloudstackTestCase):
         # Get Zone, Domain and templates
         cls.domain = get_domain(cls.api_client)
         cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+        if cls.zone.securitygroupsenabled is False:
+            physical_networks = PhysicalNetwork.list(cls.api_client, zoneid=cls.zone.id)
+            selected_physical_network = None
+            for net in physical_networks:
+                traffic_types = TrafficType.list(cls.api_client, physicalnetworkid=net.id)
+                for traffic_type in traffic_types:
+                    if traffic_type.traffictype == 'Guest':
+                        selected_physical_network = net
+                        break
+                if selected_physical_network is not None:
+                    break
+            if selected_physical_network is None:
+                raise Exception("No physical network found with guest traffic type")
+
+            # Enable security group provider for physical network
+            nsps = NetworkServiceProvider.list(cls.api_client, physicalnetworkid=selected_physical_network.id, name='SecurityGroupProvider')
+            if len(nsps) == 0:
+                raise Exception("No security group provider found for physical network")
+            NetworkServiceProvider.update(cls.api_client, nsps[0].id, state='Enabled')
+
         cls.template = get_template(cls.api_client,cls.zone.id,cls.services["ostype"])
 
         cls.services["virtual_machine"]["zoneid"] = cls.zone.id
@@ -2488,7 +2552,7 @@ class TestSecurityGroups_BasicSanity(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % vm.nic[0].ipaddress)
-            ssh = vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+            ssh = vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
 
             # Ping to outsite world
             res = ssh.execute("ping -c 1 www.google.com")
@@ -2580,7 +2644,7 @@ class TestSecurityGroups_BasicSanity(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % vm.nic[0].ipaddress)
-            ssh = vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+            ssh = vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
 
             # Ping to outsite world
             res = ssh.execute("ping -c 1 www.google.com")
@@ -2673,7 +2737,7 @@ class TestSecurityGroups_BasicSanity(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % vm.ssh_ip)
-            ssh = vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress)
+            ssh = vm.get_ssh_client(ipaddress=vm.nic[0].ipaddress, retries=5)
 
             # Ping to outsite world
             res = ssh.execute("ping -c 1 www.google.com")
@@ -2695,7 +2759,7 @@ class TestSecurityGroups_BasicSanity(cloudstackTestCase):
                          )
         return
 
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_32_delete_default_security_group(self):
         """ Test Delete the default security group when No VMs are deployed"""
 
@@ -2741,6 +2805,26 @@ class TestSharedNetworkOperations(cloudstackTestCase):
         # Get Zone, Domain and templates
         cls.domain = get_domain(cls.api_client)
         cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+        if cls.zone.securitygroupsenabled is False:
+            physical_networks = PhysicalNetwork.list(cls.api_client, zoneid=cls.zone.id)
+            selected_physical_network = None
+            for net in physical_networks:
+                traffic_types = TrafficType.list(cls.api_client, physicalnetworkid=net.id)
+                for traffic_type in traffic_types:
+                    if traffic_type.traffictype == 'Guest':
+                        selected_physical_network = net
+                        break
+                if selected_physical_network is not None:
+                    break
+            if selected_physical_network is None:
+                raise Exception("No physical network found with guest traffic type")
+
+            # Enable security group provider for physical network
+            nsps = NetworkServiceProvider.list(cls.api_client, physicalnetworkid=selected_physical_network.id, name='SecurityGroupProvider')
+            if len(nsps) == 0:
+                raise Exception("No security group provider found for physical network")
+            NetworkServiceProvider.update(cls.api_client, nsps[0].id, state='Enabled')
+
         cls.template = get_template(cls.api_client,cls.zone.id,cls.services["ostype"])
 
         cls.services["virtual_machine"]["zoneid"] = cls.zone.id
@@ -2864,7 +2948,7 @@ class TestSharedNetworkOperations(cloudstackTestCase):
         return
 
     @data("account","domain","zone")
-    @attr(tags = ["advancedsg"])
+    @attr(tags = ["advancedsg", "advanced"])
     def test_34_restart_shared_network_sg(self, value):
         """ Test restart account/domain/zone wide shared network"""
 
@@ -3196,7 +3280,7 @@ class TestAccountBasedIngressRules(cloudstackTestCase):
         self.api_client.authorizeSecurityGroupIngress(cmd)
 
         self.debug("Getting SSH client of virtual machine 1: %s" % self.virtual_machine_1.id)
-        sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress)
+        sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress, retries=5)
         try:
             if value == "accessByIp":
                 self.debug("SSHing into vm_2 %s from vm_1 %s" % (self.virtual_machine_2.nic[0].ipaddress,self.virtual_machine_1.nic[0].ipaddress))
@@ -3255,7 +3339,7 @@ class TestAccountBasedIngressRules(cloudstackTestCase):
         self.api_client.authorizeSecurityGroupIngress(cmd)
 
         self.debug("Getting SSH client of virtual machine 1: %s" % self.virtual_machine_1.id)
-        sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress)
+        sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress, retries=5)
         try:
             self.debug("SSHing into vm_2 %s from vm_1 %s" % (self.virtual_machine_2.nic[0].ipaddress,self.virtual_machine_1.nic[0].ipaddress))
             command = "ping -c 1 %s" % self.virtual_machine_2.nic[0].ipaddress
@@ -3317,7 +3401,7 @@ class TestAccountBasedIngressRules(cloudstackTestCase):
         self.debug("Deployed vm: %s" % self.virtual_machine_4.id)
 
         self.debug("Getting SSH client of virtual machine 1: %s" % self.virtual_machine_3.id)
-        sshClient = self.virtual_machine_3.get_ssh_client(ipaddress=self.virtual_machine_3.nic[0].ipaddress)
+        sshClient = self.virtual_machine_3.get_ssh_client(ipaddress=self.virtual_machine_3.nic[0].ipaddress, retries=5)
         try:
             if value == "accessByIp":
                 self.debug("SSHing into vm_4 %s from vm_3 %s" % (self.virtual_machine_4.nic[0].ipaddress,self.virtual_machine_3.nic[0].ipaddress))
@@ -3386,7 +3470,7 @@ class TestAccountBasedIngressRules(cloudstackTestCase):
         self.debug("Deployed vm: %s" % self.virtual_machine_3.id)
 
         self.debug("Getting SSH client of virtual machine 1: %s" % self.virtual_machine_3.id)
-        sshClient = self.virtual_machine_3.get_ssh_client(ipaddress=self.virtual_machine_3.nic[0].ipaddress)
+        sshClient = self.virtual_machine_3.get_ssh_client(ipaddress=self.virtual_machine_3.nic[0].ipaddress, retries=5)
         try:
             if value == "accessByIp":
                 self.debug("SSHing into vm_2 %s from vm_3 %s" % (self.virtual_machine_2.nic[0].ipaddress,self.virtual_machine_3.nic[0].ipaddress))
@@ -3446,7 +3530,7 @@ class TestAccountBasedIngressRules(cloudstackTestCase):
         self.debug("Getting SSH client of virtual machine 1: %s" % self.virtual_machine_1.id)
 
         try:
-            sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress)
+            sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress, retries=5)
             self.debug("SSHing into vm_2 %s from vm_1 %s" % (self.virtual_machine_2.nic[0].ipaddress,self.virtual_machine_1.nic[0].ipaddress))
             command = "ssh -t -t root@%s" % self.virtual_machine_2.nic[0].ipaddress
             self.debug("command: --> %s" % command)
@@ -3468,7 +3552,7 @@ class TestAccountBasedIngressRules(cloudstackTestCase):
         self.debug("Getting SSH client of virtual machine 1: %s" % self.virtual_machine_1.id)
 
         try:
-            sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress)
+            sshClient = self.virtual_machine_1.get_ssh_client(ipaddress=self.virtual_machine_1.nic[0].ipaddress, retries=5)
             self.debug("SSHing into vm_2 %s from vm_1 %s" % (self.virtual_machine_2.nic[0].ipaddress,self.virtual_machine_1.nic[0].ipaddress))
             command = "ssh -t -t root@%s" % self.virtual_machine_2.nic[0].ipaddress
             self.debug("command: --> %s" % command)

test/integration/component/test_security_groups.py

@@ -28,7 +28,10 @@ from marvin.lib.base import (Account,
                              SecurityGroup,
                              Router,
                              Host,
-                             Network)
+                             Network,
+                             PhysicalNetwork,
+                             TrafficType,
+                             NetworkServiceProvider)
 from marvin.lib.common import (get_domain,
                                get_zone,
                                get_template,
@@ -73,6 +76,26 @@ class TestDefaultSecurityGroup(cloudstackTestCase):
         # Get Zone, Domain and templates
         cls.domain = get_domain(cls.api_client)
         cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+        if cls.zone.securitygroupsenabled is False:
+            physical_networks = PhysicalNetwork.list(cls.api_client, zoneid=cls.zone.id)
+            selected_physical_network = None
+            for net in physical_networks:
+                traffic_types = TrafficType.list(cls.api_client, physicalnetworkid=net.id)
+                for traffic_type in traffic_types:
+                    if traffic_type.traffictype == 'Guest':
+                        selected_physical_network = net
+                        break
+                if selected_physical_network is not None:
+                    break
+            if selected_physical_network is None:
+                raise Exception("No physical network found with guest traffic type")
+
+            # Enable security group provider for physical network
+            nsps = NetworkServiceProvider.list(cls.api_client, physicalnetworkid=selected_physical_network.id, name='SecurityGroupProvider')
+            if len(nsps) == 0:
+                raise Exception("No security group provider found for physical network")
+            NetworkServiceProvider.update(cls.api_client, nsps[0].id, state='Enabled')
+
         cls.testdata['mode'] = cls.zone.networktype
 
         template = get_template(
@@ -115,7 +138,7 @@ class TestDefaultSecurityGroup(cloudstackTestCase):
 
         return
 
-    @attr(tags=["sg", "basic", "eip", "advancedsg"])
+    @attr(tags=["sg", "basic", "eip", "advancedsg", "advanced"])
     def test_01_deployVM_InDefaultSecurityGroup(self):
         """Test deploy VM in default security group
         """
@@ -193,7 +216,7 @@ class TestDefaultSecurityGroup(cloudstackTestCase):
         )
         return
 
-    @attr(tags=["sg", "basic", "eip", "advancedsg"])
+    @attr(tags=["sg", "basic", "eip", "advancedsg", "advanced"])
     def test_02_listSecurityGroups(self):
         """Test list security groups for admin account
         """
@@ -228,7 +251,7 @@ class TestDefaultSecurityGroup(cloudstackTestCase):
         )
         return
 
-    @attr(tags=["sg", "basic", "eip", "advancedsg"])
+    @attr(tags=["sg", "basic", "eip", "advancedsg", "advanced"])
     def test_03_accessInDefaultSecurityGroup(self):
         """Test access in default security group
         """
@@ -314,7 +337,8 @@ class TestDefaultSecurityGroup(cloudstackTestCase):
                 self.virtual_machine.ssh_ip,
                 self.virtual_machine.ssh_port,
                 self.virtual_machine.username,
-                self.virtual_machine.password
+                self.virtual_machine.password,
+                retries=5
             )
         return
 
@@ -453,7 +477,7 @@ class TestAuthorizeIngressRule(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % self.virtual_machine.id)
-            self.virtual_machine.get_ssh_client()
+            self.virtual_machine.get_ssh_client(retries=5)
         except Exception as e:
             self.fail("SSH Access failed for %s: %s" %
                       (self.virtual_machine.ipaddress, e)
@@ -604,7 +628,7 @@ class TestRevokeIngressRule(cloudstackTestCase):
         # Should be able to SSH VM
         try:
             self.debug("SSH into VM: %s" % self.virtual_machine.id)
-            self.virtual_machine.get_ssh_client()
+            self.virtual_machine.get_ssh_client(retries=5)
         except Exception as e:
             self.fail("SSH Access failed for %s: %s" %
                       (self.virtual_machine.ipaddress, e)
@@ -937,7 +961,7 @@ class TestdeployVMWithUserData(cloudstackTestCase):
                 % self.virtual_machine.ssh_ip
             )
 
-            ssh = self.virtual_machine.get_ssh_client()
+            ssh = self.virtual_machine.get_ssh_client(retries=5)
         except Exception as e:
             self.fail("SSH Access failed for %s: %s" %
                       (self.virtual_machine.ipaddress, e)
@@ -1348,7 +1372,7 @@ class TestIngressRule(cloudstackTestCase):
                 self.virtual_machine.ssh_ip,
                 self.testdata["ingress_rule"]["endport"]
             ))
-            self.virtual_machine.get_ssh_client()
+            self.virtual_machine.get_ssh_client(retries=5)
 
         except Exception as e:
             self.fail("SSH access failed for ingress rule ID: %s, %s"
@@ -1476,7 +1500,7 @@ class TestIngressRule(cloudstackTestCase):
                 self.virtual_machine.ssh_ip,
                 self.testdata["ingress_rule"]["endport"]
             ))
-            self.virtual_machine.get_ssh_client()
+            self.virtual_machine.get_ssh_client(retries=5)
 
         except Exception as e:
             self.fail("SSH access failed for ingress rule ID: %s, %s"
@@ -1623,7 +1647,7 @@ class TestIngressRule(cloudstackTestCase):
             self.debug(
                 "Trying to SSH into VM %s" %
                 self.virtual_machine.ssh_ip)
-            self.virtual_machine.get_ssh_client()
+            self.virtual_machine.get_ssh_client(retries=5)
         except Exception as e:
             self.fail("SSH access failed for ingress rule ID: %s"
                       % ingress_rule["id"]
@@ -1642,7 +1666,7 @@ class TestIngressRule(cloudstackTestCase):
             self.debug(
                 "Trying to SSH into VM %s" %
                 self.virtual_machine.ssh_ip)
-            self.virtual_machine.get_ssh_client()
+            self.virtual_machine.get_ssh_client(retries=5)
         except Exception as e:
             self.fail("SSH access failed for ingress rule ID: %s"
                       % ingress_rule["id"]

ui/public/locales/en.json

@@ -209,6 +209,7 @@
 "label.action.unmanage.volume": "Unmanage Volume",
 "label.action.unmanage.volumes": "Unmanage Volumes",
 "label.action.update.host": "Update host",
+"label.action.update.security.groups": "Update security groups",
 "label.action.update.offering.access": "Update offering access",
 "label.action.update.resource.count": "Update resource count",
 "label.action.value": "Action/Value",

ui/src/config/section/network.js

@@ -307,10 +307,7 @@ export default {
           return false
         }
         const listZoneHaveSGEnabled = store.getters.zones.filter(zone => zone.securitygroupsenabled === true)
-        if (!listZoneHaveSGEnabled || listZoneHaveSGEnabled.length === 0) {
+        return (listZoneHaveSGEnabled && listZoneHaveSGEnabled.length > 0) || store.getters.showSecurityGroups
-          return false
-        }
-        return true
       },
       actions: [
         {

ui/src/store/getters.js

@@ -36,6 +36,7 @@ const getters = {
   isLdapEnabled: state => state.user.isLdapEnabled,
   cloudian: state => state.user.cloudian,
   zones: state => state.user.zones,
+  showSecurityGroups: state => state.user.showSecurityGroups,
   timezoneoffset: state => state.user.timezoneoffset,
   usebrowsertimezone: state => state.user.usebrowsertimezone,
   server: state => state.app.server,

ui/src/store/modules/user.js

@@ -34,6 +34,7 @@ import {
   DEFAULT_THEME,
   APIS,
   ZONES,
+  SHOW_SECURTIY_GROUPS,
   TIMEZONE_OFFSET,
   USE_BROWSER_TIMEZONE,
   HEADER_NOTICES,
@@ -124,6 +125,10 @@ const user = {
       state.zones = zones
       vueProps.$localStorage.set(ZONES, zones)
     },
+    SET_SHOW_SECURITY_GROUPS: (state, show) => {
+      state.showSecurityGroups = show
+      vueProps.$localStorage.set(SHOW_SECURTIY_GROUPS, show)
+    },
     SET_DOMAIN_STORE (state, domainStore) {
       state.domainStore = domainStore
       vueProps.$localStorage.set(DOMAIN_STORE, domainStore)
@@ -290,6 +295,7 @@ const user = {
         const cachedUseBrowserTimezone = vueProps.$localStorage.get(USE_BROWSER_TIMEZONE, false)
         const cachedCustomColumns = vueProps.$localStorage.get(CUSTOM_COLUMNS, {})
         const domainStore = vueProps.$localStorage.get(DOMAIN_STORE, {})
+        const cachedShowSecurityGroups = vueProps.$localStorage.get(SHOW_SECURTIY_GROUPS, false)
         const darkMode = vueProps.$localStorage.get(DARK_MODE, false)
         const latestVersion = vueProps.$localStorage.get(LATEST_CS_VERSION, { version: '', fetchedTs: 0 })
         const hasAuth = Object.keys(cachedApis).length > 0
@@ -300,6 +306,7 @@ const user = {
         if (hasAuth) {
           console.log('Login detected, using cached APIs')
           commit('SET_ZONES', cachedZones)
+          commit('SET_SHOW_SECURITY_GROUPS', cachedShowSecurityGroups)
           commit('SET_APIS', cachedApis)
           commit('SET_TIMEZONE_OFFSET', cachedTimezoneOffset)
           commit('SET_USE_BROWSER_TIMEZONE', cachedUseBrowserTimezone)
@@ -388,6 +395,15 @@ const user = {
           reject(error)
         })
 
+        api(
+          'listNetworkServiceProviders',
+          { name: 'SecurityGroupProvider', state: 'Enabled' }
+        ).then(response => {
+          const showSecurityGroups = response.listnetworkserviceprovidersresponse.count > 0
+          commit('SET_SHOW_SECURITY_GROUPS', showSecurityGroups)
+        }).catch(ignored => {
+        })
+
         api('listCapabilities').then(response => {
           const result = response.listcapabilitiesresponse.capability
           commit('SET_FEATURES', result)
@@ -397,6 +413,9 @@ const user = {
           if (result && result.customhypervisordisplayname) {
             commit('SET_CUSTOM_HYPERVISOR_NAME', result.customhypervisordisplayname)
           }
+          if (result && result.securitygroupsenabled) {
+            commit('SET_SHOW_SECURITY_GROUPS', result.securitygroupsenabled)
+          }
         }).catch(error => {
           reject(error)
         })

ui/src/store/mutation-types.js

@@ -29,6 +29,7 @@ export const DEFAULT_CONTENT_WIDTH_TYPE = 'DEFAULT_CONTENT_WIDTH_TYPE'
 export const DEFAULT_MULTI_TAB = 'DEFAULT_MULTI_TAB'
 export const APIS = 'APIS'
 export const ZONES = 'ZONES'
+export const SHOW_SECURTIY_GROUPS = 'SHOW_SECURITY_GROUPS'
 export const HEADER_NOTICES = 'HEADER_NOTICES'
 export const TIMEZONE_OFFSET = 'TIMEZONE_OFFSET'
 export const USE_BROWSER_TIMEZONE = 'USE_BROWSER_TIMEZONE'

ui/src/views/compute/DeployVM.vue

@@ -1369,7 +1369,19 @@ export default {
       return tabList
     },
     showSecurityGroupSection () {
-      return (this.networks.length > 0 && this.zone?.securitygroupsenabled) || (this.zone?.networktype === 'Basic')
+      if (this.networks.length < 1) {
+        return false
+      }
+      for (const network of this.options.networks) {
+        if (this.form.networkids && this.form.networkids.includes(network.id)) {
+          for (const service of network.service) {
+            if (service.name === 'SecurityGroup') {
+              return true
+            }
+          }
+        }
+      }
+      return false
     },
     isUserAllowedToListSshKeys () {
       return Boolean('listSSHKeyPairs' in this.$store.getters.apis)

ui/src/views/compute/InstanceTab.vue

@@ -117,7 +117,14 @@
           :routerlinks="(record) => { return { id: '/backup/' + record.id } }"
           :showSearch="false"/>
       </a-tab-pane>
-      <a-tab-pane :tab="$t('label.securitygroups')" key="securitygroups" v-if="dataResource.securitygroup && dataResource.securitygroup.length > 0">
+      <a-tab-pane :tab="$t('label.securitygroups')" key="securitygroups" v-if="dataResource.securitygroup && dataResource.securitygroup.length > 0 || $store.getters.showSecurityGroups">
+        <a-button
+          type="primary"
+          style="width: 100%; margin-bottom: 10px"
+          @click="showUpdateSGModal"
+          :loading="loading">
+          <template #icon><edit-outlined /></template> {{ $t('label.action.update.security.groups') }}
+        </a-button>
         <ListResourceTable
           :items="dataResource.securitygroup"
           :columns="['name', 'description']"
@@ -143,6 +150,21 @@
       </a-tab-pane>
     </a-tabs>
 
+    <a-modal
+      :visible="showUpdateSecurityGroupsModal"
+      :title="$t('label.action.update.security.groups')"
+      :maskClosable="false"
+      :closable="true"
+      @ok="updateSecurityGroups"
+      @cancel="closeModals">
+      <security-group-selection
+        :zoneId="this.vm.zoneid"
+        :value="securitygroupids"
+        :loading="false"
+        :preFillContent="dataPreFill"
+        @select-security-group-item="($event) => updateSecurityGroupsSelection($event)"></security-group-selection>
+    </a-modal>
+
     <a-modal
       :visible="showAddVolumeModal"
       :title="$t('label.action.create.volume.add')"
@@ -320,6 +342,7 @@ import TooltipButton from '@/components/widgets/TooltipButton'
 import ResourceIcon from '@/components/view/ResourceIcon'
 import AnnotationsTab from '@/components/view/AnnotationsTab'
 import VolumesTab from '@/components/view/VolumesTab.vue'
+import SecurityGroupSelection from '@views/compute/wizard/SecurityGroupSelection'
 
 export default {
   name: 'InstanceTab',
@@ -333,6 +356,7 @@ export default {
     NicsTable,
     InstanceSchedules,
     ListResourceTable,
+    SecurityGroupSelection,
     TooltipButton,
     ResourceIcon,
     AnnotationsTab,
@@ -359,6 +383,7 @@ export default {
       showAddNetworkModal: false,
       showUpdateIpModal: false,
       showSecondaryIpModal: false,
+      showUpdateSecurityGroupsModal: false,
       diskOfferings: [],
       addNetworkData: {
         allNetworks: [],
@@ -366,6 +391,7 @@ export default {
         ip: ''
       },
       loadingNic: false,
+      loadingSG: false,
       editIpAddressNic: '',
       editIpAddressValue: '',
       editNetworkId: '',
@@ -378,7 +404,9 @@ export default {
         opts: []
       },
       annotations: [],
-      dataResource: {}
+      dataResource: {},
+      dataPreFill: {},
+      securitygroupids: []
     }
   },
   created () {
@@ -501,11 +529,24 @@ export default {
       this.showAddNetworkModal = true
       this.listNetworks()
     },
+    showUpdateSGModal () {
+      this.loadingSG = true
+      if (this.vm.securitygroup && this.vm.securitygroup?.length > 0) {
+        this.securitygroupids = []
+        for (const sg of this.vm.securitygroup) {
+          this.securitygroupids.push(sg.id)
+        }
+        this.dataPreFill = { securitygroupids: this.securitygroupids }
+      }
+      this.showUpdateSecurityGroupsModal = true
+      this.loadingSG = false
+    },
     closeModals () {
       this.showAddVolumeModal = false
       this.showAddNetworkModal = false
       this.showUpdateIpModal = false
       this.showSecondaryIpModal = false
+      this.showUpdateSecurityGroupsModal = false
       this.addNetworkData.network = ''
       this.addNetworkData.ip = ''
       this.editIpAddressValue = ''
@@ -730,6 +771,17 @@ export default {
         this.loadingNic = false
         this.fetchSecondaryIPs(this.selectedNicId)
       })
+    },
+    updateSecurityGroupsSelection (securitygroupids) {
+      this.securitygroupids = securitygroupids || []
+    },
+    updateSecurityGroups () {
+      api('updateVirtualMachine', { id: this.vm.id, securitygroupids: this.securitygroupids.join(',') }).catch(error => {
+        this.$notifyError(error)
+      }).finally(() => {
+        this.closeModals()
+        this.parentFetchData()
+      })
     }
   }
 }

ui/src/views/compute/wizard/SecurityGroupSelection.vue

@@ -94,7 +94,7 @@ export default {
         }
       ],
       items: [],
-      selectedRowKeys: [],
+      selectedRowKeys: this?.preFillContent?.securitygroupids || [],
       page: 1,
       pageSize: 10,
       keyword: null,

ui/src/views/infra/network/ServiceProvidersTab.vue

@@ -937,10 +937,38 @@ export default {
             }
           ]
         },
-        // {
+        {
-        //   title: 'SecurityGroupProvider',
+          title: 'SecurityGroupProvider',
-        //   details: ['name', 'state', 'id', 'servicelist'],
+          details: ['name', 'state', 'id', 'servicelist'],
-        // },
+          actions: [
+            {
+              api: 'updateNetworkServiceProvider',
+              icon: 'stop-outlined',
+              listView: true,
+              label: 'label.disable.provider',
+              confirm: 'message.confirm.disable.provider',
+              show: (record) => { return record && record.id && record.state === 'Enabled' },
+              mapping: {
+                state: {
+                  value: (record) => { return 'Disabled' }
+                }
+              }
+            },
+            {
+              api: 'updateNetworkServiceProvider',
+              icon: 'play-circle-outlined',
+              listView: true,
+              label: 'label.enable.provider',
+              confirm: 'message.confirm.enable.provider',
+              show: (record) => { return record && record.id && record.state === 'Disabled' },
+              mapping: {
+                state: {
+                  value: (record) => { return 'Enabled' }
+                }
+              }
+            }
+          ]
+        },
         {
           title: 'VirtualRouter',
           actions: [