From 868e67b480b08cc815d802cf950547c6b5ac0153 Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Thu, 16 Apr 2015 12:45:32 -0700 Subject: [PATCH] Add security groups events ML2 mech drivers have no direct exposure to security groups, and they can only infer them from the associated network/ports. This is problematic as agentless ML2 mech drivers have no way of intercepting securitygroups events and propagate the information to their backend, or more generally, react to them. This patch leverages the callback registry to dispatch such events so that interested ML2 mech drivers (or any interested party like service plugins) can be notified and react accordingly. This patch addresses create/update/delete of security groups and create/delete of security groups rules. Other events may be added over time, if need be. This patch is only about emitting the events. The actual subscription and implementation of the event handlers will have to take place where deemed appropriate. Closes-bug: #1444112 Change-Id: Ifa1d7ee9c967576f824f1129dd68e6e3abd48f5c --- neutron/callbacks/resources.py | 4 + neutron/db/securitygroups_db.py | 112 +++++++++++++++++- neutron/extensions/securitygroup.py | 20 +++- neutron/tests/base.py | 9 ++ .../tests/functional/agent/test_l3_agent.py | 7 -- .../tests/unit/db/test_securitygroups_db.py | 45 +++++++ 6 files changed, 185 insertions(+), 12 deletions(-) diff --git a/neutron/callbacks/resources.py b/neutron/callbacks/resources.py index f22791d22..f7831b8ef 100644 --- a/neutron/callbacks/resources.py +++ b/neutron/callbacks/resources.py @@ -14,10 +14,14 @@ PORT = 'port' ROUTER = 'router' ROUTER_GATEWAY = 'router_gateway' ROUTER_INTERFACE = 'router_interface' +SECURITY_GROUP = 'security_group' +SECURITY_GROUP_RULE = 'security_group_rule' VALID = ( PORT, ROUTER, ROUTER_GATEWAY, ROUTER_INTERFACE, + SECURITY_GROUP, + SECURITY_GROUP_RULE, ) diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 601279cca..f3572fd10 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -21,6 +21,10 @@ from sqlalchemy.orm import exc from sqlalchemy.orm import scoped_session from neutron.api.v2 import attributes +from neutron.callbacks import events +from neutron.callbacks import exceptions +from neutron.callbacks import registry +from neutron.callbacks import resources from neutron.common import constants from neutron.db import api as db_api from neutron.db import db_base_plugin_v2 @@ -125,6 +129,21 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): a given tenant if it does not exist. """ s = security_group['security_group'] + kwargs = { + 'context': context, + 'security_group': s, + 'is_default': default_sg, + } + # NOTE(armax): a callback exception here will prevent the request + # from being processed. This is a hook point for backend's validation; + # we raise to propagate the reason for the failure. + try: + registry.notify( + resources.SECURITY_GROUP, events.BEFORE_CREATE, self, + **kwargs) + except exceptions.CallbackFailure as e: + raise ext_sg.SecurityGroupConflict(reason=e) + tenant_id = self._get_tenant_id_for_create(context, s) if not default_sg: @@ -159,7 +178,12 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): ethertype=ethertype) context.session.add(egress_rule) - return self._make_security_group_dict(security_group_db) + secgroup_dict = self._make_security_group_dict(security_group_db) + + kwargs['security_group'] = secgroup_dict + registry.notify(resources.SECURITY_GROUP, events.AFTER_CREATE, self, + **kwargs) + return secgroup_dict def get_security_groups(self, context, filters=None, fields=None, sorts=None, limit=None, @@ -229,17 +253,58 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): if sg['name'] == 'default' and not context.is_admin: raise ext_sg.SecurityGroupCannotRemoveDefault() + kwargs = { + 'context': context, + 'security_group_id': id, + 'security_group': sg, + } + # NOTE(armax): a callback exception here will prevent the request + # from being processed. This is a hook point for backend's validation; + # we raise to propagate the reason for the failure. + try: + registry.notify( + resources.SECURITY_GROUP, events.BEFORE_DELETE, self, + **kwargs) + except exceptions.CallbackFailure as e: + reason = _('cannot be deleted due to %s') % e + raise ext_sg.SecurityGroupInUse(id=id, reason=reason) + with context.session.begin(subtransactions=True): context.session.delete(sg) + kwargs.pop('security_group') + registry.notify(resources.SECURITY_GROUP, events.AFTER_DELETE, self, + **kwargs) + def update_security_group(self, context, id, security_group): s = security_group['security_group'] + + kwargs = { + 'context': context, + 'security_group_id': id, + 'security_group': s, + } + # NOTE(armax): a callback exception here will prevent the request + # from being processed. This is a hook point for backend's validation; + # we raise to propagate the reason for the failure. + try: + registry.notify( + resources.SECURITY_GROUP, events.BEFORE_UPDATE, self, + **kwargs) + except exceptions.CallbackFailure as e: + raise ext_sg.SecurityGroupConflict(reason=e) + with context.session.begin(subtransactions=True): sg = self._get_security_group(context, id) if sg['name'] == 'default' and 'name' in s: raise ext_sg.SecurityGroupCannotUpdateDefault() sg.update(s) - return self._make_security_group_dict(sg) + sg_dict = self._make_security_group_dict(sg) + + kwargs['security_group'] = sg_dict + registry.notify(resources.SECURITY_GROUP, events.AFTER_UPDATE, self, + **kwargs) + return sg_dict def _make_security_group_dict(self, security_group, fields=None): res = {'id': security_group['id'], @@ -313,9 +378,29 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): return ret def create_security_group_rule(self, context, security_group_rule): + kwargs = { + 'context': context, + 'security_group_rule': security_group_rule, + } + # NOTE(armax): a callback exception here will prevent the request + # from being processed. This is a hook point for backend's validation; + # we raise to propagate the reason for the failure. + try: + registry.notify( + resources.SECURITY_GROUP_RULE, events.BEFORE_CREATE, self, + **kwargs) + except exceptions.CallbackFailure as e: + raise ext_sg.SecurityGroupConflict(reason=e) + bulk_rule = {'security_group_rules': [security_group_rule]} - return self.create_security_group_rule_bulk_native(context, - bulk_rule)[0] + sg_rule_dict = self.create_security_group_rule_bulk_native( + context, bulk_rule)[0] + + kwargs['security_group_rule'] = sg_rule_dict + registry.notify( + resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, self, + **kwargs) + return sg_rule_dict def _get_ip_proto_number(self, protocol): if protocol is None: @@ -494,11 +579,30 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): return sgr def delete_security_group_rule(self, context, id): + kwargs = { + 'context': context, + 'security_group_rule_id': id + } + # NOTE(armax): a callback exception here will prevent the request + # from being processed. This is a hook point for backend's validation; + # we raise to propagate the reason for the failure. + try: + registry.notify( + resources.SECURITY_GROUP_RULE, events.BEFORE_DELETE, self, + **kwargs) + except exceptions.CallbackFailure as e: + reason = _('cannot be deleted due to %s') % e + raise ext_sg.SecurityGroupRuleInUse(id=id, reason=reason) + with context.session.begin(subtransactions=True): query = self._model_query(context, SecurityGroupRule) if query.filter(SecurityGroupRule.id == id).delete() == 0: raise ext_sg.SecurityGroupRuleNotFound(id=id) + registry.notify( + resources.SECURITY_GROUP_RULE, events.AFTER_DELETE, self, + **kwargs) + def _extend_port_dict_security_group(self, port_res, port_db): # Security group bindings will be retrieved from the sqlalchemy # model. As they're loaded eagerly with ports because of the diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py index 7cb738ae8..e25be6364 100644 --- a/neutron/extensions/securitygroup.py +++ b/neutron/extensions/securitygroup.py @@ -50,7 +50,12 @@ class SecurityGroupMissingIcmpType(nexception.InvalidInput): class SecurityGroupInUse(nexception.InUse): - message = _("Security Group %(id)s in use.") + message = _("Security Group %(id)s %(reason)s.") + + def __init__(self, **kwargs): + if 'reason' not in kwargs: + kwargs['reason'] = _("in use") + super(SecurityGroupInUse, self).__init__(**kwargs) class SecurityGroupCannotRemoveDefault(nexception.InUse): @@ -106,10 +111,23 @@ class SecurityGroupRuleExists(nexception.InUse): message = _("Security group rule already exists. Rule id is %(id)s.") +class SecurityGroupRuleInUse(nexception.InUse): + message = _("Security Group Rule %(id)s %(reason)s.") + + def __init__(self, **kwargs): + if 'reason' not in kwargs: + kwargs['reason'] = _("in use") + super(SecurityGroupRuleInUse, self).__init__(**kwargs) + + class SecurityGroupRuleParameterConflict(nexception.InvalidInput): message = _("Conflicting value ethertype %(ethertype)s for CIDR %(cidr)s") +class SecurityGroupConflict(nexception.Conflict): + message = _("Error %(reason)s while attempting the operation.") + + def convert_protocol(value): if value is None: return diff --git a/neutron/tests/base.py b/neutron/tests/base.py index e6e3f44cf..d45b2d54e 100644 --- a/neutron/tests/base.py +++ b/neutron/tests/base.py @@ -35,6 +35,8 @@ from oslo_utils import strutils import testtools from neutron.agent.linux import external_process +from neutron.callbacks import manager as registry_manager +from neutron.callbacks import registry from neutron.common import config from neutron.common import rpc as n_rpc from neutron.db import agentschedulers_db @@ -251,6 +253,7 @@ class BaseTestCase(DietTestCase): self.setup_rpc_mocks() self.setup_config() + self.setup_test_registry_instance() policy.init() self.addCleanup(policy.reset) @@ -312,6 +315,12 @@ class BaseTestCase(DietTestCase): self.addCleanup(n_rpc.cleanup) n_rpc.init(CONF) + def setup_test_registry_instance(self): + """Give a private copy of the registry to each test.""" + self._callback_manager = registry_manager.CallbacksManager() + mock.patch.object(registry, '_get_callback_manager', + return_value=self._callback_manager).start() + def setup_config(self, args=None): """Tests that need a non-default config can override this method.""" self.config_parse(args=args) diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py index 43e189b59..cd9087b9d 100755 --- a/neutron/tests/functional/agent/test_l3_agent.py +++ b/neutron/tests/functional/agent/test_l3_agent.py @@ -38,7 +38,6 @@ from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib from neutron.agent.linux import utils from neutron.callbacks import events -from neutron.callbacks import manager from neutron.callbacks import registry from neutron.callbacks import resources from neutron.common import config as common_config @@ -64,12 +63,6 @@ class L3AgentTestFramework(base.BaseLinuxTestCase): def setUp(self): super(L3AgentTestFramework, self).setUp() mock.patch('neutron.agent.l3.agent.L3PluginApi').start() - - # TODO(pcm): Move this to BaseTestCase, if we find that more tests - # use this mechanism. - self._callback_manager = manager.CallbacksManager() - mock.patch.object(registry, '_get_callback_manager', - return_value=self._callback_manager).start() self.agent = self._configure_agent('agent1') def _get_config_opts(self): diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index 693668c3e..7f87802ac 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/neutron/tests/unit/db/test_securitygroups_db.py @@ -11,8 +11,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib +import mock import testtools +from neutron.callbacks import exceptions +from neutron.callbacks import registry from neutron import context from neutron.db import common_db_mixin from neutron.db import securitygroups_db @@ -32,6 +36,47 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): self.ctx = context.get_admin_context() self.mixin = SecurityGroupDbMixinImpl() + def test_create_security_group_conflict(self): + with mock.patch.object(registry, "notify") as mock_notify: + mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + secgroup = {'security_group': mock.ANY} + with testtools.ExpectedException( + securitygroup.SecurityGroupConflict): + self.mixin.create_security_group(self.ctx, secgroup) + + def test_delete_security_group_in_use(self): + with contextlib.nested( + mock.patch.object(self.mixin, '_get_port_security_group_bindings'), + mock.patch.object(self.mixin, '_get_security_group'), + mock.patch.object(registry, "notify"), + ) as (_, _, mock_notify): + mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + with testtools.ExpectedException( + securitygroup.SecurityGroupInUse): + self.mixin.delete_security_group(self.ctx, mock.ANY) + + def test_update_security_group_conflict(self): + with mock.patch.object(registry, "notify") as mock_notify: + mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + secgroup = {'security_group': mock.ANY} + with testtools.ExpectedException( + securitygroup.SecurityGroupConflict): + self.mixin.update_security_group(self.ctx, 'foo_id', secgroup) + + def test_create_security_group_rule_conflict(self): + with mock.patch.object(registry, "notify") as mock_notify: + mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + with testtools.ExpectedException( + securitygroup.SecurityGroupConflict): + self.mixin.create_security_group_rule(self.ctx, mock.ANY) + + def test_delete_security_group_rule_in_use(self): + with mock.patch.object(registry, "notify") as mock_notify: + mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + with testtools.ExpectedException( + securitygroup.SecurityGroupRuleInUse): + self.mixin.delete_security_group_rule(self.ctx, mock.ANY) + def test_delete_security_group_rule_raise_error_on_not_found(self): with testtools.ExpectedException( securitygroup.SecurityGroupRuleNotFound): -- 2.45.2