From 6d1479a8d998c5ecaeef1671de47323bbfc6fefc Mon Sep 17 00:00:00 2001 From: zhouhenglc Date: Tue, 8 Oct 2019 15:54:40 +0800 Subject: Fix recordset_records quota enforcement First I set recordset_records=3, and I can create recordset with 4 records successfully. Now enforce record quota by count records in database. when create recordset the number in database is 0., and quotas will not work no matter how much quota recordset_records are set. And once the excess quota is created successfully, it can not be updated. Unless quotas are updated. Closes-Bug: #1847200 Change-Id: If8bc6043d95f52f67899a5ac69a2f72c8fd4de17 (cherry picked from commit df10ff5b5c76657e4287b36fbf525ad91091dd34) --- designate/central/service.py | 8 +- designate/quota/base.py | 2 +- designate/tests/test_central/test_service.py | 4 +- designate/tests/test_quota/test_quota.py | 14 +-- designate/tests/unit/test_central/test_basic.py | 104 ++++++++++++++++++++- ...x-recordset-records-quota-76ed3095dd2afbbe.yaml | 4 + 6 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/Fix-recordset-records-quota-76ed3095dd2afbbe.yaml diff --git a/designate/central/service.py b/designate/central/service.py index ca851fd7..9d35585e 100644 --- a/designate/central/service.py +++ b/designate/central/service.py @@ -607,15 +607,17 @@ class Service(service.RPCService): criterion = {'tenant_id': tenant_id} count = self.storage.count_zones(context, criterion) - self.quota.limit_check(context, tenant_id, zones=count) + # Check if adding one more zone would exceed the quota + self.quota.limit_check(context, tenant_id, zones=count + 1) def _enforce_recordset_quota(self, context, zone): # Ensure the recordsets per zone quota is OK criterion = {'zone_id': zone.id} count = self.storage.count_recordsets(context, criterion) + # Check if adding one more recordset would exceed the quota self.quota.limit_check( - context, zone.tenant_id, zone_recordsets=count) + context, zone.tenant_id, zone_recordsets=count + 1) def _enforce_record_quota(self, context, zone, recordset): # Quotas don't apply to managed records. @@ -648,7 +650,7 @@ class Service(service.RPCService): # Ensure the records per recordset quota is OK self.quota.limit_check(context, zone.tenant_id, - recordset_records=recordset_records) + recordset_records=len(recordset.records)) # Misc Methods @rpc.expected_exceptions() diff --git a/designate/quota/base.py b/designate/quota/base.py index 45d4617a..1ac8c6e6 100644 --- a/designate/quota/base.py +++ b/designate/quota/base.py @@ -35,7 +35,7 @@ class Quota(DriverPlugin): if resource in quotas: # Setting the resource quota to a negative value will make # the resource unlimited - if quotas[resource] >= 0 and value >= quotas[resource]: + if quotas[resource] >= 0 and value > quotas[resource]: raise exceptions.OverQuota() else: raise exceptions.QuotaResourceUnknown("%s is not a valid quota" diff --git a/designate/tests/test_central/test_service.py b/designate/tests/test_central/test_service.py index a299946f..017ca702 100644 --- a/designate/tests/test_central/test_service.py +++ b/designate/tests/test_central/test_service.py @@ -1986,7 +1986,7 @@ class CentralServiceTest(CentralTestCase): def test_create_record_and_update_over_zone_quota(self): # SOA and NS Records exist - self.config(quota_zone_records=1) + self.config(quota_zone_records=0) # Creating the zone automatically creates SOA & NS records zone = self.create_zone() @@ -2023,7 +2023,7 @@ class CentralServiceTest(CentralTestCase): self.assertEqual(exceptions.OverQuota, exc.exc_info[0]) def test_create_record_over_recordset_quota(self): - self.config(quota_recordset_records=1) + self.config(quota_recordset_records=0) # Creating the zone automatically creates SOA & NS records zone = self.create_zone() diff --git a/designate/tests/test_quota/test_quota.py b/designate/tests/test_quota/test_quota.py index 039dbf7e..56074239 100644 --- a/designate/tests/test_quota/test_quota.py +++ b/designate/tests/test_quota/test_quota.py @@ -82,13 +82,13 @@ class QuotaTestCase(tests.TestCase): with testtools.ExpectedException(exceptions.OverQuota): self.quota.limit_check(context, 'tenant_id', - zones=cfg.CONF.quota_zones) + 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) + zone_records=cfg.CONF.quota_zone_records + 1) def test_limit_check_unlimited(self): context = self.get_admin_context() @@ -119,16 +119,16 @@ class QuotaTestCase(tests.TestCase): } self.quota.get_quotas.return_value = ret with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zones=0) + self.quota.limit_check(context, 'tenant_id', zones=1) with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zone_recordsets=0) + self.quota.limit_check(context, 'tenant_id', zone_recordsets=1) with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', zone_records=0) + self.quota.limit_check(context, 'tenant_id', zone_records=1) with testtools.ExpectedException(exceptions.OverQuota): self.quota.limit_check(context, 'tenant_id', - recordset_records=0) + recordset_records=1) with testtools.ExpectedException(exceptions.OverQuota): - self.quota.limit_check(context, 'tenant_id', api_export_size=0) + self.quota.limit_check(context, 'tenant_id', api_export_size=1) def test_limit_check_over(self): context = self.get_admin_context() diff --git a/designate/tests/unit/test_central/test_basic.py b/designate/tests/unit/test_central/test_basic.py index ec033843..8a3d1428 100644 --- a/designate/tests/unit/test_central/test_basic.py +++ b/designate/tests/unit/test_central/test_basic.py @@ -2191,8 +2191,16 @@ class CentralStatusTests(CentralBasic): class CentralQuotaTest(unittest.TestCase): def setUp(self): + self.CONF = cfg_fixture.Config(cfg.CONF) + cfg.CONF([], project='designate') + self.CONF.config(quota_driver="noop") self.context = mock.Mock() self.zone = mock.Mock() + self.quotas_of_one = {'zones': 1, + 'zone_recordsets': 1, + 'zone_records': 1, + 'recordset_records': 1, + 'api_export_size': 1} @patch('designate.central.service.storage') @patch('designate.central.service.quota') @@ -2220,6 +2228,100 @@ class CentralQuotaTest(unittest.TestCase): # Check the recordset limit as well check_recordset_records = mock.call( - self.context, self.zone.tenant_id, recordset_records=10 + self.context, self.zone.tenant_id, recordset_records=5 ) assert check_recordset_records in service.quota.limit_check.mock_calls + + @patch('designate.quota.base.Quota.get_quotas') + @patch('designate.central.service.storage') + def test_enforce_zone_quota(self, storage, mock_get_quotas): + service = Service() + mock_get_quotas.return_value = self.quotas_of_one + + # Test creating one zone, 1 quota, no existing zones + service.storage.count_zones.return_value = 0 + self.assertIsNone(service._enforce_zone_quota(self.context, + 'fake_project_id')) + + # Test creating one zone, 1 quota, one existing zone + service.storage.count_zones.return_value = 1 + self.assertRaises(exceptions.OverQuota, service._enforce_zone_quota, + self.context, 'fake_project_id') + + @patch('designate.quota.base.Quota.get_quotas') + @patch('designate.central.service.storage') + def test_enforce_recordset_quota(self, storage, mock_get_quotas): + service = Service() + mock_get_quotas.return_value = self.quotas_of_one + + # Test creating one recordset, 1 quota, no existing recordsets + service.storage.count_recordsets.return_value = 0 + self.assertIsNone(service._enforce_recordset_quota(self.context, + self.zone)) + + # Test creating one recordset, 1 quota, one existing recordset + service.storage.count_recordsets.return_value = 1 + self.assertRaises(exceptions.OverQuota, + service._enforce_recordset_quota, + self.context, self.zone) + + @patch('designate.quota.base.Quota.get_quotas') + @patch('designate.central.service.storage') + def test_enforce_record_quota(self, storage, mock_get_quotas): + service = Service() + mock_get_quotas.return_value = self.quotas_of_one + + service.storage.count_records.side_effect = [ + 0, 0, + 1, 0, + 0, 1, + 1, 1, + 1, 1, + ] + + managed_recordset = mock.Mock() + managed_recordset.managed = True + + recordset_one_record = mock.Mock() + recordset_one_record.managed = False + recordset_one_record.records = ['192.0.2.1'] + + # Test that managed recordsets have no quota limit + self.assertIsNone(service._enforce_record_quota(self.context, + self.zone, + managed_recordset)) + service.storage.count_records.assert_not_called() + + # Test creating recordset with one record, no existing zone records, + # no existing recordsets + self.assertIsNone(service._enforce_record_quota(self.context, + self.zone, + recordset_one_record)) + + # Test creating recordset with one record, one existing zone record, + # no exiting recordsets + self.assertRaises(exceptions.OverQuota, service._enforce_record_quota, + self.context, self.zone, recordset_one_record) + + # Test creating recordset with one record, one existing zone record, + # no exiting recordsets + # Note: Recordsets replace the existing recordset + self.assertIsNone(service._enforce_record_quota(self.context, + self.zone, + recordset_one_record)) + + # Test creating recordset with one record, no existing zone record, + # one exiting recordsets + # Note: Recordsets replace the existing recordset + self.assertIsNone(service._enforce_record_quota(self.context, + self.zone, + recordset_one_record)) + + recordset_two_record = mock.Mock() + recordset_two_record.managed = False + recordset_two_record.records = ['192.0.2.1', '192.0.2.2'] + + # Test creating recordset with two records, one existing zone record, + # one exiting recordsets + self.assertRaises(exceptions.OverQuota, service._enforce_record_quota, + self.context, self.zone, recordset_two_record) diff --git a/releasenotes/notes/Fix-recordset-records-quota-76ed3095dd2afbbe.yaml b/releasenotes/notes/Fix-recordset-records-quota-76ed3095dd2afbbe.yaml new file mode 100644 index 00000000..f1958cb9 --- /dev/null +++ b/releasenotes/notes/Fix-recordset-records-quota-76ed3095dd2afbbe.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Fixed an issue that caused the recordset_records quota to not be enforced. -- cgit v1.2.1