Isolated security groups with conf toggle

Allow isolated networks security groups using the
QUARK.environment_capabilities configuration value.

JIRA:NCP-1465
JIRA:NCP-1681

quark/cache/security_groups_client.py

@@ -16,13 +16,17 @@
 import json
 
 import netaddr
+from oslo_config import cfg
 from oslo_log import log as logging
 
 from quark.cache import redis_base
+from quark.environment import Capabilities
+from quark import exceptions as q_exc
 from quark import protocols
 from quark import utils
 
 
+CONF = cfg.CONF
 LOG = logging.getLogger(__name__)
 SECURITY_GROUP_RULE_KEY = "rules"
 SECURITY_GROUP_HASH_ATTR = "security group rules"
@@ -60,6 +64,10 @@ class SecurityGroupsClient(redis_base.ClientBase):
                 prefix = rule["remote_ip_prefix"]
                 if direction == "ingress":
                     source = self._convert_remote_network(prefix)
+                else:
+                    if (Capabilities.EGRESS not in
+                            CONF.QUARK.environment_capabilities):
+                        raise q_exc.EgressSecurityGroupRulesNotEnabled()
                     else:
                         destination = self._convert_remote_network(prefix)
 

quark/drivers/nvp_driver.py

@@ -26,6 +26,8 @@ from oslo_config import cfg
 from oslo_log import log as logging
 
 from quark.drivers import base
+from quark.drivers import security_groups as sg_driver
+from quark.environment import Capabilities
 from quark import exceptions
 from quark import utils
 
@@ -100,6 +102,9 @@ class NVPDriver(base.BaseDriver):
         self.limits = {'max_ports_per_switch': 0,
                        'max_rules_per_group': 0,
                        'max_rules_per_port': 0}
+        self.sg_driver = None
+        if Capabilities.SECURITY_GROUPS in CONF.QUARK.environment_capabilities:
+            self.sg_driver = sg_driver.SecurityGroupDriver()
         super(NVPDriver, self).__init__()
 
     @classmethod
@@ -268,9 +273,8 @@ class NVPDriver(base.BaseDriver):
         return {'logical_switches': [self._collect_lswitch_info(s, get_status)
                 for s in switches]}
 
-    def create_port(self, context, network_id, port_id,
+    def create_port(self, context, network_id, port_id, status=True,
-                    status=True, security_groups=None,
+                    security_groups=None, device_id="", **kwargs):
-                    device_id="", **kwargs):
         security_groups = security_groups or []
         tenant_id = context.tenant_id
         lswitch = self._create_or_choose_lswitch(context, network_id)
@@ -280,6 +284,7 @@ class NVPDriver(base.BaseDriver):
             with self.get_connection() as connection:
                 port = connection.lswitch_port(lswitch)
                 port.admin_status_enabled(status)
+                if not self.sg_driver:
                     nvp_group_ids = self._get_security_groups_for_port(
                         context, security_groups)
                     port.security_profiles(nvp_group_ids)
@@ -291,8 +296,8 @@ class NVPDriver(base.BaseDriver):
                 port.tags(tags)
                 res = port.create()
                 try:
-                    """Catching odd NVP returns here will make it safe to assume that
+                    """Catching odd NVP returns here will make it safe to
-                    NVP returned something correct."""
+                    assume that NVP returned something correct."""
                     res["lswitch"] = lswitch
                 except TypeError:
                     LOG.exception("Unexpected return from NVP: %s" % res)
@@ -304,14 +309,22 @@ class NVPDriver(base.BaseDriver):
         return _create_lswitch_port()
 
     @utils.retry_loop(CONF.NVP.operation_retries)
-    def update_port(self, context, port_id, status=True,
+    def update_port(self, context, port_id, mac_address=None, device_id=None,
-                    security_groups=None, **kwargs):
+                    status=True, security_groups=None, **kwargs):
+        if not self.sg_driver:
             security_groups = security_groups or []
+        else:
+            kwargs.update({'security_groups': security_groups})
         with self.get_connection() as connection:
+            if self.sg_driver:
+                kwargs.update({'mac_addres': mac_address,
+                               'device_id': device_id})
+                self.sg_driver.update_port(**kwargs)
             lswitch_id = self._lswitch_from_port(context, port_id)
             port = connection.lswitch_port(lswitch_id, port_id)
-            nvp_group_ids = self._get_security_groups_for_port(context,
+            if not self.sg_driver:
-                                                               security_groups)
+                nvp_group_ids = self._get_security_groups_for_port(
+                    context, security_groups)
                 if nvp_group_ids:
                     port.security_profiles(nvp_group_ids)
             port.admin_status_enabled(status)
@@ -327,6 +340,8 @@ class NVPDriver(base.BaseDriver):
                 LOG.debug("Deleting port %s from lswitch %s"
                           % (port_id, lswitch_uuid))
                 connection.lswitch_port(lswitch_uuid, port_id).delete()
+                if self.sg_driver:
+                    self.sg_driver.delete_port(**kwargs)
             except aiclib.core.AICException as ae:
                 if ae.code == 404:
                     LOG.info("LSwitchPort/Port %s not found in NVP."

quark/drivers/optimized_nvp_driver.py

@@ -91,9 +91,11 @@ class OptimizedNVPDriver(NVPDriver):
     def update_port(self, context, port_id, status=True,
                     security_groups=None, **kwargs):
         security_groups = security_groups or []
+        mac_address = kwargs.get('mac_address')
+        device_id = kwargs.get('device_id')
         nvp_port = super(OptimizedNVPDriver, self).update_port(
-            context, port_id, status=status,
+            context, port_id, mac_address=mac_address, device_id=device_id,
-            security_groups=security_groups)
+            status=status, security_groups=security_groups)
         port = self._lport_select_by_id(context, port_id)
         port.update(nvp_port)
 

quark/environment.py

@@ -21,6 +21,7 @@ CONF = cfg.CONF
 
 class Capabilities(object):
     SECURITY_GROUPS = "security_groups"
+    EGRESS = "egress"
 
 
 quark_opts = [

quark/exceptions.py

@@ -116,9 +116,9 @@ class SecurityGroupsNotImplemented(exceptions.InvalidInput):
                 "create")
 
 
-class TenantNetworkSecurityGroupsNotImplemented(exceptions.InvalidInput):
+class EgressSecurityGroupRulesNotEnabled(exceptions.InvalidInput):
-    message = _("Security Groups are not currently implemented for "
+    message = _("Egress security group rules are not currently allowed "
-                "tenant networks")
+                "by environment_capabilities configuration.")
 
 
 class SecurityGroupsRequireDevice(exceptions.InvalidInput):

quark/plugin_modules/ports.py

@@ -23,6 +23,7 @@ from oslo_utils import uuidutils
 
 from quark.db import api as db_api
 from quark.drivers import registry
+from quark.environment import Capabilities
 from quark import exceptions as q_exc
 from quark import ipam
 from quark import network_strategy
@@ -145,8 +146,8 @@ def create_port(context, port):
 
     net_driver = registry.DRIVER_REGISTRY.get_driver(net["network_plugin"])
 
-    # TODO(anyone): security groups are not currently supported on port create,
+    # TODO(anyone): security groups are not currently supported on port create.
-    #               nor on isolated networks today. Please see RM8615
+    #               Please see JIRA:NCP-801
     security_groups = utils.pop_param(port_attrs, "security_groups")
     if security_groups is not None:
         raise q_exc.SecurityGroupsNotImplemented()
@@ -294,9 +295,8 @@ def update_port(context, id, port):
         quota.QUOTAS.limit_check(context, context.tenant_id,
                                  fixed_ips_per_port=len(fixed_ips))
 
-    # TODO(anyone): security groups are not currently supported on port create,
-    #               nor on isolated networks today. Please see RM8615
     new_security_groups = utils.pop_param(port_dict, "security_groups")
+    if Capabilities.SECURITY_GROUPS not in CONF.QUARK.environment_capabilities:
         if new_security_groups is not None:
             if not STRATEGY.is_parent_network(port_db["network_id"]):
                 raise q_exc.TenantNetworkSecurityGroupsNotImplemented()

quark/tests/cache/test_security_groups_client.py

@@ -23,6 +23,7 @@ import redis
 from quark.agent.xapi import VIF
 from quark.cache import security_groups_client as sg_client
 from quark.db import models
+from quark.environment import Capabilities
 from quark import exceptions as q_exc
 from quark.tests import test_base
 
@@ -33,6 +34,13 @@ class TestRedisSecurityGroupsClient(test_base.TestBase):
 
     def setUp(self):
         super(TestRedisSecurityGroupsClient, self).setUp()
+        # Forces the connection pool to be recreated on every test
+        sg_client.SecurityGroupsClient.connection_pool = None
+        temp_envcaps = [Capabilities.SECURITY_GROUPS, Capabilities.EGRESS]
+        CONF.set_override('environment_capabilities', temp_envcaps, 'QUARK')
+
+    def tearDown(self):
+        CONF.clear_override('environment_capabilities', 'QUARK')
 
     @mock.patch("uuid.uuid4")
     @mock.patch("quark.cache.redis_base.TwiceRedis")

quark/tests/plugin_modules/test_ports.py

@@ -769,17 +769,6 @@ class TestQuarkUpdatePortSecurityGroups(test_quark_plugin.TestQuarkPlugin):
             yield (port_find, port_update, alloc_ip, dealloc_ip, sg_find,
                    driver_port_update)
 
-    def test_update_port_security_groups_on_tenant_net_raises(self):
-        with self._stubs(
-            port=dict(id=1, device_id="device")
-        ) as (port_find, port_update, alloc_ip, dealloc_ip, sg_find,
-              driver_port_update):
-            new_port = dict(port=dict(name="ourport",
-                                      security_groups=[1]))
-            with self.assertRaises(
-                    q_exc.TenantNetworkSecurityGroupsNotImplemented):
-                self.plugin.update_port(self.context, 1, new_port)
-
     def test_update_port_security_groups(self):
         with self._stubs(
             port=dict(id=1, device_id="device"), parent_net=True

quark/tests/test_nvp_driver.py

@@ -29,10 +29,10 @@ from quark.tests import test_base
 class TestNVPDriver(test_base.TestBase):
     def setUp(self):
         super(TestNVPDriver, self).setUp()
-
+        cfg.CONF.set_override('environment_capabilities', [], 'QUARK')
         if not hasattr(self, 'driver'):
             self.driver = quark.drivers.nvp_driver.NVPDriver()
-
+        cfg.CONF.clear_override('environment_capabilities', 'QUARK')
         cfg.CONF.set_override('max_rules_per_group', 3, 'NVP')
         cfg.CONF.set_override('max_rules_per_port', 1, 'NVP')
         self.driver.max_ports_per_switch = 0
@@ -491,6 +491,7 @@ class TestNVPDriverCreatePort(TestNVPDriver):
             self.assertTrue(False in status_args)
 
     def test_create_port_with_security_groups(self):
+        cfg.CONF.set_override('environment_capabilities', [], 'QUARK')
         with self._stubs() as connection:
             connection.securityprofile = self._create_security_profile()
             self.driver.create_port(self.context, self.net_id,
@@ -499,8 +500,10 @@ class TestNVPDriverCreatePort(TestNVPDriver):
             connection.lswitch_port().assert_has_calls([
                 mock.call.security_profiles([self.profile_id]),
             ], any_order=True)
+        cfg.CONF.clear_override('environment_capabilities', 'QUARK')
 
     def test_create_port_with_security_groups_max_rules(self):
+        cfg.CONF.set_override('environment_capabilities', [], 'QUARK')
         with self._stubs() as connection:
             connection.securityprofile = self._create_security_profile()
             connection.securityprofile().read().update(
@@ -512,6 +515,7 @@ class TestNVPDriverCreatePort(TestNVPDriver):
                 self.driver.create_port(
                     self.context, self.net_id, self.port_id,
                     security_groups=[1])
+        cfg.CONF.clear_override('environment_capabilities', 'QUARK')
 
 
 class TestNVPDriverUpdatePort(TestNVPDriver):
@@ -527,6 +531,7 @@ class TestNVPDriverUpdatePort(TestNVPDriver):
             yield connection
 
     def test_update_port(self):
+        cfg.CONF.set_override('environment_capabilities', [], 'QUARK')
         with self._stubs() as connection:
             self.driver.update_port(
                 self.context, self.port_id,
@@ -534,8 +539,10 @@ class TestNVPDriverUpdatePort(TestNVPDriver):
             connection.lswitch_port().assert_has_calls([
                 mock.call.security_profiles([self.profile_id]),
             ], any_order=True)
+        cfg.CONF.clear_override('environment_capabilities', 'QUARK')
 
     def test_update_port_max_rules(self):
+        cfg.CONF.set_override('environment_capabilities', [], 'QUARK')
         with self._stubs() as connection:
             connection.securityprofile().read().update(
                 {'logical_port_ingress_rules': [{'ethertype': 'IPv4'},
@@ -546,6 +553,7 @@ class TestNVPDriverUpdatePort(TestNVPDriver):
                 self.driver.update_port(
                     self.context, self.port_id,
                     security_groups=[1])
+        cfg.CONF.clear_override('environment_capabilities', 'QUARK')
 
 
 class TestNVPDriverLswitchesForNetwork(TestNVPDriver):