Merge "Fix the tagging policy engine" into stable/2024.1

neutron/extensions/tagging.py

@@ -12,8 +12,10 @@
 #    under the License.
 
 import abc
+import collections
 import copy
 import functools
+import itertools
 
 from neutron_lib.api.definitions import port
 from neutron_lib.api import extensions as api_extensions
@@ -29,7 +31,15 @@ import webob.exc
 from neutron._i18n import _
 from neutron.api import extensions
 from neutron.api.v2 import resource as api_resource
-from neutron.objects import subnet
+from neutron.objects import network as network_obj
+from neutron.objects import network_segment_range as network_segment_range_obj
+from neutron.objects import ports as ports_obj
+from neutron.objects.qos import policy as policy_obj
+from neutron.objects import router as router_obj
+from neutron.objects import securitygroup as securitygroup_obj
+from neutron.objects import subnet as subnet_obj
+from neutron.objects import subnetpool as subnetpool_obj
+from neutron.objects import trunk as trunk_obj
 from neutron import policy
 
 
@@ -58,7 +68,26 @@ TAG_ATTRIBUTE_MAP_PORTS[TAGS] = {
         'validate': {'type:list_of_unique_strings': MAX_TAG_LEN},
         'default': [], 'is_visible': True, 'is_filter': True
 }
-RESOURCES_AND_PARENTS = {'subnets': ('network', subnet.Subnet.get_network_id)}
+PARENTS = {
+    'floatingips': router_obj.FloatingIP,
+    'network_segment_ranges': network_segment_range_obj.NetworkSegmentRange,
+    'networks': network_obj.Network,
+    'policies': policy_obj.QosPolicy,
+    'ports': ports_obj.Port,
+    'routers': router_obj.Router,
+    'security_groups': securitygroup_obj.SecurityGroup,
+    'subnets': ('networks', subnet_obj.Subnet),
+    'subnetpools': subnetpool_obj.SubnetPool,
+    'trunks': trunk_obj.Trunk,
+}
+ResourceInfo = collections.namedtuple(
+    'ResourceInfo', ['project_id',
+                     'parent_type',
+                     'parent_id',
+                     'upper_parent_type',
+                     'upper_parent_id',
+                     ])
+EMPTY_RESOURCE_INFO = ResourceInfo(None, None, None, None, None)
 
 
 def _policy_init(f):
@@ -107,44 +136,68 @@ class TaggingController(object):
         self.plugin = directory.get_plugin(TAG_PLUGIN_TYPE)
         self.supported_resources = TAG_SUPPORTED_RESOURCES
 
-    @staticmethod
+    def _get_target(self, res_info):
-    def _get_target(ctx, res_id, p_res, p_res_id, tag_id=None):
+        target = {'id': res_info.parent_id,
-        target = {'id': res_id,
+                  'tenant_id': res_info.project_id,
-                  'tenant_id': ctx.project_id,
+                  'project_id': res_info.project_id}
-                  'project_id': ctx.project_id}
+        if res_info.upper_parent_type:
-        if p_res:
+            res_id = (self.supported_resources[res_info.upper_parent_type] +
-            target[p_res + '_id'] = p_res_id
+                      '_id')
-        if tag_id:
+            target[res_id] = res_info.upper_parent_id
-            target['tag_id'] = tag_id
         return target
 
-    @staticmethod
+    def _get_resource_info(self, context, kwargs):
-    def _get_pparent_resource_and_id(context, resource, resource_id):
+        """Return the tag parent resource information
-        """Retrieve the parent of the resource and ID (e.g.: subnet->net)"""
-        parent, getter_id = RESOURCES_AND_PARENTS[resource]
-        parent_id = getter_id(context.elevated(), resource_id)
-        return parent, parent_id
 
-    def _get_parent_resource_and_id(self, context, kwargs):
+        Some parent resources, like the subnets, depend on other upper parent
-        parent, parent_id = None, None
+        resources (networks). In that case, it is needed to provide the upper
-        for key in kwargs:
+        parent resource information.
-            for resource in self.supported_resources:
+
-                if key == self.supported_resources[resource] + '_id':
+        :param kwargs: dictionary with the parent resource ID, along with other
-                    if resource in RESOURCES_AND_PARENTS.keys():
+                       information not needed. It is formated as
-                        parent, parent_id = self._get_pparent_resource_and_id(
+                       {"resource_id": "id", ...}
-                            context, resource, kwargs[key])
+        :return: ``ResourceInfo`` named tuple with the parent and upper parent
-                    return resource, kwargs[key], parent, parent_id
+                 information and the project ID (of the parent or upper
-        return None, None, None, None
+                 parent).
+        """
+        for key, parent_type in itertools.product(
+                kwargs.keys(), self.supported_resources.keys()):
+            if key != self.supported_resources[parent_type] + '_id':
+                continue
+
+            parent_id = kwargs[key]
+            parent_obj = PARENTS[parent_type]
+            if isinstance(parent_obj, tuple):
+                upper_parent_type = parent_obj[0]
+                parent_obj = parent_obj[1]
+                res_id = (self.supported_resources[upper_parent_type] +
+                          '_id')
+                upper_parent_id = parent_obj.get_values(
+                    context.elevated(), res_id, id=parent_id)[0]
+            else:
+                upper_parent_type = upper_parent_id = None
+
+            try:
+                project_id = parent_obj.get_values(
+                    context.elevated(), 'project_id', id=parent_id)[0]
+            except IndexError:
+                return EMPTY_RESOURCE_INFO
+
+            return ResourceInfo(project_id, parent_type, parent_id,
+                                upper_parent_type, upper_parent_id)
+
+        # This should never be returned.
+        return EMPTY_RESOURCE_INFO
 
     @_policy_init
     def index(self, request, **kwargs):
         # GET /v2.0/{parent_resource}/{parent_resource_id}/tags
         ctx = request.context
-        res, res_id, p_res, p_res_id = self._get_parent_resource_and_id(
+        rinfo = self._get_resource_info(ctx, kwargs)
-            ctx, kwargs)
+        target = self._get_target(rinfo)
-        target = self._get_target(ctx, res_id, p_res, p_res_id)
+        policy.enforce(ctx, 'get_{}_{}'.format(rinfo.parent_type, TAGS),
-        policy.enforce(ctx, 'get_%s_%s' % (res, TAGS), target)
+                       target)
-        return self.plugin.get_tags(ctx, res, res_id)
+        return self.plugin.get_tags(ctx, rinfo.parent_type, rinfo.parent_id)
 
     @_policy_init
     def show(self, request, id, **kwargs):
@@ -152,11 +205,11 @@ class TaggingController(object):
         # id == tag
         validate_tag(id)
         ctx = request.context
-        res, res_id, p_res, p_res_id = self._get_parent_resource_and_id(
+        rinfo = self._get_resource_info(ctx, kwargs)
-            ctx, kwargs)
+        target = self._get_target(rinfo)
-        target = self._get_target(ctx, res_id, p_res, p_res_id, tag_id=id)
+        policy.enforce(ctx, 'get_{}_{}'.format(rinfo.parent_type, TAGS),
-        policy.enforce(ctx, 'get_%s_%s' % (res, TAGS), target)
+                       target)
-        return self.plugin.get_tag(ctx, res, res_id, id)
+        return self.plugin.get_tag(ctx, rinfo.parent_type, rinfo.parent_id, id)
 
     def create(self, request, **kwargs):
         # not supported
@@ -169,13 +222,16 @@ class TaggingController(object):
         # id == tag
         validate_tag(id)
         ctx = request.context
-        res, res_id, p_res, p_res_id = self._get_parent_resource_and_id(
+        rinfo = self._get_resource_info(ctx, kwargs)
-            ctx, kwargs)
+        target = self._get_target(rinfo)
-        target = self._get_target(ctx, res_id, p_res, p_res_id, tag_id=id)
+        policy.enforce(ctx, 'update_{}_{}'.format(rinfo.parent_type, TAGS),
-        policy.enforce(ctx, 'update_%s_%s' % (res, TAGS), target)
+                       target)
-        notify_tag_action(ctx, 'create.start', res, res_id, [id])
+        notify_tag_action(ctx, 'create.start', rinfo.parent_type,
-        result = self.plugin.update_tag(ctx, res, res_id, id)
+                          rinfo.parent_id, [id])
-        notify_tag_action(ctx, 'create.end', res, res_id, [id])
+        result = self.plugin.update_tag(ctx, rinfo.parent_type,
+                                        rinfo.parent_id, id)
+        notify_tag_action(ctx, 'create.end', rinfo.parent_type,
+                          rinfo.parent_id, [id])
         return result
 
     @_policy_init
@@ -184,14 +240,16 @@ class TaggingController(object):
         # body: {"tags": ["aaa", "bbb"]}
         validate_tags(body)
         ctx = request.context
-        res, res_id, p_res, p_res_id = self._get_parent_resource_and_id(
+        rinfo = self._get_resource_info(ctx, kwargs)
-            ctx, kwargs)
+        target = self._get_target(rinfo)
-        target = self._get_target(ctx, res_id, p_res, p_res_id)
+        policy.enforce(ctx, 'update_{}_{}'.format(rinfo.parent_type, TAGS),
-        policy.enforce(ctx, 'update_%s_%s' % (res, TAGS), target)
+                       target)
-        notify_tag_action(ctx, 'update.start', res, res_id, body['tags'])
+        notify_tag_action(ctx, 'update.start', rinfo.parent_type,
-        result = self.plugin.update_tags(ctx, res, res_id, body)
+                          rinfo.parent_id, body['tags'])
-        notify_tag_action(ctx, 'update.end', res, res_id,
+        result = self.plugin.update_tags(ctx, rinfo.parent_type,
-                          body['tags'])
+                                         rinfo.parent_id, body)
+        notify_tag_action(ctx, 'update.end', rinfo.parent_type,
+                          rinfo.parent_id, body['tags'])
         return result
 
     @_policy_init
@@ -200,26 +258,32 @@ class TaggingController(object):
         # id == tag
         validate_tag(id)
         ctx = request.context
-        res, res_id, p_res, p_res_id = self._get_parent_resource_and_id(
+        rinfo = self._get_resource_info(ctx, kwargs)
-            ctx, kwargs)
+        target = self._get_target(rinfo)
-        target = self._get_target(ctx, res_id, p_res, p_res_id, tag_id=id)
+        policy.enforce(ctx, 'delete_{}_{}'.format(rinfo.parent_type, TAGS),
-        policy.enforce(ctx, 'delete_%s_%s' % (res, TAGS), target)
+                       target)
-        notify_tag_action(ctx, 'delete.start', res, res_id, [id])
+        notify_tag_action(ctx, 'delete.start', rinfo.parent_type,
-        result = self.plugin.delete_tag(ctx, res, res_id, id)
+                          rinfo.parent_id, [id])
-        notify_tag_action(ctx, 'delete.end', res, res_id, [id])
+        result = self.plugin.delete_tag(ctx, rinfo.parent_type,
+                                        rinfo.parent_id, id)
+        notify_tag_action(ctx, 'delete.end', rinfo.parent_type,
+                          rinfo.parent_id, [id])
         return result
 
     @_policy_init
     def delete_all(self, request, **kwargs):
         # DELETE /v2.0/{parent_resource}/{parent_resource_id}/tags
         ctx = request.context
-        res, res_id, p_res, p_res_id = self._get_parent_resource_and_id(
+        rinfo = self._get_resource_info(ctx, kwargs)
-            ctx, kwargs)
+        target = self._get_target(rinfo)
-        target = self._get_target(ctx, res_id, p_res, p_res_id)
+        policy.enforce(ctx, 'delete_{}_{}'.format(rinfo.parent_type, TAGS),
-        policy.enforce(ctx, 'delete_%s_%s' % (res, TAGS), target)
+                       target)
-        notify_tag_action(ctx, 'delete_all.start', res, res_id)
+        notify_tag_action(ctx, 'delete_all.start', rinfo.parent_type,
-        result = self.plugin.delete_tags(ctx, res, res_id)
+                          rinfo.parent_id)
-        notify_tag_action(ctx, 'delete_all.end', res, res_id)
+        result = self.plugin.delete_tags(ctx, rinfo.parent_type,
+                                         rinfo.parent_id)
+        notify_tag_action(ctx, 'delete_all.end', rinfo.parent_type,
+                          rinfo.parent_id)
         return result
 
 

neutron/objects/subnet.py

@@ -13,7 +13,6 @@
 import netaddr
 from neutron_lib.api import validators
 from neutron_lib import constants as const
-from neutron_lib.db import api as db_api
 from neutron_lib.db import model_query
 from neutron_lib.objects import common_types
 from neutron_lib.utils import net as net_utils
@@ -23,7 +22,6 @@ from oslo_utils import versionutils
 from oslo_versionedobjects import fields as obj_fields
 from sqlalchemy import and_, or_
 from sqlalchemy import orm
-from sqlalchemy.orm import exc as orm_exc
 from sqlalchemy.sql import exists
 
 from neutron.db.models import dns as dns_models
@@ -547,15 +545,6 @@ class Subnet(base.NeutronDbObject):
 
         return [segment_id for (segment_id,) in query.all()]
 
-    @classmethod
-    @db_api.CONTEXT_READER
-    def get_network_id(cls, context, subnet_id):
-        try:
-            return context.session.query(cls.db_model.network_id).filter(
-                cls.db_model.id == subnet_id).one()[0]
-        except orm_exc.NoResultFound:
-            return None
-
 
 @base.NeutronObjectRegistry.register
 class NetworkSubnetLock(base.NeutronDbObject):

neutron/tests/unit/extensions/test_tagging.py

@@ -0,0 +1,179 @@
+# Copyright 2024 Red Hat, Inc.
+# All rights reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+import netaddr
+from neutron_lib import constants as n_const
+from neutron_lib import context
+from neutron_lib.utils import net as net_utils
+from oslo_utils import uuidutils
+
+from neutron.extensions import tagging
+from neutron.objects import network as network_obj
+from neutron.objects import network_segment_range as network_segment_range_obj
+from neutron.objects import ports as ports_obj
+from neutron.objects.qos import policy as policy_obj
+from neutron.objects import router as router_obj
+from neutron.objects import securitygroup as securitygroup_obj
+from neutron.objects import subnet as subnet_obj
+from neutron.objects import subnetpool as subnetpool_obj
+from neutron.objects import trunk as trunk_obj
+from neutron.tests.unit import testlib_api
+
+
+class TaggingControllerDbTestCase(testlib_api.WebTestCase):
+    def setUp(self):
+        super().setUp()
+        self.user_id = uuidutils.generate_uuid()
+        self.project_id = uuidutils.generate_uuid()
+        self.ctx = context.Context(user_id=self.user_id,
+                                   tenant_id=self.project_id,
+                                   is_admin=False)
+        self.tc = tagging.TaggingController()
+
+    def test_all_parents_have_a_reference(self):
+        tc_supported_resources = set(self.tc.supported_resources.keys())
+        parent_resources = set(tagging.PARENTS.keys())
+        self.assertEqual(tc_supported_resources, parent_resources)
+
+    def _check_resource_info(self, parent_id, parent_type,
+                             upper_parent_id=None, upper_parent_type=None):
+        p_id = self.tc.supported_resources[parent_type] + '_id'
+        res = self.tc._get_resource_info(self.ctx, {p_id: parent_id})
+        reference = tagging.ResourceInfo(
+            self.project_id, parent_type, parent_id,
+            upper_parent_type, upper_parent_id)
+        self.assertEqual(reference, res)
+
+    def test__get_resource_info_floatingips(self):
+        ext_net_id = uuidutils.generate_uuid()
+        fip_port_id = uuidutils.generate_uuid()
+        fip_id = uuidutils.generate_uuid()
+        network_obj.Network(
+            self.ctx, id=ext_net_id, project_id=self.project_id).create()
+        network_obj.ExternalNetwork(
+            self.ctx, project_id=self.project_id,
+            network_id=ext_net_id).create()
+        mac_str = next(net_utils.random_mac_generator(
+            ['ca', 'fe', 'ca', 'fe']))
+        mac = netaddr.EUI(mac_str)
+        ports_obj.Port(
+            self.ctx, id=fip_port_id, project_id=self.project_id,
+            mac_address=mac, network_id=ext_net_id, admin_state_up=True,
+            status='UP', device_id='', device_owner='').create()
+        ip_address = netaddr.IPAddress('1.2.3.4')
+        router_obj.FloatingIP(
+            self.ctx, id=fip_id, project_id=self.project_id,
+            floating_network_id=ext_net_id, floating_port_id=fip_port_id,
+            floating_ip_address=ip_address).create()
+        self._check_resource_info(fip_id, 'floatingips')
+
+    def test__get_resource_info_network_segment_ranges(self):
+        srange_id = uuidutils.generate_uuid()
+        network_segment_range_obj.NetworkSegmentRange(
+            self.ctx, id=srange_id, project_id=self.project_id,
+            shared=False, network_type=n_const.TYPE_GENEVE).create()
+        self._check_resource_info(srange_id, 'network_segment_ranges')
+
+    def test__get_resource_info_networks(self):
+        net_id = uuidutils.generate_uuid()
+        network_obj.Network(
+            self.ctx, id=net_id, project_id=self.project_id).create()
+        self._check_resource_info(net_id, 'networks')
+
+    def test__get_resource_info_policies(self):
+        qos_id = uuidutils.generate_uuid()
+        policy_obj.QosPolicy(
+            self.ctx, id=qos_id, project_id=self.project_id).create()
+        self._check_resource_info(qos_id, 'policies')
+
+    def test__get_resource_info_ports(self):
+        net_id = uuidutils.generate_uuid()
+        port_id = uuidutils.generate_uuid()
+        network_obj.Network(
+            self.ctx, id=net_id, project_id=self.project_id).create()
+        mac_str = next(net_utils.random_mac_generator(
+            ['ca', 'fe', 'ca', 'fe']))
+        mac = netaddr.EUI(mac_str)
+        ports_obj.Port(
+            self.ctx, id=port_id, project_id=self.project_id,
+            mac_address=mac, network_id=net_id, admin_state_up=True,
+            status='UP', device_id='', device_owner='').create()
+        self._check_resource_info(port_id, 'ports')
+
+    def test__get_resource_info_routers(self):
+        router_id = uuidutils.generate_uuid()
+        router_obj.Router(
+            self.ctx, id=router_id, project_id=self.project_id).create()
+        self._check_resource_info(router_id, 'routers')
+
+    def test__get_resource_info_security_groups(self):
+        sg_id = uuidutils.generate_uuid()
+        securitygroup_obj.SecurityGroup(
+            self.ctx, id=sg_id, project_id=self.project_id,
+            is_default=True).create()
+        self._check_resource_info(sg_id, 'security_groups')
+
+    def test__get_resource_info_subnets(self):
+        net_id = uuidutils.generate_uuid()
+        subnet_id = uuidutils.generate_uuid()
+        network_obj.Network(
+            self.ctx, id=net_id, project_id=self.project_id).create()
+        cidr = netaddr.IPNetwork('1.2.3.0/24')
+        subnet_obj.Subnet(
+            self.ctx, id=subnet_id, project_id=self.project_id,
+            ip_version=n_const.IP_VERSION_4, cidr=cidr,
+            network_id=net_id).create()
+        self._check_resource_info(subnet_id, 'subnets',
+                                  upper_parent_id=net_id,
+                                  upper_parent_type='networks')
+
+    def test__get_resource_info_subnetpools(self):
+        sp_id = uuidutils.generate_uuid()
+        subnetpool_obj.SubnetPool(
+            self.ctx, id=sp_id, project_id=self.project_id,
+            ip_version=n_const.IP_VERSION_4, default_prefixlen=26,
+            min_prefixlen=28, max_prefixlen=26).create()
+        self._check_resource_info(sp_id, 'subnetpools')
+
+    def test__get_resource_info_trunks(self):
+        trunk_id = uuidutils.generate_uuid()
+        net_id = uuidutils.generate_uuid()
+        port_id = uuidutils.generate_uuid()
+        network_obj.Network(
+            self.ctx, id=net_id, project_id=self.project_id).create()
+        mac_str = next(net_utils.random_mac_generator(
+            ['ca', 'fe', 'ca', 'fe']))
+        mac = netaddr.EUI(mac_str)
+        ports_obj.Port(
+            self.ctx, id=port_id, project_id=self.project_id,
+            mac_address=mac, network_id=net_id, admin_state_up=True,
+            status='UP', device_id='', device_owner='').create()
+        trunk_obj.Trunk(
+            self.ctx, id=trunk_id, project_id=self.project_id,
+            port_id=port_id).create()
+        self._check_resource_info(trunk_id, 'trunks')
+
+    def test__get_resource_info_parent_not_present(self):
+        missing_id = uuidutils.generate_uuid()
+        p_id = self.tc.supported_resources['trunks'] + '_id'
+        res = self.tc._get_resource_info(self.ctx, {p_id: missing_id})
+        self.assertEqual(tagging.EMPTY_RESOURCE_INFO, res)
+
+    def test__get_resource_info_wrong_resource(self):
+        missing_id = uuidutils.generate_uuid()
+        res = self.tc._get_resource_info(self.ctx,
+                                         {'wrong_resource_id': missing_id})
+        self.assertEqual(tagging.EMPTY_RESOURCE_INFO, res)