From faa000f2f5572ad4ca40fe84323100ad920cdf43 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Wed, 30 Nov 2016 11:46:40 +0200 Subject: [PATCH] NSX|v+v3 handle provider sgs in create port sg list This patch handles the case of booting a nova instance with a provider security group as a regular security group. Instead of failing the create-port command, which doesn't inform the user of the error, we will remove this provider security group from the port. Since all the tenants provider security groups are added to the port anyway, this action is harmless. In addition, if there are no other security groups for this port, add the default one. Change-Id: I11ec2f9bac6e2e011d66857df74962be07854d34 --- vmware_nsx/db/extended_security_group.py | 65 ++++++++++++++++--- vmware_nsx/plugins/nsx_v/plugin.py | 5 +- vmware_nsx/plugins/nsx_v3/plugin.py | 9 +-- .../test_provider_security_groups.py | 6 +- 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/vmware_nsx/db/extended_security_group.py b/vmware_nsx/db/extended_security_group.py index bb3b346e0b..1338365d59 100644 --- a/vmware_nsx/db/extended_security_group.py +++ b/vmware_nsx/db/extended_security_group.py @@ -13,8 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. -from neutron_lib.db import model_base -from neutron_lib.utils import helpers +from oslo_log import log as logging from oslo_utils import uuidutils import sqlalchemy as sa from sqlalchemy import orm @@ -29,12 +28,18 @@ from neutron.db.models import securitygroup as securitygroups_db # noqa from neutron.extensions import securitygroup as ext_sg from neutron_lib.api import validators from neutron_lib import constants as n_constants +from neutron_lib.db import model_base +from neutron_lib.utils import helpers +from vmware_nsx._i18n import _LW from vmware_nsx.extensions import providersecuritygroup as provider_sg from vmware_nsx.extensions import securitygrouplogging as sg_logging from vmware_nsx.extensions import securitygrouppolicy as sg_policy +LOG = logging.getLogger(__name__) + + class NsxExtendedSecurityGroupProperties(model_base.BASEV2): __tablename__ = 'nsx_extended_security_group_properties' @@ -149,12 +154,28 @@ class ExtendedSecurityGroupPropertiesMixin(object): if not sg[provider_sg.PROVIDER]: raise provider_sg.SecurityGroupNotProvider(id=sg) - def _check_invalid_security_groups_specified(self, context, port): + def _check_invalid_security_groups_specified(self, context, port, + only_warn=False): + """Check if the lists of security groups are valid + + When only_warn is True we do not raise an exception here, because this + may fail nova boot. + Instead we will later remove provider security groups from the regular + security groups list of the port. + Since all the provider security groups of the tenant will be on this + list anyway, the result will be the same. + """ if validators.is_attr_set(port.get(ext_sg.SECURITYGROUPS)): for sg in port.get(ext_sg.SECURITYGROUPS, []): # makes sure user doesn't add non-provider secgrp as secgrp if self._is_provider_security_group(context, sg): - raise provider_sg.SecurityGroupIsProvider(id=sg) + if only_warn: + LOG.warning( + _LW("Ignored provider security group %(sg)s in " + "security groups list for port %(id)s"), + {'sg': sg, 'id': port['id']}) + else: + raise provider_sg.SecurityGroupIsProvider(id=sg) if validators.is_attr_set( port.get(provider_sg.PROVIDER_SECURITYGROUPS)): @@ -193,8 +214,6 @@ class ExtendedSecurityGroupPropertiesMixin(object): if p.get('device_owner') and n_utils.is_port_trusted(p): return - self._check_invalid_security_groups_specified(context, p) - if not validators.is_attr_set(provider_sgs): if provider_sgs is n_constants.ATTR_NOT_SPECIFIED: provider_sgs = self._get_tenant_provider_security_groups( @@ -205,6 +224,36 @@ class ExtendedSecurityGroupPropertiesMixin(object): provider_sgs = [] return provider_sgs + def _get_port_security_groups_lists(self, context, port): + """Return 2 lists of this port security groups: + + 1) Regular security groups for this port + 2) Provider security groups for this port + """ + port_data = port['port'] + # First check that the configuration is valid + self._check_invalid_security_groups_specified( + context, port_data, only_warn=True) + + # get the 2 separate lists of security groups + sgids = self._get_security_groups_on_port( + context, port) or [] + psgids = self._get_provider_security_groups_on_port( + context, port) or [] + had_sgs = len(sgids) > 0 + + # remove provider security groups which were specified also in the + # regular sg list + sgids = list(set(sgids) - set(psgids)) + if not len(sgids) and had_sgs: + # Add the default sg of the tenant if no other remained + tenant_id = port_data.get('tenant_id') + default_sg = self._ensure_default_security_group( + context, tenant_id) + sgids.append(default_sg) + + return (sgids, psgids) + def _process_port_create_provider_security_group(self, context, p, security_group_ids): if validators.is_attr_set(security_group_ids): @@ -227,6 +276,8 @@ class ExtendedSecurityGroupPropertiesMixin(object): sg_changed = ( set(original_port[ext_sg.SECURITYGROUPS]) != set(updated_port[ext_sg.SECURITYGROUPS])) + if sg_changed or provider_sg_changed: + self._check_invalid_security_groups_specified(context, p) if provider_sg_changed: port['port']['tenant_id'] = original_port['id'] @@ -234,8 +285,6 @@ class ExtendedSecurityGroupPropertiesMixin(object): updated_port[provider_sg.PROVIDER_SECURITYGROUPS] = ( self._get_provider_security_groups_on_port(context, port)) else: - if sg_changed: - self._check_invalid_security_groups_specified(context, p) updated_port[provider_sg.PROVIDER_SECURITYGROUPS] = ( original_port[provider_sg.PROVIDER_SECURITYGROUPS]) diff --git a/vmware_nsx/plugins/nsx_v/plugin.py b/vmware_nsx/plugins/nsx_v/plugin.py index 555b22a205..4db72a8c1a 100644 --- a/vmware_nsx/plugins/nsx_v/plugin.py +++ b/vmware_nsx/plugins/nsx_v/plugin.py @@ -1365,8 +1365,9 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, else: port_data[provider_sg.PROVIDER_SECURITYGROUPS] = [] - sgids = self._get_security_groups_on_port(context, port) - ssgids = self._get_provider_security_groups_on_port(context, port) + (sgids, ssgids) = self._get_port_security_groups_lists( + context, port) + self._process_port_create_security_group(context, port_data, sgids) self._process_port_create_provider_security_group(context, port_data, diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py index 0a05ae0232..cacdf3a8e9 100644 --- a/vmware_nsx/plugins/nsx_v3/plugin.py +++ b/vmware_nsx/plugins/nsx_v3/plugin.py @@ -1873,19 +1873,16 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, context, port_data, dhcp_opts) # handle adding security groups to port - sgids = self._get_security_groups_on_port(context, port) + (sgids, provider_groups) = self._get_port_security_groups_lists( + context, port) self._process_port_create_security_group( context, port_data, sgids) - - # handling adding provider security group to port if there are any - provider_groups = self._get_provider_security_groups_on_port( - context, port) self._process_port_create_provider_security_group( context, port_data, provider_groups) # add provider groups to other security groups list. # sgids is a set() so we need to | it in. if provider_groups: - sgids |= set(provider_groups) + sgids = list(set(sgids) | set(provider_groups)) self._extend_port_dict_binding(context, port_data) if validators.is_attr_set(port_data.get(mac_ext.MAC_LEARNING)): if is_psec_on: diff --git a/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py b/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py index 73f019e5c1..830efe6846 100644 --- a/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py +++ b/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py @@ -67,7 +67,9 @@ class ProviderSecurityGroupTestPlugin( with context.session.begin(subtransactions=True): self._ensure_default_security_group_on_port(context, port) - sgids = self._get_security_groups_on_port(context, port) + (sgids, provider_groups) = self._get_port_security_groups_lists( + context, port) + port_db = super(ProviderSecurityGroupTestPlugin, self).create_port( context, port) port_data.update(port_db) @@ -77,8 +79,6 @@ class ProviderSecurityGroupTestPlugin( context, port_db, sgids) # handling adding provider security group to port if there are any - provider_groups = self._get_provider_security_groups_on_port( - context, port) self._process_port_create_provider_security_group( context, port_data, provider_groups) return port_data