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)

6 files changed, 122 insertions, 15 deletions

diff --git a/designate/central/service.py b/designate/central/service.py
index 4d504be5..35cd0e3c 100644
--- a/designate/central/service.py
+++ b/designate/central/service.py
@@ -606,15 +606,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.
@@ -647,7 +649,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 86dc553b..dc38c3a4 100644
--- a/designate/quota/base.py
+++ b/designate/quota/base.py
@@ -33,7 +33,7 @@ class Quota(DriverPlugin, metaclass=abc.ABCMeta):
             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 26694b9a..2c6f8cf5 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 b7711955..688ca340 100644
--- a/designate/tests/unit/test_central/test_basic.py
+++ b/designate/tests/unit/test_central/test_basic.py
@@ -2190,13 +2190,20 @@ 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')
     def test_zone_record_quota_allows_lowering_value(self, quota, storage):
-        cfg.CONF([], project='designate')
         service = Service()
         service.storage.count_records.return_value = 10
 
@@ -2220,6 +2227,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.