From 413793fa086eddce7bf5fea0141c20ca270de780 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 29 Jan 2024 23:47:06 +0000 Subject: [PATCH] Add "subnet-external-network" extension to "subnet" resource Added a new extension called "subnet-external-network". This extension adds a new field to the subnet resource: "router:external". It is named after the field "router:external" in the network resource. The subnet inherits the value form the network it belongs. Subnet OVO: the subnet OVO receives a new field called "external", same as the network OVO. It is a synthetic field inherited from the network one. Filter query: same as the network database query, the subnet database query receives a new extra filter to add the action "access_as_external", inherited from the network RBAC. It is needed to be able to retrieve these subnets that belong to external network and thus visible by a non-admin user. The minimum neutron-lib version required is 3.11.0 that contains the patch [1]. [1]https://review.opendev.org/c/openstack/neutron-lib/+/907949 Depends-On: https://review.opendev.org/c/openstack/tempest/+/922938 Depends-On: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/922711 Depends-On: https://review.opendev.org/c/openstack/neutron-lib/+/912273 Closes-Bug: #2051831 Change-Id: Ice91de9ae7f82b983579272af6e2bf10c3a02dbf --- neutron/common/ovn/extensions.py | 2 + neutron/conf/policies/subnet.py | 9 ++++ neutron/db/external_net_db.py | 14 ++++++ neutron/db/models/external_net.py | 5 ++ neutron/extensions/subnet_external_network.py | 22 +++++++++ neutron/objects/subnet.py | 39 ++++++++++++++-- .../ovn/mech_driver/ovsdb/ovn_client.py | 8 +--- neutron/plugins/ml2/plugin.py | 3 ++ .../tests/contrib/hooks/api_all_extensions | 1 + .../tests/unit/conf/policies/test_subnet.py | 46 ++++++++++++++++++- neutron/tests/unit/objects/test_base.py | 6 ++- neutron/tests/unit/objects/test_objects.py | 2 +- neutron/tests/unit/objects/test_subnet.py | 17 +++++++ 13 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 neutron/extensions/subnet_external_network.py diff --git a/neutron/common/ovn/extensions.py b/neutron/common/ovn/extensions.py index e40e01c0c09..f8e7c93dae0 100644 --- a/neutron/common/ovn/extensions.py +++ b/neutron/common/ovn/extensions.py @@ -85,6 +85,7 @@ from neutron_lib.api.definitions import segment as seg_def from neutron_lib.api.definitions import sorting from neutron_lib.api.definitions import stateful_security_group from neutron_lib.api.definitions import subnet_dns_publish_fixed_ip +from neutron_lib.api.definitions import subnet_external_network from neutron_lib.api.definitions import subnet_service_types from neutron_lib.api.definitions import subnetpool_prefix_ops from neutron_lib.api.definitions import trunk @@ -183,6 +184,7 @@ ML2_SUPPORTED_API_EXTENSIONS = [ 'standard-attr-tag', 'standard-attr-timestamp', subnetpool_prefix_ops.ALIAS, + subnet_external_network.ALIAS, subnet_service_types.ALIAS, trunk.ALIAS, seg_def.ALIAS, diff --git a/neutron/conf/policies/subnet.py b/neutron/conf/policies/subnet.py index 5052e674763..ce7a38ec8b4 100644 --- a/neutron/conf/policies/subnet.py +++ b/neutron/conf/policies/subnet.py @@ -52,6 +52,12 @@ ACTION_DELETE_TAGS = [ rules = [ + policy.RuleDefault( + name='external_network', + check_str='field:subnets:router:external=True', + description='Definition of a subnet that belongs to an external ' + 'network' + ), policy.DocumentedRuleDefault( name='create_subnet', check_str=base.ADMIN_OR_NET_OWNER_MEMBER, @@ -97,6 +103,7 @@ rules = [ check_str=neutron_policy.policy_or( base.PROJECT_READER, 'rule:shared', + 'rule:external_network', base.ADMIN_OR_NET_OWNER_MEMBER, ), scope_types=['project'], @@ -106,6 +113,7 @@ rules = [ name='get_subnet', check_str=neutron_policy.policy_or( 'rule:shared', + 'rule:external_network', neutron_policy.RULE_ADMIN_OR_OWNER, ), deprecated_reason=DEPRECATED_REASON, @@ -128,6 +136,7 @@ rules = [ check_str=neutron_policy.policy_or( base.PROJECT_READER, 'rule:shared', + 'rule:external_network', base.ADMIN_OR_NET_OWNER_MEMBER, ), scope_types=['project'], diff --git a/neutron/db/external_net_db.py b/neutron/db/external_net_db.py index fb6c3c9d902..057b59c9b03 100644 --- a/neutron/db/external_net_db.py +++ b/neutron/db/external_net_db.py @@ -15,6 +15,7 @@ from neutron_lib.api.definitions import external_net as extnet_apidef from neutron_lib.api.definitions import network as net_def +from neutron_lib.api.definitions import subnet as subnet_def from neutron_lib.api import validators from neutron_lib.callbacks import events from neutron_lib.callbacks import registry @@ -78,6 +79,12 @@ class External_net_db_mixin(object): query_hook=None, filter_hook=_network_filter_hook, result_filters=_network_result_filter_hook) + model_query.register_hook( + models_v2.Subnet, + "external_subnet", + query_hook=None, + filter_hook=_network_filter_hook, + result_filters=None) return super(External_net_db_mixin, cls).__new__(cls, *args, **kwargs) def _network_is_external(self, context, net_id): @@ -91,6 +98,13 @@ class External_net_db_mixin(object): network_res[extnet_apidef.EXTERNAL] = network_db.external is not None return network_res + @staticmethod + @resource_extend.extends([subnet_def.COLLECTION_NAME]) + def _extend_subnet_dict_l3(subnet_res, subnet_db): + # Comparing with None for converting uuid into bool + subnet_res[extnet_apidef.EXTERNAL] = bool(subnet_db.external) + return subnet_res + def _process_l3_create(self, context, net_data, req_data): external = req_data.get(extnet_apidef.EXTERNAL) external_set = validators.is_attr_set(external) diff --git a/neutron/db/models/external_net.py b/neutron/db/models/external_net.py index ae8081b90f0..b5466656e18 100644 --- a/neutron/db/models/external_net.py +++ b/neutron/db/models/external_net.py @@ -35,4 +35,9 @@ class ExternalNetwork(model_base.BASEV2): models_v2.Network, load_on_pending=True, backref=orm.backref("external", lazy='joined', uselist=False, cascade='delete')) + subnet = orm.relationship( + models_v2.Subnet, load_on_pending=True, + backref=orm.backref('external', lazy='joined', uselist=False), + primaryjoin='foreign(Subnet.network_id) == ExternalNetwork.network_id', + viewonly=True) revises_on_change = ('network', ) diff --git a/neutron/extensions/subnet_external_network.py b/neutron/extensions/subnet_external_network.py new file mode 100644 index 00000000000..c8296721d72 --- /dev/null +++ b/neutron/extensions/subnet_external_network.py @@ -0,0 +1,22 @@ +# Copyright 2024 Red Hat Inc. +# +# 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.api.definitions import subnet_external_network +from neutron_lib.api import extensions + + +class Subnet_external_network(extensions.APIExtensionDescriptor): + """Extension class to define if a subnet belongs to an external network""" + + api_definition = subnet_external_network diff --git a/neutron/objects/subnet.py b/neutron/objects/subnet.py index bafddf09e06..36caed24779 100644 --- a/neutron/objects/subnet.py +++ b/neutron/objects/subnet.py @@ -11,6 +11,7 @@ # under the License. import netaddr +from neutron_lib.api.definitions import external_net from neutron_lib.api import validators from neutron_lib import constants as const from neutron_lib.db import api as db_api @@ -208,7 +209,8 @@ class SubnetServiceType(base.NeutronDbObject): class Subnet(base.NeutronDbObject): # Version 1.0: Initial version # Version 1.1: Add dns_publish_fixed_ip field - VERSION = '1.1' + # Version 1.2: Add external field + VERSION = '1.2' db_model = models_v2.Subnet @@ -234,12 +236,13 @@ class Subnet(base.NeutronDbObject): 'host_routes': obj_fields.ListOfObjectsField('Route', nullable=True), 'ipv6_ra_mode': common_types.IPV6ModeEnumField(nullable=True), 'ipv6_address_mode': common_types.IPV6ModeEnumField(nullable=True), - 'service_types': obj_fields.ListOfStringsField(nullable=True) + 'service_types': obj_fields.ListOfStringsField(nullable=True), + 'external': obj_fields.BooleanField(nullable=True), } synthetic_fields = ['allocation_pools', 'dns_nameservers', 'dns_publish_fixed_ip', 'host_routes', - 'service_types', 'shared'] + 'service_types', 'shared', 'external'] foreign_keys = {'Network': {'network_id': 'id'}} @@ -260,6 +263,8 @@ class Subnet(base.NeutronDbObject): return self._load_shared() if attrname == 'service_types': return self._load_service_types() + if attrname == 'external': + return self._load_external() super(Subnet, self).obj_load_attr(attrname) def _load_dns_publish_fixed_ip(self, db_obj=None): @@ -306,11 +311,37 @@ class Subnet(base.NeutronDbObject): service_type in service_types] self.obj_reset_changes(['service_types']) + def _load_external(self, db_obj=None): + if db_obj: + external_network = bool(db_obj.get('external')) + else: + external_network = network.ExternalNetwork.get_objects( + self.obj_context, network_id=self.network_id) + + setattr(self, 'external', external_network) + self.obj_reset_changes(['external']) + + @classmethod + def get_objects(cls, context, _pager=None, validate_filters=True, + fields=None, return_db_obj=False, **kwargs): + external = kwargs.pop(external_net.EXTERNAL, None) + if isinstance(external, list): + external = external[0] + subnets = super().get_objects( + context, _pager=_pager, validate_filters=validate_filters, + fields=fields, return_db_obj=return_db_obj, **kwargs) + + if external is not None: + return [subnet for subnet in subnets if + subnet.external == external] + return subnets + def from_db_object(self, db_obj): super(Subnet, self).from_db_object(db_obj) self._load_dns_publish_fixed_ip(db_obj) self._load_shared(db_obj) self._load_service_types(db_obj) + self._load_external(db_obj) @classmethod def modify_fields_from_db(cls, db_obj): @@ -520,6 +551,8 @@ class Subnet(base.NeutronDbObject): _target_version = versionutils.convert_version_to_tuple(target_version) if _target_version < (1, 1): # version 1.1 adds "dns_publish_fixed_ip" primitive.pop('dns_publish_fixed_ip', None) + if _target_version < (1, 2): # version 1.2 adds "external" + primitive.pop('external', None) @classmethod def get_subnet_segment_ids(cls, context, network_id, diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 2168aee9780..f08f3331198 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -1256,13 +1256,7 @@ class OVNClient(object): for fixed_ip in port_fixed_ips: subnet_id = fixed_ip['subnet_id'] - # NOTE(ralonsoh): it is needed to use the "admin" context here to - # retrieve the subnet. The subnet object is not handling correctly - # the RBAC filtering because is not filtering by - # "access_as_external", as network object is doing in - # ``_network_filter_hook``. See LP#2051831. - # TODO(ralonsoh): once LP#2051831 is fixed, remove "elevated()". - subnet = self._plugin.get_subnet(context.elevated(), subnet_id) + subnet = self._plugin.get_subnet(context, subnet_id) cidr = netaddr.IPNetwork(subnet['cidr']) networks.add("%s/%s" % (fixed_ip['ip_address'], str(cidr.prefixlen))) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 89abddf8757..1aae01e3d8f 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -66,6 +66,8 @@ from neutron_lib.api.definitions import \ from neutron_lib.api.definitions import security_groups_shared_filtering from neutron_lib.api.definitions import stateful_security_group from neutron_lib.api.definitions import subnet as subnet_def +from neutron_lib.api.definitions import subnet_external_network as \ + subnet_ext_net_def from neutron_lib.api.definitions import subnet_onboard as subnet_onboard_def from neutron_lib.api.definitions import subnet_service_types from neutron_lib.api.definitions import subnetpool_prefix_ops \ @@ -250,6 +252,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, sg_default_rules_ext.ALIAS, sg_rules_default_sg.ALIAS, phot_def.ALIAS, + subnet_ext_net_def.ALIAS, ] # List of agent types for which all binding_failed ports should try to be diff --git a/neutron/tests/contrib/hooks/api_all_extensions b/neutron/tests/contrib/hooks/api_all_extensions index e237666261b..19cd1d4a57c 100644 --- a/neutron/tests/contrib/hooks/api_all_extensions +++ b/neutron/tests/contrib/hooks/api_all_extensions @@ -77,6 +77,7 @@ NETWORK_API_EXTENSIONS+=",standard-attr-tag" NETWORK_API_EXTENSIONS+=",stateful-security-group" NETWORK_API_EXTENSIONS+=",subnet_allocation" NETWORK_API_EXTENSIONS+=",subnet-dns-publish-fixed-ip" +NETWORK_API_EXTENSIONS+=",subnet-external-network" NETWORK_API_EXTENSIONS+=",tag-ports-during-bulk-creation" NETWORK_API_EXTENSIONS+=",trunk" NETWORK_API_EXTENSIONS+=",trunk-details" diff --git a/neutron/tests/unit/conf/policies/test_subnet.py b/neutron/tests/unit/conf/policies/test_subnet.py index a490982a3e9..609492f0adb 100644 --- a/neutron/tests/unit/conf/policies/test_subnet.py +++ b/neutron/tests/unit/conf/policies/test_subnet.py @@ -15,6 +15,9 @@ from unittest import mock +from neutron_lib.api import attributes +from neutron_lib.api.definitions import subnet as subnet_def +from neutron_lib.api.definitions import subnet_external_network as sen_def from oslo_policy import policy as base_policy from oslo_utils import uuidutils @@ -25,6 +28,11 @@ from neutron.tests.unit.conf.policies import test_base as base class SubnetAPITestCase(base.PolicyBaseTestCase): def setUp(self): + # Extend subnet "subnet-external-network" extension. This extension + # is not loaded in the unit tests. + rname = subnet_def.COLLECTION_NAME + attributes.RESOURCES[rname].update( + sen_def.RESOURCE_ATTRIBUTE_MAP[rname]) super().setUp() self.network = { @@ -35,17 +43,23 @@ class SubnetAPITestCase(base.PolicyBaseTestCase): 'id': uuidutils.generate_uuid(), 'tenant_id': self.alt_project_id, 'project_id': self.alt_project_id} + self.ext_alt_network = { + 'id': uuidutils.generate_uuid(), + 'tenant_id': self.alt_project_id, + 'project_id': self.alt_project_id} networks = { self.network['id']: self.network, - self.alt_network['id']: self.alt_network} + self.alt_network['id']: self.alt_network, + self.ext_alt_network['id']: self.ext_alt_network, + } self.target = { 'project_id': self.project_id, 'tenant_id': self.project_id, 'network_id': self.network['id'], 'ext_parent_network_id': self.network['id']} - # This network belongs to "project_id", but not the network that + # This subnet belongs to "project_id", but not the network that # belongs to "alt_project_id". self.target_net_alt_target = { 'project_id': self.project_id, @@ -57,6 +71,14 @@ class SubnetAPITestCase(base.PolicyBaseTestCase): 'tenant_id': self.alt_project_id, 'network_id': self.alt_network['id'], 'ext_parent_network_id': self.alt_network['id']} + # Both the subnet and the network belongs to "alt_project_id" and the + # network is external. + self.target_net_ext_alt_target = { + 'project_id': self.alt_project_id, + 'tenant_id': self.alt_project_id, + 'network_id': self.ext_alt_network['id'], + 'ext_parent_network_id': self.ext_alt_network['id'], + 'router:external': True} def get_network(context, id, fields=None): return networks.get(id) @@ -127,6 +149,10 @@ class SystemAdminTests(SubnetAPITestCase): base_policy.InvalidScope, policy.enforce, self.context, 'get_subnet', self.target_net_alt_target) + self.assertRaises( + base_policy.InvalidScope, + policy.enforce, + self.context, 'get_subnet', self.target_net_ext_alt_target) self.assertRaises( base_policy.InvalidScope, policy.enforce, @@ -155,6 +181,10 @@ class SystemAdminTests(SubnetAPITestCase): base_policy.InvalidScope, policy.enforce, self.context, 'get_subnets_tags', self.target_net_alt_target) + self.assertRaises( + base_policy.InvalidScope, + policy.enforce, + self.context, 'get_subnets_tags', self.target_net_ext_alt_target) self.assertRaises( base_policy.InvalidScope, policy.enforce, @@ -306,6 +336,9 @@ class AdminTests(SubnetAPITestCase): self.assertTrue( policy.enforce(self.context, 'get_subnet', self.target_net_alt_target)) + self.assertTrue( + policy.enforce(self.context, 'get_subnet', + self.target_net_ext_alt_target)) self.assertTrue( policy.enforce(self.context, 'get_subnet', self.alt_target)) @@ -325,6 +358,9 @@ class AdminTests(SubnetAPITestCase): self.assertTrue( policy.enforce(self.context, 'get_subnets_tags', self.target_net_alt_target)) + self.assertTrue( + policy.enforce(self.context, 'get_subnets_tags', + self.target_net_ext_alt_target)) self.assertTrue( policy.enforce(self.context, 'get_subnets_tags', self.alt_target)) @@ -445,6 +481,9 @@ class ProjectManagerTests(AdminTests): self.assertTrue( policy.enforce(self.context, 'get_subnet', self.target_net_alt_target)) + self.assertTrue( + policy.enforce(self.context, 'get_subnet', + self.target_net_ext_alt_target)) self.assertRaises( base_policy.PolicyNotAuthorized, policy.enforce, @@ -470,6 +509,9 @@ class ProjectManagerTests(AdminTests): self.assertTrue( policy.enforce(self.context, 'get_subnets_tags', self.target_net_alt_target)) + self.assertTrue( + policy.enforce(self.context, 'get_subnets_tags', + self.target_net_ext_alt_target)) self.assertRaises( base_policy.PolicyNotAuthorized, policy.enforce, diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py index 9281b072552..7e5ddda033f 100644 --- a/neutron/tests/unit/objects/test_base.py +++ b/neutron/tests/unit/objects/test_base.py @@ -1551,12 +1551,16 @@ class BaseDbObjectTestCase(_BaseObjectTestCase, ] def _create_test_network(self, name='test-network1', network_id=None, - qos_policy_id=None): + qos_policy_id=None, external=False): network_id = (uuidutils.generate_uuid() if network_id is None else network_id) _network = net_obj.Network(self.context, name=name, id=network_id, qos_policy_id=qos_policy_id) _network.create() + if external: + ext_net = net_obj.ExternalNetwork(self.context, + network_id=network_id) + ext_net.create() return _network def _create_test_network_id(self, qos_policy_id=None): diff --git a/neutron/tests/unit/objects/test_objects.py b/neutron/tests/unit/objects/test_objects.py index 128ac397e0d..7d29d2e8054 100644 --- a/neutron/tests/unit/objects/test_objects.py +++ b/neutron/tests/unit/objects/test_objects.py @@ -120,7 +120,7 @@ object_data = { 'SegmentHostMapping': '1.0-521597cf82ead26217c3bd10738f00f0', 'ServiceProfile': '1.0-9beafc9e7d081b8258f3c5cb66ac5eed', 'StandardAttribute': '1.0-617d4f46524c4ce734a6fc1cc0ac6a0b', - 'Subnet': '1.1-5b7e1789a1732259d1e28b4bd87eb1c2', + 'Subnet': '1.2-476759b72624961bb4aade8203289227', 'SubnetDNSPublishFixedIP': '1.0-db22af6fa20b143986f0cbe06cbfe0ea', 'SubnetPool': '1.1-a0e03895d1a6e7b9d4ab7b0ca13c3867', 'SubnetPoolPrefix': '1.0-13c15144135eb869faa4a76dc3ee3b6c', diff --git a/neutron/tests/unit/objects/test_subnet.py b/neutron/tests/unit/objects/test_subnet.py index 05ddb9a3eb5..b44893eb19c 100644 --- a/neutron/tests/unit/objects/test_subnet.py +++ b/neutron/tests/unit/objects/test_subnet.py @@ -288,6 +288,23 @@ class SubnetDbObjectTestCase(obj_test_base.BaseDbObjectTestCase, self.assertNotEqual( candidate_subnet[0]['id'], candidate_subnet[1]['id']) + def test_get_external_network(self): + for idx, external in ((0, False), (1, True)): + net = self._create_test_network(external=external) + self.obj_fields[idx]['network_id'] = net.id + snet = self._make_object(self.obj_fields[idx]) + snet.create() + snet_obj = subnet.Subnet.get_object(self.context, id=snet.id) + self.assertEqual(external, snet_obj.external) + + def test_object_version_degradation_1_2_to_1_1_no_external(self): + self.objs[0].create() + subnet_obj = self.objs[0] + subnet_dict = subnet_obj.obj_to_primitive('1.2') + self.assertIn('external', subnet_dict['versioned_object.data']) + subnet_dict = subnet_obj.obj_to_primitive('1.1') + self.assertNotIn('external', subnet_dict['versioned_object.data']) + class NetworkSubnetLockTestCase(obj_test_base.BaseObjectIfaceTestCase):