diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 5c19281d364..8c637f5e13c 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -557,6 +557,7 @@ def __init__(self, integration_bridge): self._initialize_sg() self._update_cookie = None self._deferred = False + self._ports_pending_invalid_ct_cleanup = [] self.iptables_helper = iptables.Helper(self.int_br.br) self.iptables_helper.load_driver_if_needed() self.ipconntrack = ip_conntrack.OvsIpConntrackManager() @@ -702,11 +703,22 @@ def _get_port_physical_network(self, port_name): return get_physical_network_from_other_config( self.int_br.br, port_name) - def _delete_invalid_conntrack_entries_for_port(self, port, of_port): - port['of_port'] = of_port + def _delete_invalid_conntrack_entries_for_port(self, ports): for ethertype in [lib_const.IPv4, lib_const.IPv6]: self.ipconntrack.delete_conntrack_state_by_remote_ips( - [port], ethertype, set(), mark=ovsfw_consts.CT_MARK_INVALID) + ports, ethertype, set(), mark=ovsfw_consts.CT_MARK_INVALID) + + def _schedule_invalid_conntrack_entries_cleanup(self, port): + if self._deferred: + self._ports_pending_invalid_ct_cleanup.append(port) + else: + self._delete_invalid_conntrack_entries_for_port([port]) + + def _flush_pending_invalid_conntrack_cleanup(self): + self._delete_invalid_conntrack_entries_for_port( + self._ports_pending_invalid_ct_cleanup + ) + self._ports_pending_invalid_ct_cleanup = [] def get_ofport(self, port): port_id = port['device'] @@ -768,7 +780,8 @@ def prepare_port_filter(self, port): self._update_flows_for_port(of_port, old_of_port) else: self._set_port_filters(of_port) - self._delete_invalid_conntrack_entries_for_port(port, of_port) + port['of_port'] = of_port + self._schedule_invalid_conntrack_entries_cleanup(port) except exceptions.OVSFWPortNotFound as not_found_error: LOG.info("port %(port_id)s does not exist in ovsdb: %(err)s.", {'port_id': port['device'], @@ -808,7 +821,8 @@ def update_port_filter(self, port): else: self._set_port_filters(of_port) - self._delete_invalid_conntrack_entries_for_port(port, of_port) + port['of_port'] = of_port + self._schedule_invalid_conntrack_entries_cleanup(port) except exceptions.OVSFWPortNotFound as not_found_error: LOG.info("port %(port_id)s does not exist in ovsdb: %(err)s.", @@ -897,11 +911,13 @@ def remove_trusted_ports(self, port_ids): def filter_defer_apply_on(self): self._deferred = True + self._ports_pending_invalid_ct_cleanup = [] def filter_defer_apply_off(self): if self._deferred: self._cleanup_stale_sg() self.int_br.apply_flows() + self._flush_pending_invalid_conntrack_cleanup() self._deferred = False @property diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 8619deeaf1b..fc0c9adcac4 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -629,6 +629,11 @@ def _get_port_ip4_ips_and_ip6_flag(self, port): LOG.warning("Port %s MAC column is empty, cannot retrieve IP " "addresses", port.uuid) return [], False + if port.mac == [ovn_const.UNKNOWN_ADDR]: + LOG.warning("Port %s MAC column value is %s, this port will " + "not have metadata service", port.uuid, port.mac) + return [], False + mac, ips = ovn_utils.get_mac_and_ips_from_port_binding(port) if not ips: LOG.debug("Port %s IP addresses were not retrieved from the " diff --git a/neutron/conf/policies/base.py b/neutron/conf/policies/base.py index 9573eb9bfb2..63765c80154 100644 --- a/neutron/conf/policies/base.py +++ b/neutron/conf/policies/base.py @@ -76,6 +76,7 @@ # related to the "network owner" and network isn't really parent of the subnet # or port. Because of that, using parent owner in those cases may be # missleading for users so it's better to keep also "network owner" rules. +NET_OWNER_MANAGER = 'role:manager and ' + RULE_NET_OWNER NET_OWNER_MEMBER = 'role:member and ' + RULE_NET_OWNER NET_OWNER_READER = 'role:reader and ' + RULE_NET_OWNER ADMIN_OR_NET_OWNER_MEMBER = ( diff --git a/neutron/conf/policies/port.py b/neutron/conf/policies/port.py index 2c2e7cb181a..daeda35341c 100644 --- a/neutron/conf/policies/port.py +++ b/neutron/conf/policies/port.py @@ -100,7 +100,6 @@ check_str=neutron_policy.policy_or( 'not rule:network_device', base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -119,7 +118,6 @@ name='create_port:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER), scope_types=['project'], description='Specify ``mac_address`` attribute when creating a port', @@ -136,7 +134,6 @@ name='create_port:fixed_ips', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, 'rule:shared'), scope_types=['project'], @@ -155,7 +152,6 @@ name='create_port:fixed_ips:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER), scope_types=['project'], description='Specify IP address in ``fixed_ips`` when creating a port', @@ -172,7 +168,6 @@ name='create_port:fixed_ips:subnet_id', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, 'rule:shared'), scope_types=['project'], @@ -191,7 +186,6 @@ name='create_port:port_security_enabled', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER), scope_types=['project'], description=( @@ -258,7 +252,6 @@ name='create_port:allowed_address_pairs', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -276,7 +269,6 @@ name='create_port:allowed_address_pairs:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -294,7 +286,6 @@ name='create_port:allowed_address_pairs:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -496,7 +487,6 @@ check_str=neutron_policy.policy_or( 'not rule:network_device', base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, ), scope_types=['project'], @@ -515,7 +505,7 @@ name='update_port:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER + base.NET_OWNER_MANAGER, ), scope_types=['project'], description='Update ``mac_address`` attribute of a port', @@ -532,7 +522,6 @@ name='update_port:fixed_ips', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -550,7 +539,6 @@ name='update_port:fixed_ips:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -571,7 +559,6 @@ name='update_port:fixed_ips:subnet_id', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, 'rule:shared' ), @@ -594,7 +581,6 @@ name='update_port:port_security_enabled', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -653,7 +639,6 @@ name='update_port:allowed_address_pairs', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description='Update ``allowed_address_pairs`` attribute of a port', @@ -668,7 +653,6 @@ name='update_port:allowed_address_pairs:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -686,7 +670,6 @@ name='update_port:allowed_address_pairs:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( diff --git a/neutron/conf/policies/router.py b/neutron/conf/policies/router.py index 52cdd8d13ea..419d7e6f8a9 100644 --- a/neutron/conf/policies/router.py +++ b/neutron/conf/policies/router.py @@ -315,7 +315,7 @@ scope_types=['project'], description=('Specify ``enable_default_route_bfd`` attribute when ' 'updating a router'), - operations=ACTION_POST, + operations=ACTION_PUT, ), policy.DocumentedRuleDefault( name='update_router:enable_default_route_ecmp', @@ -323,7 +323,7 @@ scope_types=['project'], description=('Specify ``enable_default_route_ecmp`` attribute when ' 'updating a router'), - operations=ACTION_POST, + operations=ACTION_PUT, ), policy.DocumentedRuleDefault( name='update_router:tags', diff --git a/neutron/db/quota/driver.py b/neutron/db/quota/driver.py index 694a40735fa..fc6dffc52c4 100644 --- a/neutron/db/quota/driver.py +++ b/neutron/db/quota/driver.py @@ -97,7 +97,12 @@ def get_detailed_project_quotas(self, context, resources, project_id): # pass it regardless to keep the quota driver API intact plugins = directory.get_plugins() plugin = plugins.get(key, plugins[constants.CORE]) - used = resource.count(context, plugin, project_id) + try: + used = resource.count(context, plugin, project_id) + except NotImplementedError: + LOG.info('Skipping quota resource %s: no plugin loaded ' + 'that supports counting it.', key) + continue project_quota_ext[key] = { 'limit': resource.default, @@ -108,7 +113,8 @@ def get_detailed_project_quotas(self, context, resources, project_id): quota_objs = quota_obj.Quota.get_objects(context, project_id=project_id) for item in quota_objs: - project_quota_ext[item['resource']]['limit'] = item['limit'] + if item['resource'] in project_quota_ext: + project_quota_ext[item['resource']]['limit'] = item['limit'] return project_quota_ext @staticmethod diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 53f73f080b3..9199a11352d 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -517,8 +517,10 @@ def _restore_local_vlan_map(self): self.available_local_vlans.remove(local_vlan) # Restore the br-tun flood output ports # See LP #1978088 - tun_ofports = self.tun_br.get_flood_to_tun_ofports( - local_vlan) + tun_ofports = set() + if self.enable_tunneling: + tun_ofports = self.tun_br.get_flood_to_tun_ofports( + local_vlan) self._local_vlan_hints[key] = { 'vlan': local_vlan, 'tun_ofports': tun_ofports} @@ -1489,7 +1491,15 @@ def port_unbound(self, vif_id, net_uuid=None): if not lvm.vif_ports: self.reclaim_local_vlan(net_uuid, lvm.segmentation_id) + @staticmethod + def _has_valid_ofport(port): + return port.ofport and port.ofport != ovs_lib.INVALID_OFPORT + def port_alive(self, port, log_errors=True): + if not self._has_valid_ofport(port): + LOG.warning("port_alive skipped for port %s: invalid ofport %s", + port.port_name, port.ofport) + return cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag", log_errors=log_errors) # Port normal vlan tag is set correctly, remove the drop flows @@ -1505,6 +1515,10 @@ def port_dead(self, port, log_errors=True): :param port: an ovs_lib.VifPort object. ''' + if not self._has_valid_ofport(port): + LOG.warning("port_dead skipped for port %s: invalid ofport %s", + port.port_name, port.ofport) + return # Don't kill a port if it's already dead cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag", log_errors=log_errors) @@ -1856,9 +1870,11 @@ def _process_port(port, ports, ancillary_ports): iface_id = self.int_br.portid_from_external_ids( port['external_ids']) if iface_id: - if port['ofport'] == ovs_lib.UNASSIGNED_OFPORT: - LOG.debug("Port %s not ready yet on the bridge", - iface_id) + if port['ofport'] in (ovs_lib.UNASSIGNED_OFPORT, + ovs_lib.INVALID_OFPORT): + LOG.debug("Port %s not ready yet on the bridge " + "(ofport=%s)", + iface_id, port['ofport']) ports_not_ready_yet.add(port['name']) return # check if port belongs to ancillary bridge @@ -1964,19 +1980,12 @@ def treat_vif_port(self, vif_port, port_id, network_id, network_type, physical_network, segmentation_id, admin_state_up, fixed_ips, device_owner, provisioning_needed): port_needs_binding = True - if not vif_port.ofport: - # Log an error if the VIF port has no ofport, which indicates - # that the port might not be able to transmit traffic. - LOG.error("VIF port: %s has no ofport and might not " - "be able to transmit.", vif_port.vif_id) - elif vif_port.ofport == ovs_lib.INVALID_OFPORT: - # When the ofport is set to INVALID_OFPORT, it indicates that - # the port is in a transitional state and has not yet been fully - # configured. - LOG.info("VIF port: %s is in a transitional state and has not " - "yet been assigned a valid ofport. This is expected " - "during port initialization. (ofport=%s)", - vif_port.vif_id, vif_port.ofport) + if not self._has_valid_ofport(vif_port): + LOG.warning("VIF port: %s has no valid ofport (ofport=%s), " + "skipping OF operations; the port will be " + "retried on the next iteration.", + vif_port.vif_id, vif_port.ofport) + return False if admin_state_up: port_needs_binding = self.port_bound( vif_port, network_id, network_type, diff --git a/neutron/plugins/ml2/extensions/dns_integration.py b/neutron/plugins/ml2/extensions/dns_integration.py index fab682a0a6f..ba2e28ca67d 100644 --- a/neutron/plugins/ml2/extensions/dns_integration.py +++ b/neutron/plugins/ml2/extensions/dns_integration.py @@ -23,7 +23,6 @@ from neutron_lib.exceptions import dns as dns_exc from neutron_lib.plugins import directory from neutron_lib.plugins.ml2 import api -from neutron_lib.plugins import utils as plugin_utils from oslo_config import cfg from oslo_log import log as logging @@ -349,35 +348,56 @@ def _get_details(self, context, network_id): class DNSExtensionDriverML2(DNSExtensionDriver): + def __init__(self): + super().__init__() + self._vlan_driver = None + self._tunnel_drivers = {} + self._plugin = None + def initialize(self): LOG.info("DNSExtensionDriverML2 initialization complete") - def _is_tunnel_project_network(self, provider_net): - if provider_net['network_type'] == lib_const.TYPE_GENEVE: - tunnel_ranges = cfg.CONF.ml2_type_geneve.vni_ranges - elif provider_net['network_type'] == lib_const.TYPE_VXLAN: - tunnel_ranges = cfg.CONF.ml2_type_vxlan.vni_ranges - else: - tunnel_ranges = cfg.CONF.ml2_type_gre.tunnel_id_ranges + @property + def plugin(self): + if not self._plugin: + self._plugin = directory.get_plugin() + return self._plugin + + @property + def vlan_driver(self): + if not self._vlan_driver: + self._vlan_driver = self.plugin.type_manager.drivers.get( + lib_const.TYPE_VLAN) + return self._vlan_driver + + def get_tunnel_driver(self, network_type): + if network_type not in self._tunnel_drivers: + self._tunnel_drivers[network_type] = ( + self.plugin.type_manager.drivers.get(network_type)) + return self._tunnel_drivers[network_type] + def _is_tunnel_project_network(self, provider_net): + network_type = provider_net['network_type'] + tunnel_driver = self.get_tunnel_driver(network_type) + if not tunnel_driver: + return False + tunnel_ranges = tunnel_driver.obj.get_network_segment_ranges() + if not tunnel_ranges: + return False segmentation_id = int(provider_net['segmentation_id']) - for entry in tunnel_ranges: - entry = entry.strip() - tun_min, tun_max = entry.split(':') - tun_min = tun_min.strip() - tun_max = tun_max.strip() - return int(tun_min) <= segmentation_id <= int(tun_max) + return any(tun_min <= segmentation_id <= tun_max + for tun_min, tun_max in tunnel_ranges) def _is_vlan_project_network(self, provider_net): - network_vlan_ranges = plugin_utils.parse_network_vlan_ranges( - cfg.CONF.ml2_type_vlan.network_vlan_ranges) - vlan_ranges = network_vlan_ranges[provider_net['physical_network']] + if not self.vlan_driver: + return False + network_vlan_ranges = self.vlan_driver.obj.get_network_segment_ranges() + vlan_ranges = network_vlan_ranges.get(provider_net['physical_network']) if not vlan_ranges: return False segmentation_id = int(provider_net['segmentation_id']) - for vlan_range in vlan_ranges: - if vlan_range[0] <= segmentation_id <= vlan_range[1]: - return True + return any(vlan_range[0] <= segmentation_id <= vlan_range[1] + for vlan_range in vlan_ranges) def external_dns_not_needed(self, context, network, subnets): dns_driver = _get_dns_driver() diff --git a/neutron/services/conntrack_helper/plugin.py b/neutron/services/conntrack_helper/plugin.py index d05d1b873df..88b71217096 100644 --- a/neutron/services/conntrack_helper/plugin.py +++ b/neutron/services/conntrack_helper/plugin.py @@ -108,9 +108,9 @@ def _find_existing_conntrack_helper(self, context, router_id, if objs: return (objs[0], param) - def _get_conntrack_helper(self, context, id): + def _get_conntrack_helper(self, context, id, router_id): cth_obj = cth.ConntrackHelper.get_object(context, id=id) - if not cth_obj: + if not cth_obj or cth_obj.router_id != router_id: raise cth_exc.ConntrackHelperNotFound(id=id) return cth_obj @@ -153,7 +153,7 @@ def update_router_conntrack_helper(self, context, id, router_id, conntrack_helper = conntrack_helper.get(apidef.RESOURCE_NAME) try: with db_api.CONTEXT_WRITER.using(context): - cth_obj = self._get_conntrack_helper(context, id) + cth_obj = self._get_conntrack_helper(context, id, router_id) cth_obj.update_fields(conntrack_helper, reset_changes=True) self._check_conntrack_helper_constraints(cth_obj) cth_obj.update() @@ -171,7 +171,7 @@ def update_router_conntrack_helper(self, context, id, router_id, @db_base_plugin_common.make_result_with_fields @db_base_plugin_common.convert_result_to_dict def get_router_conntrack_helper(self, context, id, router_id, fields=None): - return self._get_conntrack_helper(context, id) + return self._get_conntrack_helper(context, id, router_id) @db_base_plugin_common.make_result_with_fields @db_base_plugin_common.convert_result_to_dict @@ -185,7 +185,7 @@ def get_router_conntrack_helpers(self, context, router_id=None, router_id=router_id, **filters) def delete_router_conntrack_helper(self, context, id, router_id): - cth_obj = self._get_conntrack_helper(context, id) + cth_obj = self._get_conntrack_helper(context, id, router_id) with db_api.CONTEXT_WRITER.using(context): cth_obj.delete() self.push_api.push(context, [cth_obj], rpc_events.DELETED) diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 74ac89b1f7c..5242489e0bc 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -183,18 +183,20 @@ def subscribe(self): cancellable=True) def _post_fork_initialize(self, resource, event, trigger, payload=None): + # TODO(ralonsoh): once [1] is released and required in Neutron, it + # won't be needed the ``get_method_class`` method. + # [1] https://review.opendev.org/c/openstack/neutron-lib/+/988563 + if utils.get_method_class(trigger) != wsgi.WorkerService: + return + if not self._nb_ovn or not self._sb_ovn: raise ovn_l3_exc.MechanismDriverOVNNotReady() # Register needed events, only for the Neutron API workers. - # TODO(ralonsoh): once [1] is released and required in Neutron, it - # won't be needed the ``get_method_class`` method. - # [1] https://review.opendev.org/c/openstack/neutron-lib/+/988563 - if utils.get_method_class(trigger) == wsgi.WorkerService: - self._nb_ovn.idl.notify_handler.watch_events([ - ovsdb_monitor.LogicalRouterPortEvent(self), - ovsdb_monitor.LogicalRouterPortGatewayChassisEvent(self), - ]) + self._nb_ovn.idl.notify_handler.watch_events([ + ovsdb_monitor.LogicalRouterPortEvent(self), + ovsdb_monitor.LogicalRouterPortGatewayChassisEvent(self), + ]) def _add_neutron_router_interface(self, context, router_id, interface_info): diff --git a/neutron/services/portforwarding/pf_plugin.py b/neutron/services/portforwarding/pf_plugin.py index 16506b43ab5..8519801b0cb 100644 --- a/neutron/services/portforwarding/pf_plugin.py +++ b/neutron/services/portforwarding/pf_plugin.py @@ -416,7 +416,7 @@ def update_floatingip_port_forwarding(self, context, id, floatingip_id, with db_api.CONTEXT_WRITER.using(context): fip_obj = self._get_fip_obj(context, floatingip_id) pf_obj = pf.PortForwarding.get_object(context, id=id) - if not pf_obj: + if not pf_obj or pf_obj.floatingip_id != floatingip_id: raise pf_exc.PortForwardingNotFound(id=id) original_pf_obj = copy.deepcopy(pf_obj) ori_internal_port_id = pf_obj.internal_port_id @@ -670,7 +670,7 @@ def get_floatingip_port_forwarding(self, context, id, floatingip_id, fields=None): self._get_fip_obj(context, floatingip_id) obj = pf.PortForwarding.get_object(context, id=id) - if not obj: + if not obj or obj.floatingip_id != floatingip_id: raise pf_exc.PortForwardingNotFound(id=id) return obj diff --git a/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py b/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py index 8e5345e919e..799e58bd5d5 100644 --- a/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py +++ b/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py @@ -127,3 +127,30 @@ def test_negative_delete_conntrack_helper(self): self.assertRaises(cth_exc.ConntrackHelperNotFound, self.cth_plugin.delete_router_conntrack_helper, self.context, INVALID_ID, self.router['id']) + + def test_negative_singleton_operations_wrong_router(self): + res = self.cth_plugin.create_router_conntrack_helper( + self.context, self.router['id'], self.conntack_helper) + other_router = self._create_router(distributed=True) + new_conntack_helper = { + apidef.RESOURCE_NAME: + {apidef.PROTOCOL: 'udp', + apidef.PORT: 6969, + apidef.HELPER: 'tftp'} + } + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.get_router_conntrack_helper, + self.context, res['id'], other_router['id']) + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.update_router_conntrack_helper, + self.context, res['id'], other_router['id'], + new_conntack_helper) + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.delete_router_conntrack_helper, + self.context, res['id'], other_router['id']) + helper = self.cth_plugin.get_router_conntrack_helper( + self.context, res['id'], self.router['id']) + self.assertEqual(res['id'], helper['id']) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 91dc8e59406..27a1e3133d0 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -1032,15 +1032,23 @@ def test_update_port_filter_port_security_disabled(self): self.assertTrue(self.mock_bridge.br.delete_flows.called) self.delete_invalid_conntrack_entries_mock.assert_not_called() - def test_update_port_filter_applies_added_flows(self): - """Check flows are applied right after _set_flows is called.""" + def test_update_port_filter_applies_added_flows_and_clean_conntrack(self): + """Check flows are applied right after _set_flows is called. + + Additionally this test checks also if conntrack entries marked as + CT_MARK_INVALID are cleaned once after new flows are set. + """ + port_dict = {'device': 'port-id', 'security_groups': [1]} self._prepare_security_group() self.firewall.prepare_port_filter(port_dict) + self.delete_invalid_conntrack_entries_mock.reset_mock() with self.firewall.defer_apply(): self.firewall.update_port_filter(port_dict) + self.delete_invalid_conntrack_entries_mock.assert_not_called() self.mock_bridge.apply_flows.assert_called_once() + self._assert_invalid_conntrack_entries_deleted(port_dict) def test_update_port_filter_clean_when_port_not_found(self): """Check flows are cleaned if port is not found in the bridge.""" diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 839c3d7164f..d0613989108 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -483,6 +483,36 @@ def test__get_provision_params_returns_provision_parameters_ipv6(self): self._test__get_provision_params_returns_provision_parameters( 'fe80::f816:3eff:feb6:c0c0') + def test__get_provision_params_skips_port_with_unknown_mac(self): + """Should skip ports with mac=['unknown'] and return valid ports.""" + network_id = '1' + datapath = DatapathInfo(uuid='test123', + external_ids={'name': f'neutron-{network_id}'}) + valid_port = makePort(datapath, + mac=['fa:16:3e:e7:ac:ab 1.2.3.4']) + unknown_mac_port = mock.Mock() + unknown_mac_port.type = '' + unknown_mac_port.mac = [ovn_const.UNKNOWN_ADDR] + unknown_mac_port.datapath = datapath + unknown_mac_port.uuid = 'fake-uuid' + metadadata_port = makePort( + datapath, + mac=['fa:16:3e:22:65:18 10.204.0.1'], + external_ids={'neutron:cidrs': '10.204.0.10/29'}, + logical_port='3b66c176-199b-48ec-8331-c1fd3f6e2b44') + + with mock.patch.object(self.agent.sb_idl, 'get_metadata_port', + return_value=metadadata_port),\ + mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', + return_value=[valid_port, unknown_mac_port]): + actual_params = self.agent._get_provision_params(datapath) + + net_name, datapath_port_ips, any_ip6, metadata_port_info = ( + actual_params) + self.assertEqual(network_id, net_name) + self.assertListEqual(['1.2.3.4'], datapath_port_ips) + self.assertFalse(any_ip6) + def test__get_port_ip4_ips_and_ip6_flag_empty_mac(self): """Should return empty IPs for ports with empty MAC column.""" port = mock.Mock() @@ -492,6 +522,15 @@ def test__get_port_ip4_ips_and_ip6_flag_empty_mac(self): self.assertEqual([], ip4_ips) self.assertFalse(any_ip6) + def test__get_port_ip4_ips_and_ip6_flag_unknown_mac(self): + """Should return empty IPs for ports with mac=['unknown'].""" + port = mock.Mock() + port.mac = [ovn_const.UNKNOWN_ADDR] + port.uuid = 'fake-uuid' + ip4_ips, any_ip6 = self.agent._get_port_ip4_ips_and_ip6_flag(port) + self.assertEqual([], ip4_ips) + self.assertFalse(any_ip6) + def _test_provision_datapath(self, ipv6_enabled): """Test datapath provisioning. diff --git a/neutron/tests/unit/conf/policies/test_port.py b/neutron/tests/unit/conf/policies/test_port.py index f078c405da2..9c27736f9d9 100644 --- a/neutron/tests/unit/conf/policies/test_port.py +++ b/neutron/tests/unit/conf/policies/test_port.py @@ -41,6 +41,12 @@ def setUp(self): 'project_id': self.alt_project_id, 'network_id': self.alt_network['id'], 'ext_parent_network_id': self.alt_network['id']} + # This port belongs to "project_id", but the network belongs to + # "alt_project_id". + self.target_net_alt_target = { + 'project_id': self.project_id, + 'network_id': self.alt_network['id'], + 'ext_parent_network_id': self.alt_network['id']} networks = { self.network['id']: self.network, @@ -56,6 +62,33 @@ def get_network(context, id, fields=None): 'neutron_lib.plugins.directory.get_plugin', return_value=self.plugin_mock).start() + def _assert_network_owner_policy(self, action, target=None, + alt_target=None, + target_net_alt_target=None): + target = target if target is not None else self.target + alt_target = alt_target if alt_target is not None else self.alt_target + if target_net_alt_target is None: + target_net_alt_target = self.target_net_alt_target + self.assertTrue(policy.enforce(self.context, action, target)) + self.assertRaises( + base_policy.PolicyNotAuthorized, + policy.enforce, self.context, action, alt_target) + self.assertRaises( + base_policy.PolicyNotAuthorized, + policy.enforce, self.context, action, target_net_alt_target) + + def _assert_denied_network_owner_policy(self, action, target=None, + alt_target=None, + target_net_alt_target=None): + target = target if target is not None else self.target + alt_target = alt_target if alt_target is not None else self.alt_target + if target_net_alt_target is None: + target_net_alt_target = self.target_net_alt_target + for test_target in (target, alt_target, target_net_alt_target): + self.assertRaises( + base_policy.PolicyNotAuthorized, + policy.enforce, self.context, action, test_target) + class SystemAdminTests(PortAPITestCase): @@ -868,57 +901,26 @@ def test_create_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'create_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_network_owner_policy( + 'create_port:device_owner', target, alt_target, + target_net_alt_target) def test_create_port_with_mac_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:mac_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.alt_target) + self._assert_network_owner_policy('create_port:mac_address') def test_create_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.alt_target) + self._assert_network_owner_policy('create_port:fixed_ips') def test_create_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:fixed_ips:ip_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.alt_target) + self._assert_network_owner_policy('create_port:fixed_ips:ip_address') def test_create_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:fixed_ips:subnet_id', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_network_owner_policy('create_port:fixed_ips:subnet_id') def test_create_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:port_security_enabled', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.alt_target) + self._assert_network_owner_policy('create_port:port_security_enabled') def test_create_port_with_binding_host_id(self): self.assertRaises( @@ -950,36 +952,15 @@ def test_create_port_with_binding_vnic_type(self): self.alt_target) def test_create_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:allowed_address_pairs', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.alt_target) + self._assert_network_owner_policy('create_port:allowed_address_pairs') def test_create_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_network_owner_policy( + 'create_port:allowed_address_pairs:mac_address') def test_create_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_network_owner_policy( + 'create_port:allowed_address_pairs:ip_address') def test_create_port_with_hints(self): self.assertRaises( @@ -1116,57 +1097,26 @@ def test_update_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'update_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_network_owner_policy( + 'update_port:device_owner', target, alt_target, + target_net_alt_target) def test_update_port_with_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:mac_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:mac_address', - self.alt_target) + self._assert_network_owner_policy('update_port:mac_address') def test_update_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips', - self.alt_target) + self._assert_network_owner_policy('update_port:fixed_ips') def test_update_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:fixed_ips:ip_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:ip_address', - self.alt_target) + self._assert_network_owner_policy('update_port:fixed_ips:ip_address') def test_update_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:fixed_ips:subnet_id', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_network_owner_policy('update_port:fixed_ips:subnet_id') def test_update_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:port_security_enabled', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.alt_target) + self._assert_network_owner_policy('update_port:port_security_enabled') def test_update_port_with_binding_host_id(self): self.assertRaises( @@ -1198,36 +1148,15 @@ def test_update_port_with_binding_vnic_type(self): self.alt_target) def test_update_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:allowed_address_pairs', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.alt_target) + self._assert_network_owner_policy('update_port:allowed_address_pairs') def test_update_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_network_owner_policy( + 'update_port:allowed_address_pairs:mac_address') def test_update_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_network_owner_policy( + 'update_port:allowed_address_pairs:ip_address') def test_update_port_data_plane_status(self): self.assertRaises( @@ -1280,184 +1209,8 @@ def setUp(self): super().setUp() self.context = self.project_member_ctx - def test_create_port_with_device_owner(self): - target = self.target.copy() - target['device_owner'] = 'network:test' - alt_target = self.alt_target.copy() - alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'create_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - alt_target) - - def test_create_port_with_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:mac_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.alt_target) - - def test_create_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, 'create_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.alt_target) - - def test_create_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:fixed_ips:ip_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.alt_target) - - def test_create_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:fixed_ips:subnet_id', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.alt_target) - - def test_create_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:port_security_enabled', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.alt_target) - - def test_create_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:allowed_address_pairs', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.alt_target) - - def test_create_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.alt_target) - - def test_create_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.alt_target) - - def test_update_port_with_device_owner(self): - target = self.target.copy() - target['device_owner'] = 'network:test' - alt_target = self.alt_target.copy() - alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'update_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - alt_target) - def test_update_port_with_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:mac_address', - self.alt_target) - - def test_update_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips', - self.alt_target) - - def test_update_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:ip_address', - self.alt_target) - - def test_update_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce(self.context, 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:subnet_id', - self.alt_target) - - def test_update_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:port_security_enabled', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.alt_target) - - def test_update_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:allowed_address_pairs', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.alt_target) - - def test_update_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.alt_target) - - def test_update_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy('update_port:mac_address') class ProjectReaderTests(ProjectMemberTests): @@ -1479,24 +1232,14 @@ def test_create_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_denied_network_owner_policy( + 'create_port:device_owner', target, alt_target, + target_net_alt_target) def test_create_port_with_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.alt_target) + self._assert_denied_network_owner_policy('create_port:mac_address') def test_create_port_with_device_id(self): self.assertRaises( @@ -1509,80 +1252,31 @@ def test_create_port_with_device_id(self): self.alt_target) def test_create_port_with_fixed_ips(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.alt_target) + self._assert_denied_network_owner_policy('create_port:fixed_ips') def test_create_port_with_fixed_ips_and_subnet_id(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:fixed_ips:subnet_id') def test_create_port_with_port_security_enabled(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:port_security_enabled') def test_create_port_with_allowed_address_pairs(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:allowed_address_pairs') def test_create_port_with_allowed_address_pairs_and_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:allowed_address_pairs:mac_address') def test_create_port_with_allowed_address_pairs_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:allowed_address_pairs:ip_address') def test_create_port_with_fixed_ips_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:fixed_ips:ip_address') def test_create_port_with_binding_vnic_type(self): self.assertRaises( @@ -1615,14 +1309,14 @@ def test_update_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_denied_network_owner_policy( + 'update_port:device_owner', target, alt_target, + target_net_alt_target) + + def test_update_port_with_mac_address(self): + self._assert_denied_network_owner_policy('update_port:mac_address') def test_update_port_with_device_id(self): self.assertRaises( @@ -1635,86 +1329,31 @@ def test_update_port_with_device_id(self): self.alt_target) def test_update_port_with_port_security_enabled(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:port_security_enabled') def test_update_port_with_allowed_address_pairs(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:allowed_address_pairs') def test_update_port_with_allowed_address_pairs_and_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:allowed_address_pairs:mac_address') def test_update_port_with_allowed_address_pairs_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:allowed_address_pairs:ip_address') def test_update_port_with_fixed_ips(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips', - self.alt_target) + self._assert_denied_network_owner_policy('update_port:fixed_ips') def test_update_port_with_fixed_ips_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:fixed_ips:ip_address') def test_update_port_with_fixed_ips_and_subnet_id(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:subnet_id', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:fixed_ips:subnet_id') def test_update_port_with_binding_vnic_type(self): self.assertRaises( diff --git a/neutron/tests/unit/db/quota/test_driver.py b/neutron/tests/unit/db/quota/test_driver.py index d3f0f7c3cc7..362279efb23 100644 --- a/neutron/tests/unit/db/quota/test_driver.py +++ b/neutron/tests/unit/db/quota/test_driver.py @@ -308,6 +308,28 @@ def test_get_detailed_project_quotas_multiple_resource(self): self.assertEqual(7, detailed_quota[self.resource_2]['reserved']) self.assertEqual(3, detailed_quota[self.resource_2]['used']) + def test_get_detailed_project_quotas_skips_uncount_resource(self): + def _raise_not_implemented(context, resource, project_id): + raise NotImplementedError( + 'No plugins that support counting %s found.' % resource) + + resources = { + self.resource_1: + TestTrackedResource(self.resource_1, test_quota.MehModel), + self.resource_2: + TestCountableResource(self.resource_2, + _raise_not_implemented)} + self.plugin.update_quota_limit(self.context, self.project_1, + self.resource_1, 6) + self.plugin.update_quota_limit(self.context, self.project_1, + self.resource_2, 9) + detailed_quota = self.plugin.get_detailed_project_quotas( + self.context, resources, self.project_1) + + self.assertIn(self.resource_1, detailed_quota) + self.assertNotIn(self.resource_2, detailed_quota) + self.assertEqual(6, detailed_quota[self.resource_1]['limit']) + def test_quota_limit_check(self): resources = self._create_resources() self.plugin.update_quota_limit(self.context, self.project_1, diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 3427631c3f6..070a42452d5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -286,7 +286,14 @@ def test_agent_available_local_vlans(self): self.assertNotIn(tag, available_vlan) def _test_restore_local_vlan_maps(self, tag, segmentation_id='1', - tun_ofports=None): + tun_ofports=None, no_tun_br=False): + if no_tun_br: + self.agent.enable_tunneling = False + self.agent.tun_br = None + else: + # _make_agent() leaves enable_tunneling False (tunnel_types=[]); + # enable it so the gated get_flood_to_tun_ofports() runs. + self.agent.enable_tunneling = True tun_ofports = tun_ofports or set() port = mock.Mock() port.port_name = 'fake_port' @@ -318,19 +325,25 @@ def _test_restore_local_vlan_maps(self, tag, segmentation_id='1', 'other_config': local_vlan_map, 'tag': tag}] - with mock.patch.object(self.agent.int_br, - 'get_ports_attributes', - side_effect=[get_interfaces, - get_ports]) as gpa,\ - mock.patch.object(self.agent.tun_br, - 'get_flood_to_tun_ofports') as gftto: - gftto.return_value = tun_ofports + if no_tun_br: + # enable_tunneling=False: no br-tun to mock. + tun_ctx = contextlib.nullcontext() + expected_tun_ofports = set() + else: + tun_ctx = mock.patch.object(self.agent.tun_br, + 'get_flood_to_tun_ofports', + return_value=tun_ofports) + expected_tun_ofports = tun_ofports + with tun_ctx, mock.patch.object(self.agent.int_br, + 'get_ports_attributes', + side_effect=[get_interfaces, + get_ports]) as gpa: self.agent._restore_local_vlan_map() expected_hints = {} if tag: key = f"{net_uuid}/{segmentation_id}" expected_hints[key] = {'vlan': tag, - 'tun_ofports': tun_ofports} + 'tun_ofports': expected_tun_ofports} self.assertEqual(expected_hints, self.agent._local_vlan_hints) # make sure invalid and unassigned ports were skipped gpa.assert_has_calls([ @@ -353,6 +366,11 @@ def test_restore_local_vlan_map_segmentation_id_compat(self): def test_restore_local_vlan_map_tun_ofports(self): self._test_restore_local_vlan_maps(2, tun_ofports={2, 3}) + def test_restore_local_vlan_map_no_tun_br(self): + # Non-tunneling deployment (enable_tunneling=False): tun_br is None, + # so the flood-port restore must be skipped instead of crashing. + self._test_restore_local_vlan_maps(2, no_tun_br=True) + def test_check_agent_configurations_for_dvr_raises(self): self.agent.enable_distributed_routing = True self.agent.enable_tunneling = True @@ -411,6 +429,59 @@ def test_port_dead_with_port_already_dead(self): def test_port_dead_with_valid_tag(self): self._test_port_dead(cur_tag=1) + def test_port_dead_invalid_ofport_unassigned(self): + port = mock.Mock() + port.ofport = ovs_lib.UNASSIGNED_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_dead(port) + int_br.set_db_attribute.assert_not_called() + int_br.drop_port.assert_not_called() + + def test_port_dead_invalid_ofport_negative(self): + port = mock.Mock() + port.ofport = ovs_lib.INVALID_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_dead(port) + int_br.set_db_attribute.assert_not_called() + int_br.drop_port.assert_not_called() + + def test_port_alive_invalid_ofport_unassigned(self): + port = mock.Mock() + port.ofport = ovs_lib.UNASSIGNED_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_alive(port) + int_br.db_get_val.assert_not_called() + int_br.uninstall_flows.assert_not_called() + + def test_port_alive_invalid_ofport_negative(self): + port = mock.Mock() + port.ofport = ovs_lib.INVALID_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_alive(port) + int_br.db_get_val.assert_not_called() + int_br.uninstall_flows.assert_not_called() + + def test_treat_vif_port_invalid_ofport_returns_false(self): + for ofport in (ovs_lib.UNASSIGNED_OFPORT, ovs_lib.INVALID_OFPORT, 0): + vif_port = mock.Mock() + vif_port.ofport = ofport + vif_port.vif_id = 'test-port-id' + with mock.patch.object( + self.agent, 'port_bound' + ) as port_bound, mock.patch.object( + self.agent, 'port_alive' + ) as port_alive: + result = self.agent.treat_vif_port( + vif_port, 'port-id', 'net-id', 'vxlan', + None, 100, True, [], 'compute:nova', False) + self.assertFalse(result) + port_bound.assert_not_called() + port_alive.assert_not_called() + def mock_scan_ports(self, vif_port_set=None, registered_ports=None, updated_ports=None, port_tags_dict=None, sync=False): if port_tags_dict is None: # Because empty dicts evaluate as False. @@ -1152,7 +1223,7 @@ def test_treat_vif_port_shut_down_port(self): "iface-id": "407a79e0-e0be-4b7d-92a6-513b2161011b", "vif_mac": "fa:16:3e:68:46:7b", "port_name": "qr-407a79e0-e0", - "ofport": -1, + "ofport": 10, "bridge_name": "br-int"}) with mock.patch.object( self.agent.plugin_rpc, 'update_device_down' diff --git a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py index c7c349fab56..d9c1a1ea594 100644 --- a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py +++ b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py @@ -760,6 +760,191 @@ def test_update_fixed_ips_no_effect_after_clearing_dns_domain(self, dns_data_db, dns_data_db_1, dns_data_db_2) +class DNSExtensionDriverML2TestCase(testtools.TestCase): + + def setUp(self): + super().setUp() + self.driver = dns_integration.DNSExtensionDriverML2() + self.mock_vlan_driver = mock.Mock() + self.mock_vlan_driver_property = mock.patch.object( + type(self.driver), 'vlan_driver', + new_callable=mock.PropertyMock, + return_value=self.mock_vlan_driver).start() + + def test__is_vlan_project_network_with_multiple_ranges(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = { + 'physnet1': [(100, 200), (300, 400)], + 'physnet2': [(500, 600)] + } + + provider_net_1 = { + 'physical_network': 'physnet1', + 'segmentation_id': 150 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_1)) + + provider_net_2 = { + 'physical_network': 'physnet1', + 'segmentation_id': 250 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_2)) + + provider_net_3 = { + 'physical_network': 'physnet1', + 'segmentation_id': 350 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_3)) + + provider_net_4 = { + 'physical_network': 'physnet2', + 'segmentation_id': 550 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_4)) + + provider_net_5 = { + 'physical_network': 'physnet2', + 'segmentation_id': 650 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_5)) + + def test__is_vlan_project_network_boundary_values(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = { + 'physnet1': [(100, 200)] + } + + provider_net_1 = { + 'physical_network': 'physnet1', + 'segmentation_id': 100 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_1)) + + provider_net_2 = { + 'physical_network': 'physnet1', + 'segmentation_id': 200 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_2)) + + provider_net_3 = { + 'physical_network': 'physnet1', + 'segmentation_id': 99 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_3)) + + provider_net_4 = { + 'physical_network': 'physnet1', + 'segmentation_id': 201 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_4)) + + def test__is_vlan_project_network_physnet_not_found(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = { + 'physnet1': [(100, 200)] + } + + provider_net = { + 'physical_network': 'physnet2', + 'segmentation_id': 150 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + + def test__is_vlan_project_network_empty_ranges(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = {} + + provider_net = { + 'physical_network': 'physnet1', + 'segmentation_id': 100 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + + def test__is_vlan_project_network_no_vlan_driver(self): + self.mock_vlan_driver_property.return_value = None + provider_net = { + 'physical_network': 'physnet1', + 'segmentation_id': 100 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + + def _mock_tunnel_driver(self, network_type, ranges): + mock_tunnel_driver = mock.Mock() + mock_tunnel_driver.obj.get_network_segment_ranges.return_value = ranges + return mock.patch.object( + self.driver, 'get_tunnel_driver', + return_value=mock_tunnel_driver) + + def test__is_tunnel_project_network_with_multiple_ranges(self): + with self._mock_tunnel_driver( + constants.TYPE_VXLAN, [(100, 200), (300, 400)]): + provider_net_in_range = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 150, + } + self.assertTrue( + self.driver._is_tunnel_project_network(provider_net_in_range)) + + provider_net_between_ranges = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 250, + } + self.assertFalse(self.driver._is_tunnel_project_network( + provider_net_between_ranges)) + + provider_net_second_range = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 350, + } + self.assertTrue(self.driver._is_tunnel_project_network( + provider_net_second_range)) + + def test__is_tunnel_project_network_boundary_values(self): + with self._mock_tunnel_driver(constants.TYPE_GENEVE, [(100, 200)]): + provider_net_min = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 100, + } + self.assertTrue( + self.driver._is_tunnel_project_network(provider_net_min)) + + provider_net_max = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 200, + } + self.assertTrue( + self.driver._is_tunnel_project_network(provider_net_max)) + + provider_net_below = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 99, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net_below)) + + provider_net_above = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 201, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net_above)) + + def test__is_tunnel_project_network_empty_ranges(self): + with self._mock_tunnel_driver(constants.TYPE_GRE, []): + provider_net = { + 'network_type': constants.TYPE_GRE, + 'segmentation_id': 100, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net)) + + def test__is_tunnel_project_network_no_tunnel_driver(self): + with mock.patch.object( + self.driver, 'get_tunnel_driver', return_value=None): + provider_net = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 100, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net)) + + class TestDesignateClientKeystoneV3(testtools.TestCase): """Test case for designate clients """ diff --git a/neutron/tests/unit/services/conntrack_helper/test_plugin.py b/neutron/tests/unit/services/conntrack_helper/test_plugin.py index 93b258ee25f..e3c53662361 100644 --- a/neutron/tests/unit/services/conntrack_helper/test_plugin.py +++ b/neutron/tests/unit/services/conntrack_helper/test_plugin.py @@ -193,9 +193,10 @@ def test_update_conntrack_helper(self, mock_cth_get_object, mock_rpc_push): cth_obj = mock.Mock() cth_obj.helper = 'tftp' cth_obj.protocol = 'udp' + cth_obj.router_id = 'fake-router' mock_cth_get_object.return_value = cth_obj self.cth_plugin.update_router_conntrack_helper( - self.ctxt, 'cth_id', mock.ANY, **cth_input) + self.ctxt, 'cth_id', 'fake-router', **cth_input) mock_cth_get_object.assert_called_once_with(self.ctxt, id='cth_id') self.assertTrue(cth_obj.update_fields) self.assertTrue(cth_obj.update) @@ -216,12 +217,31 @@ def test_negative_update_conntrack_helper(self, mock_cth_get_object): self.assertRaises( cth_exc.ConntrackHelperNotFound, self.cth_plugin.update_router_conntrack_helper, - self.ctxt, 'cth_id', mock.ANY, **cth_input) + self.ctxt, 'cth_id', 'fake-router', **cth_input) + + @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') + def test_negative_update_conntrack_helper_wrong_router( + self, mock_cth_get_object): + cth_input = { + 'conntrack_helper': { + 'conntrack_helper': { + 'protocol': 'udp', + 'port': 69, + 'helper': 'tftp'} + } + } + mock_cth_get_object.return_value = mock.Mock( + router_id='correct-router') + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.update_router_conntrack_helper, + self.ctxt, 'cth_id', 'other-router', **cth_input) @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') def test_get_conntrack_helper(self, get_object_mock): + get_object_mock.return_value = mock.Mock(router_id='fake-router') self.cth_plugin.get_router_conntrack_helper( - self.ctxt, 'cth_id', mock.ANY, fields=None) + self.ctxt, 'cth_id', 'fake-router', fields=None) get_object_mock.assert_called_once_with(self.ctxt, id='cth_id') @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') @@ -230,7 +250,16 @@ def test_negative_get_conntrack_helper(self, get_object_mock): self.assertRaises( cth_exc.ConntrackHelperNotFound, self.cth_plugin.get_router_conntrack_helper, - self.ctxt, 'cth_id', mock.ANY, fields=None) + self.ctxt, 'cth_id', 'fake-router', fields=None) + + @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') + def test_negative_get_conntrack_helper_wrong_router(self, + get_object_mock): + get_object_mock.return_value = mock.Mock(router_id='victim-router') + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.get_router_conntrack_helper, + self.ctxt, 'cth_id', 'other-router', fields=None) @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_objects') def test_get_conntrack_helpers(self, get_objects_mock): @@ -248,7 +277,7 @@ def test_delete_conntrack_helper(self, get_object_mock, mock_rpc_push): helper='tftp') get_object_mock.return_value = cth_obj self.cth_plugin.delete_router_conntrack_helper(self.ctxt, 'cth_id', - mock.ANY) + 'fake-router') cth_obj.delete.assert_called() mock_rpc_push.assert_called_once_with( self.ctxt, mock.ANY, rpc_events.DELETED) @@ -258,4 +287,12 @@ def test_negative_delete_conntrack_helper(self, get_object_mock): get_object_mock.return_value = None self.assertRaises(cth_exc.ConntrackHelperNotFound, self.cth_plugin.delete_router_conntrack_helper, - self.ctxt, 'cth_id', mock.ANY) + self.ctxt, 'cth_id', 'fake-router') + + @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') + def test_negative_delete_conntrack_helper_wrong_router( + self, get_object_mock): + get_object_mock.return_value = mock.Mock(router_id='correct-router') + self.assertRaises(cth_exc.ConntrackHelperNotFound, + self.cth_plugin.delete_router_conntrack_helper, + self.ctxt, 'cth_id', 'other-router') diff --git a/neutron/tests/unit/services/portforwarding/test_pf_plugin.py b/neutron/tests/unit/services/portforwarding/test_pf_plugin.py index 1d73650da09..298e134e122 100644 --- a/neutron/tests/unit/services/portforwarding/test_pf_plugin.py +++ b/neutron/tests/unit/services/portforwarding/test_pf_plugin.py @@ -90,10 +90,20 @@ def setUp(self): @mock.patch.object(port_forwarding.PortForwarding, 'get_object') def test_get_floatingip_port_forwarding(self, get_object_mock): + get_object_mock.return_value = mock.Mock(floatingip_id='test-fip-id') self.pf_plugin.get_floatingip_port_forwarding( self.ctxt, 'pf_id', 'test-fip-id', fields=None) get_object_mock.assert_called_once_with(self.ctxt, id='pf_id') + @mock.patch.object(port_forwarding.PortForwarding, 'get_object') + def test_negative_get_floatingip_port_forwarding_wrong_parent( + self, get_object_mock): + get_object_mock.return_value = mock.Mock(floatingip_id='other-fip-id') + self.assertRaises( + pf_exc.PortForwardingNotFound, + self.pf_plugin.get_floatingip_port_forwarding, + self.ctxt, 'pf_id', 'test-fip-id', fields=None) + @mock.patch.object(port_forwarding.PortForwarding, 'get_object', return_value=None) def test_negative_get_floatingip_port_forwarding(self, get_object_mock): @@ -186,6 +196,7 @@ def test_update_floatingip_port_forwarding( pf_obj.internal_ip_address = "10.0.0.1" pf_obj.internal_port = 22 pf_obj.external_port = 222 + pf_obj.floatingip_id = 'fip_id' mock_pf_get_object.return_value = pf_obj port_dict = {'id': 'ID', 'fixed_ips': [{"subnet_id": "test-subnet-id", "ip_address": "10.0.0.1"}]} @@ -293,6 +304,22 @@ def test_negative_update_floatingip_port_forwarding( self.pf_plugin.update_floatingip_port_forwarding, self.ctxt, 'pf_id', **pf_input) + @mock.patch.object(port_forwarding.PortForwarding, 'get_object') + def test_negative_update_floatingip_port_forwarding_wrong_parent( + self, mock_pf_get_object): + pf_input = { + 'port_forwarding': + {'port_forwarding': { + 'internal_ip_address': '1.1.1.1', + 'floatingip_id': 'fip_id'}}, + 'floatingip_id': 'fip_id'} + mock_pf_get_object.return_value = mock.Mock( + floatingip_id='other-fip-id') + self.assertRaises( + pf_exc.PortForwardingNotFound, + self.pf_plugin.update_floatingip_port_forwarding, + self.ctxt, 'pf_id', **pf_input) + @mock.patch.object(pf_plugin.PortForwardingPlugin, '_check_port_has_binding_floating_ip') @mock.patch.object(obj_base.NeutronDbObject, 'update_objects') diff --git a/releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml b/releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml new file mode 100644 index 00000000000..356a4f8a92b --- /dev/null +++ b/releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml @@ -0,0 +1,31 @@ +--- +security: + - | + Fixed overly permissive default RBAC policies for several port actions + that require network ownership. Those policies previously included + ``PROJECT_MANAGER``, which allowed any project manager to perform the + action even when not owning the network (for example, on shared/RBAC + networks). They now rely on ``NET_OWNER_MEMBER`` or + ``ADMIN_OR_NET_OWNER_MEMBER`` only. Project managers who own the + network remain authorized through the default Keystone role implication + chain (``manager`` implies ``member``). + + Affected actions: ``create_port:device_owner``, + ``update_port:device_owner``, ``create_port:mac_address``, + ``update_port:mac_address``, ``create_port:fixed_ips``, + ``create_port:fixed_ips:ip_address``, ``create_port:fixed_ips:subnet_id``, + ``update_port:fixed_ips``, ``update_port:fixed_ips:ip_address``, + ``update_port:fixed_ips:subnet_id``, ``create_port:port_security_enabled``, + ``update_port:port_security_enabled``, + ``create_port:allowed_address_pairs``, + ``create_port:allowed_address_pairs:mac_address``, + ``create_port:allowed_address_pairs:ip_address``, + ``update_port:allowed_address_pairs``, + ``update_port:allowed_address_pairs:mac_address``, and + ``update_port:allowed_address_pairs:ip_address``. +upgrade: + - | + Default RBAC policies for the port actions listed above no longer + include ``PROJECT_MANAGER``. Deployments with customized policy files + should review and update those rules if they rely on the previous + behaviour. diff --git a/releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml b/releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml new file mode 100644 index 00000000000..636cd66722d --- /dev/null +++ b/releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixed a 500 Internal Server Error when querying the quota details API + (``GET /v2.0/quotas/{project_id}/details``) while a service plugin + package (e.g. ``neutron-fwaas``, ``neutron-vpnaas``, + ``networking-sfc``) is installed but its service plugin is not + configured in ``service_plugins``. The quota engine now gracefully + skips resources whose service plugin is not loaded instead of raising + a ``NotImplementedError``. diff --git a/releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml b/releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml new file mode 100644 index 00000000000..df142ec3cff --- /dev/null +++ b/releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml @@ -0,0 +1,8 @@ +--- +security: + - | + Fixed an issue in the router conntrack helper API where a project + member authorized on one router could read, update, or delete another + project's conntrack helper by supplying an incorrect helper ID under their + own router ID in the request URL. Singleton operations now verify that + the conntrack helper belongs to the router specified in the URL. diff --git a/releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml b/releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml new file mode 100644 index 00000000000..3b0f3dadcf1 --- /dev/null +++ b/releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + The ``OVNL3RouterPlugin._post_fork_initialize`` method now returns early + when the worker is not a Neutron API ``WorkerService`` instance. This + prevents ``MechanismDriverOVNNotReady`` errors in the + ``neutron-periodic-workers`` and other non-API processes, where the OVN + IDL connections are not initialized. diff --git a/releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml b/releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml new file mode 100644 index 00000000000..49ea45a9ce7 --- /dev/null +++ b/releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Fixed a race condition in the OVS agent where ``port_alive`` and + ``port_dead`` could issue OpenFlow operations with an invalid + ``in_port`` value when a VIF port had an unassigned or error ofport + (e.g. ``[]`` or ``-1``). The malformed OpenFlow message caused the + os-ken ``send_msg`` call to hang until the 300-second + ``of_request_timeout`` expired, which blocked the agent's + ``rpc_loop`` and prevented all subsequent port processing for the + remainder of the timeout period. The functions now validate the + ofport before performing any OpenFlow operations and log a warning + so the port is retried on the next iteration.