openstack/neutron
Code Issues Proposed changes

Merge "Add limit of tags for every resource"

Browse Source
This commit is contained in:
Zuul
2025-01-29 13:28:36 +00:00
committed by Gerrit Code Review
parent 8c479abcbb 810b224b91
commit e10e171f83
5 changed files with 278 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
neutron/cmd/upgrade_checks/checks.py
View File

@@ -40,8 +40,10 @@ from neutron.db.models import l3 as l3_models
from neutron.db.models import l3ha as l3ha_models
from neutron.db.models.plugins.ml2 import vlanallocation
from neutron.db.models import segment
from neutron.db.models import tag as tag_model
from neutron.db import models_v2
from neutron.db.qos import models as qos_models
from neutron.extensions import tagging
from neutron.objects import ports as port_obj
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
@@ -180,6 +182,18 @@ def get_duplicated_ha_networks_per_project():
return query.all()
def is_tags_limit_reached_for_any_resource():
"""Return True if any resource has more tags then defined limit."""
ctx = context.get_admin_context()
with db_api.CONTEXT_READER.using(ctx):
query = ctx.session.query(
tag_model.Tag,
func.count(tag_model.Tag.tag))
query = query.group_by(tag_model.Tag.standard_attr_id)
query = query.having(func.count() > tagging.MAX_TAGS_COUNT)
return bool(query.one_or_none())
def get_ovn_client():
global _OVN_CLIENT
if _OVN_CLIENT is None:
@@ -227,6 +241,8 @@ class CoreChecks(base.BaseChecks):
self.ml2_ovs_igmp_flood_check),
(_('Floating IP Port forwarding and OVN L3 plugin configuration'),
self.ovn_port_forwarding_configuration_check),
(_('Existing tags exceeds limit per resource'),
self.tags_over_limit_check)
]
@staticmethod
@@ -620,3 +636,25 @@ class CoreChecks(base.BaseChecks):
'can not be used with ML2/OVN backend, distributed '
'floating IPs and provider network type(s) used as '
'tenant networks.'))
@staticmethod
def tags_over_limit_check(checker):
if not cfg.CONF.database.connection:
return upgradecheck.Result(
upgradecheck.Code.WARNING,
_("Database connection string is not set. Check for "
"extra_dhcp_opts can't be done."))
if is_tags_limit_reached_for_any_resource():
return upgradecheck.Result(
upgradecheck.Code.WARNING,
_('Some resources have already more than %(limit)d tags '
'created. There is a limit of %(limit)d tags per '
'resource and because of that limit creation of new '
'tags for the resources that exceed the limit will '
'not be possible until some of the tags are removed.' %
{'limit': tagging.MAX_TAGS_COUNT}))
return upgradecheck.Result(
upgradecheck.Code.SUCCESS,
_('Number of tags for each resource is below the limit of %d. ' %
tagging.MAX_TAGS_COUNT))

15
neutron/extensions/tagging.py
View File

@@ -50,6 +50,7 @@ TAGS_ANY = TAGS + '-any'
NOT_TAGS = 'not-' + TAGS
NOT_TAGS_ANY = NOT_TAGS + '-any'
MAX_TAG_LEN = 255
MAX_TAGS_COUNT = 50
TAG_PLUGIN_TYPE = 'TAG'
TAG_SUPPORTED_RESOURCES = standard_attr.get_tag_resource_parent_map()
@@ -105,6 +106,14 @@ def validate_tag(tag):
raise exceptions.InvalidInput(error_message=msg)
def validate_tags_limit(resource, tags):
tags = set(tags)
if len(tags) > MAX_TAGS_COUNT:
msg = (_("The number of tags exceed the per-resource limit of %d")
% MAX_TAGS_COUNT)
raise exceptions.BadRequest(resource=resource, msg=msg)
def validate_tags(body):
if not isinstance(body, dict) or 'tags' not in body:
raise exceptions.InvalidInput(error_message=_("Invalid tags body"))
@@ -210,6 +219,7 @@ class TaggingController:
rinfo = self._get_resource_info(ctx, kwargs, tags=body[TAGS])
policy.enforce(ctx, 'create_{}:{}'.format(rinfo.obj_type, TAGS),
rinfo.obj)
validate_tags_limit(rinfo.obj_type, body['tags'])
notify_tag_action(ctx, 'create.start', rinfo.obj_type,
rinfo.obj['id'], body['tags'])
result = self.plugin.create_tags(ctx, rinfo.obj_type,
@@ -226,6 +236,10 @@ class TaggingController:
rinfo = self._get_resource_info(ctx, kwargs, tags=[id])
policy.enforce(ctx, 'update_{}:{}'.format(rinfo.obj_type, TAGS),
rinfo.obj)
current_tags = self.plugin.get_tags(
ctx, rinfo.obj_type, rinfo.obj['id'])['tags']
new_tags = current_tags + [id]
validate_tags_limit(rinfo.obj_type, new_tags)
notify_tag_action(ctx, 'create.start', rinfo.obj_type,
rinfo.obj['id'], [id])
result = self.plugin.update_tag(ctx, rinfo.obj_type,
@@ -244,6 +258,7 @@ class TaggingController:
ctx,
self._get_policy_action("update", rinfo.obj_type),
rinfo.obj)
validate_tags_limit(rinfo.obj_type, body['tags'])
notify_tag_action(ctx, 'update.start', rinfo.obj_type,
rinfo.obj['id'], body['tags'])
result = self.plugin.update_tags(ctx, rinfo.obj_type,

18
neutron/tests/unit/cmd/upgrade_checks/test_checks.py
View File

@@ -330,3 +330,21 @@ class TestChecks(base.BaseTestCase):
mock.ANY)
self.assertEqual(Code.WARNING, result.code)
validate_mock.assert_called_once_with()
def test_tags_over_limit_check_success(self):
with mock.patch.object(
checks, 'is_tags_limit_reached_for_any_resource',
return_value=False
) as is_tags_limit_reached_for_any_resource:
result = checks.CoreChecks.tags_over_limit_check(mock.ANY)
self.assertEqual(Code.SUCCESS, result.code)
is_tags_limit_reached_for_any_resource.assert_called_once_with()
def test_tags_over_limit_check_failure(self):
with mock.patch.object(
checks, 'is_tags_limit_reached_for_any_resource',
return_value=True
) as is_tags_limit_reached_for_any_resource:
result = checks.CoreChecks.tags_over_limit_check(mock.ANY)
self.assertEqual(Code.WARNING, result.code)
is_tags_limit_reached_for_any_resource.assert_called_once_with()

194
neutron/tests/unit/extensions/test_tagging.py
View File

@@ -20,6 +20,7 @@ import netaddr
from neutron_lib.api import attributes
from neutron_lib import constants as n_const
from neutron_lib import context
from neutron_lib import exceptions
from neutron_lib.utils import net as net_utils
from oslo_utils import uuidutils
@@ -281,3 +282,196 @@ class TaggingControllerDbTestCase(testlib_api.WebTestCase):
res = self.tc._get_resource_info(self.ctx,
{'wrong_resource_id': missing_id})
self.assertEqual(tagging.EMPTY_RESOURCE_INFO, res)
def test_create_tags_for_resource_below_max_tags_limit(self):
req = mock.Mock(context=self.ctx)
tags = ['tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT - 1)]
body = {'tags': tags}
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.tc.create(req, body)
get_res.assert_called_once_with(self.ctx, mock.ANY, tags=tags)
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.create_tags.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY, body)
notify.assert_has_calls([
mock.call(self.ctx, 'create.start', mock.ANY, mock.ANY, tags),
mock.call(self.ctx, 'create.end', mock.ANY, mock.ANY, tags)])
def test_create_tags_for_resource_over_max_tags_limit(self):
req = mock.Mock(context=self.ctx)
tags = ['tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT + 1)]
body = {'tags': tags}
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.assertRaises(
exceptions.BadRequest,
self.tc.create, req, body)
get_res.assert_called_once_with(self.ctx, mock.ANY, tags=tags)
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.create_tags.assert_not_called()
notify.assert_not_called()
def test_update_tag_for_resource_below_max_tags_limit(self):
req = mock.Mock(context=self.ctx)
kwargs = {'network_id': uuidutils.generate_uuid()}
existing_tags = [
'tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT - 2)]
new_tag = 'new-tag'
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
self.tc.plugin.get_tags.return_value = {'tags': existing_tags}
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.tc.update(req, new_tag, **kwargs)
get_res.assert_called_once_with(self.ctx, kwargs, tags=[new_tag])
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.get_tags.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY)
self.tc.plugin.update_tag.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY, new_tag)
notify.assert_has_calls([
mock.call(
self.ctx, 'create.start', mock.ANY, mock.ANY, [new_tag]),
mock.call(
self.ctx, 'create.end', mock.ANY, mock.ANY, [new_tag])])
def test_update_exising_tag_for_resource_above_max_tags_limit(self):
"""Test to ensure that TaggingController.update() method is idempotent.
"""
req = mock.Mock(context=self.ctx)
kwargs = {'network_id': uuidutils.generate_uuid()}
existing_tags = [
'tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT)]
new_tag = existing_tags[0]
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
self.tc.plugin.get_tags.return_value = {'tags': existing_tags}
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.tc.update(req, new_tag, **kwargs)
get_res.assert_called_once_with(self.ctx, kwargs, tags=[new_tag])
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.get_tags.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY)
self.tc.plugin.update_tag.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY, new_tag)
notify.assert_has_calls([
mock.call(
self.ctx, 'create.start', mock.ANY, mock.ANY, [new_tag]),
mock.call(
self.ctx, 'create.end', mock.ANY, mock.ANY, [new_tag])])
def test_update_tag_for_resource_over_max_tags_limit(self):
req = mock.Mock(context=self.ctx)
kwargs = {'network_id': uuidutils.generate_uuid()}
existing_tags = ['tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT)]
new_tag = 'new-tag'
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
self.tc.plugin.get_tags.return_value = {'tags': existing_tags}
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.assertRaises(
exceptions.BadRequest,
self.tc.update, req, new_tag, **kwargs)
get_res.assert_called_once_with(self.ctx, kwargs, tags=[new_tag])
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.get_tags.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY)
self.tc.plugin.update_tag.assert_not_called()
notify.assert_not_called()
def test_update_all_tags_for_resource_below_max_tags_limit(self):
req = mock.Mock(context=self.ctx)
tags = ['tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT - 1)]
body = {'tags': tags}
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.tc.update_all(req, body)
get_res.assert_called_once_with(self.ctx, mock.ANY, tags=tags)
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.update_tags.assert_called_once_with(
self.ctx, mock.ANY, mock.ANY, body)
notify.assert_has_calls([
mock.call(self.ctx, 'update.start', mock.ANY, mock.ANY, tags),
mock.call(self.ctx, 'update.end', mock.ANY, mock.ANY, tags)])
def test_update_all_tags_for_resource_over_max_tags_limit(self):
req = mock.Mock(context=self.ctx)
tags = ['tag-%i' % i for i in range(tagging.MAX_TAGS_COUNT + 1)]
body = {'tags': tags}
obj_mock = {
'id': uuidutils.generate_uuid()
}
with mock.patch.object(self.tc, '_get_resource_info') as get_res, \
mock.patch('neutron.policy.enforce') as policy_enforce, \
mock.patch.object(tagging, 'notify_tag_action') as notify:
self.tc.plugin = mock.Mock()
get_res.return_value = mock.MagicMock(obj_type='networks',
obj=obj_mock)
self.assertRaises(
exceptions.BadRequest,
self.tc.update_all, req, body)
get_res.assert_called_once_with(self.ctx, mock.ANY, tags=tags)
policy_enforce.assert_called_once_with(
self.ctx, mock.ANY, obj_mock)
self.tc.plugin.update_tags.assert_not_called()
notify.assert_not_called()

13
releasenotes/notes/Add-limit-of-the-tags-for-resources-23f825398d301d20.yaml Normal file
View File

@@ -0,0 +1,13 @@
---
upgrade:
- |
A limit on the number of tags per resource has been added. In case when
there are any resources with more than ``50`` tags created, it will not be
possible to create or modify existing tags for such resource until some of
them are deleted.
fixes:
- |
Fix bug `2091410 <https://bugs.launchpad.net/neutron/+bug/2091410>`_ which
could cause potential Denial of Service by adding a lot of tags to a single
resource, like, for example ``network``. Now there is a limit of ``50`` tags
for every resource.