From 907a029625a7f95369ca5a2f5173efefdb04d28f Mon Sep 17 00:00:00 2001 From: Erik Olof Gunnar Andersson Date: Mon, 14 Feb 2022 18:17:12 -0800 Subject: Add proper quota error messages This patch adds a message to OverQuota containing information on the actual quota reached and fixes a minor visual bug with QuotaResourceUnknown. Change-Id: Ibd01f6ad3046ca29eec02032e9115183f89a6a4b --- designate/quota/base.py | 15 ++- designate/tests/test_quota/test_quota.py | 152 ++++++++++++++++++++----------- 2 files changed, 112 insertions(+), 55 deletions(-) diff --git a/designate/quota/base.py b/designate/quota/base.py index dc38c3a4..49a0be5b 100644 --- a/designate/quota/base.py +++ b/designate/quota/base.py @@ -27,6 +27,7 @@ class Quota(DriverPlugin, metaclass=abc.ABCMeta): __plugin_type__ = 'quota' def limit_check(self, context, tenant_id, **values): + resources_exceeded = [] quotas = self.get_quotas(context, tenant_id) for resource, value in values.items(): @@ -34,10 +35,18 @@ class Quota(DriverPlugin, metaclass=abc.ABCMeta): # Setting the resource quota to a negative value will make # the resource unlimited if quotas[resource] >= 0 and value > quotas[resource]: - raise exceptions.OverQuota() + resources_exceeded.append(resource) else: - raise exceptions.QuotaResourceUnknown("%s is not a valid quota" - " resource", resource) + raise exceptions.QuotaResourceUnknown( + "'%s' is not a valid quota resource." % resource + ) + + if resources_exceeded: + resources_exceeded.sort(key=len) + raise exceptions.OverQuota( + 'Quota exceeded for %s.' % + ', '.join(resources_exceeded) + ) def get_quotas(self, context, tenant_id): quotas = self.get_default_quotas(context) diff --git a/designate/tests/test_quota/test_quota.py b/designate/tests/test_quota/test_quota.py index 61f31ce5..0596ec65 100644 --- a/designate/tests/test_quota/test_quota.py +++ b/designate/tests/test_quota/test_quota.py @@ -18,13 +18,11 @@ from unittest import mock from oslo_config import cfg from oslo_log import log as logging from testscenarios import load_tests_apply_scenarios as load_tests # noqa -import testtools from designate import exceptions from designate import quota from designate import tests - LOG = logging.getLogger(__name__) @@ -56,11 +54,19 @@ class QuotaTestCase(tests.TestCase): def test_limit_check_unknown(self): context = self.get_admin_context() - with testtools.ExpectedException(exceptions.QuotaResourceUnknown): - self.quota.limit_check(context, 'tenant_id', unknown=0) + self.assertRaisesRegex( + exceptions.QuotaResourceUnknown, + "'unknown' is not a valid quota resource.", + self.quota.limit_check, + context, 'tenant_id', unknown=0 + ) - with testtools.ExpectedException(exceptions.QuotaResourceUnknown): - self.quota.limit_check(context, 'tenant_id', unknown=0, zones=0) + self.assertRaisesRegex( + exceptions.QuotaResourceUnknown, + "'unknown' is not a valid quota resource.", + self.quota.limit_check, + context, 'tenant_id', unknown=0, zones=0 + ) def test_limit_check_under(self): context = self.get_admin_context() @@ -80,25 +86,27 @@ class QuotaTestCase(tests.TestCase): def test_limit_check_at(self): context = self.get_admin_context() - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', - zones=cfg.CONF.quota_zones + 1) + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zones\\.', + self.quota.limit_check, + context, 'tenant_id', zones=cfg.CONF.quota_zones + 1 + ) - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check( - context, - 'tenant_id', - zone_records=cfg.CONF.quota_zone_records + 1) + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zone_records\\.', + self.quota.limit_check, + context, 'tenant_id', zone_records=cfg.CONF.quota_zone_records + 1 + ) def test_limit_check_unlimited(self): context = self.get_admin_context() self.quota.get_quotas = mock.Mock() ret = { - 'zones': -1, - 'zone_recordsets': -1, - 'zone_records': -1, - 'recordset_records': -1, - 'api_export_size': -1, + 'zones': -1, + 'zone_recordsets': -1, + 'zone_records': -1, + 'recordset_records': -1, + 'api_export_size': -1, } self.quota.get_quotas.return_value = ret self.quota.limit_check(context, 'tenant_id', zones=99999) @@ -111,42 +119,82 @@ class QuotaTestCase(tests.TestCase): context = self.get_admin_context() self.quota.get_quotas = mock.Mock() ret = { - 'zones': 0, - 'zone_recordsets': 0, - 'zone_records': 0, - 'recordset_records': 0, - 'api_export_size': 0, + 'zones': 0, + 'zone_recordsets': 0, + 'zone_records': 0, + 'recordset_records': 0, + 'api_export_size': 0, } self.quota.get_quotas.return_value = ret - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zones=1) - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zone_recordsets=1) - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zone_records=1) - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', - recordset_records=1) - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', api_export_size=1) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zones\\.', + self.quota.limit_check, + context, 'tenant_id', zones=1 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zone_recordsets\\.', + self.quota.limit_check, + context, 'tenant_id', zone_recordsets=1 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zone_records\\.', + self.quota.limit_check, + context, 'tenant_id', zone_records=1 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, + 'Quota exceeded for recordset_records\\.', + self.quota.limit_check, + context, 'tenant_id', recordset_records=1 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for api_export_size\\.', + self.quota.limit_check, + context, 'tenant_id', api_export_size=1 + ) def test_limit_check_over(self): context = self.get_admin_context() - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zones=99999) - - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zone_records=99999) - - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zones=99999, - zone_records=99999) - - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zones=99999, - zone_records=0) - - with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zones=0, - zone_records=99999) + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zones\\.', + self.quota.limit_check, + context, 'tenant_id', zones=99999 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zone_records\\.', + self.quota.limit_check, + context, 'tenant_id', zone_records=99999 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zones, zone_records\\.', + self.quota.limit_check, + context, 'tenant_id', zones=99999, zone_records=99999 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, + 'Quota exceeded for zones, zone_records, zone_recordsets\\.', + self.quota.limit_check, + context, 'tenant_id', zones=99999, zone_records=99999, + zone_recordsets=99999 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zones\\.', + self.quota.limit_check, + context, 'tenant_id', zones=99999, zone_records=0 + ) + + self.assertRaisesRegex( + exceptions.OverQuota, 'Quota exceeded for zone_records\\.', + self.quota.limit_check, + context, 'tenant_id', zones=0, zone_records=99999 + ) -- cgit v1.2.1