From 358c2edb53c9bfc8ad1d91d74f3a16a1a07fc502 Mon Sep 17 00:00:00 2001 From: Nguyen Phuong An Date: Mon, 22 Jan 2018 13:50:55 +0700 Subject: [PATCH] Validating if a port is supported by FWaaS L2 driver Currently, FWaaS L2 driver based OVS only works correctly with VM ports, which are landed at compute nodes with: * mechanism_drivers=openvswitch * firewall_driver=noop or openvswitch for security group If you try to add a VM port to a FWG, which is landed at compute nodes with: * mechanism_drivers=linuxbridge and firewall_driver=iptables * mechanism_drivers=openvswitch and firewall_driver=iptables_hybrid Then, FWaaS V2 API won't work correctly. So this patch validates if VM ports are supported fully by FWaaS L2 driver at this moment. In the future, if FWaaS L2 driver can support not only hybrid port but also other ports, we can remove this validation. Change-Id: Ib0a85b55840d8dfe6bcae91484a0440902d3c49a Closes-Bug: #1746855 --- neutron_fwaas/common/exceptions.py | 24 +++++++++++++ .../services/firewall/fwaas_plugin_v2.py | 36 +++++++++++++++++-- .../services/firewall/test_fwaas_plugin_v2.py | 2 ++ ...if_port_is_supported-639d0df705eb67f9.yaml | 8 +++++ 4 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 neutron_fwaas/common/exceptions.py create mode 100644 releasenotes/notes/validation_if_port_is_supported-639d0df705eb67f9.yaml diff --git a/neutron_fwaas/common/exceptions.py b/neutron_fwaas/common/exceptions.py new file mode 100644 index 000000000..9ecfe0903 --- /dev/null +++ b/neutron_fwaas/common/exceptions.py @@ -0,0 +1,24 @@ +# Copyright 2018 Fujitsu Limited. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron_lib import exceptions as n_exc + +from neutron_fwaas._i18n import _ + + +# TODO(annp): migrate to neutron-lib after Queen release +class FirewallGroupPortNotSupported(n_exc.Conflict): + message = _("Port %(port_id)s is not supported by firewall L2 driver. " + "This may happen due to incompatible driver combination.") diff --git a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py index 6f19e2e5d..415c6a707 100644 --- a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py +++ b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py @@ -29,6 +29,7 @@ from oslo_config import cfg from oslo_log import log as logging import oslo_messaging +from neutron_fwaas.common import exceptions from neutron_fwaas.common import fwaas_constants from neutron_fwaas.db.firewall.v2 import firewall_db_v2 @@ -247,6 +248,34 @@ class FirewallPluginV2( and not device_owner.startswith( nl_constants.DEVICE_OWNER_COMPUTE_PREFIX)): raise f_exc.FirewallGroupPortInvalid(port_id=port_id) + if (device_owner.startswith( + nl_constants.DEVICE_OWNER_COMPUTE_PREFIX) and not + self._is_supported_by_fw_l2_driver(context, port_id)): + raise exceptions.FirewallGroupPortNotSupported(port_id=port_id) + + def _is_supported_by_fw_l2_driver(self, context, port_id): + """Whether this port is supported by firewall l2 driver""" + + # Re-fetch to get up-to-date data from db + port = self._core_plugin.get_port(context, id=port_id) + + # Skip port binding is unbound or failed + if port[pb_def.VIF_TYPE] in [pb_def.VIF_TYPE_UNBOUND, + pb_def.VIF_TYPE_BINDING_FAILED]: + return False + + if not port['port_security_enabled']: + return True + + if port[pb_def.VIF_TYPE] == pb_def.VIF_TYPE_OVS: + # TODO(annp): remove these lines after we fully support for hybrid + # port + if not port[pb_def.VIF_DETAILS][pb_def.OVS_HYBRID_PLUG]: + return True + LOG.warning("Doesn't support hybrid port at the moment") + else: + LOG.warning("Doesn't support vif type %s", port[pb_def.VIF_TYPE]) + return False def _check_no_need_pending(self, context, fwg_id, fwg_body): fwg_db = self._get_firewall_group(context, fwg_id) @@ -304,11 +333,12 @@ class FirewallPluginV2( # only attach security group to vm port. return - if updated_port[pb_def.VIF_TYPE] in [pb_def.VIF_TYPE_UNBOUND, - pb_def.VIF_TYPE_BINDING_FAILED]: - return context = kwargs['context'] port_id = updated_port['id'] + # Check port is supported by firewall l2 driver or not + if not self._is_supported_by_fw_l2_driver(context, port_id): + return + project_id = updated_port['project_id'] LOG.debug("Try to associate port %s at %s", port_id, project_id) self.set_port_for_default_firewall_group(context, port_id, project_id) diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py index a00e31434..3431dda98 100644 --- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py +++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py @@ -233,6 +233,8 @@ class TestFirewallPluginBasev2(TestFirewallRouterPortBase, def setUp(self): super(TestFirewallPluginBasev2, self).setUp(fw_plugin=FW_PLUGIN_KLASS) fake_notifier.reset() + mock.patch.object(self.plugin, '_is_supported_by_fw_l2_driver', + return_value=True).start() @property def _self_context(self): diff --git a/releasenotes/notes/validation_if_port_is_supported-639d0df705eb67f9.yaml b/releasenotes/notes/validation_if_port_is_supported-639d0df705eb67f9.yaml new file mode 100644 index 000000000..4a24db60a --- /dev/null +++ b/releasenotes/notes/validation_if_port_is_supported-639d0df705eb67f9.yaml @@ -0,0 +1,8 @@ +--- +prelude: > + Validating if a port is supported by FWaaS V2 +fixes: + - | + [`bug 1746855 `__] + Now, FWaaS V2 will validate if a port is supported before adding it + to a FWG. This helps to make sure FWaaS V2 API works as expected. \ No newline at end of file