Make calculate_usage() work if limits are missing

The calculate_usage interface was added recently to allow consumers
to probe limits and usage without requiring the enforce behavior
workflow. If a limit was passed to it that was not registered in
keystone, get_project_limits() would raise a ProjectOverLimit
exception itself to abort the process immediately, providing the
"unregistered means zero" behavior. This works fine for the enforce
workflow, but not the calculate one.

This changes get_project_limits() to just return a zero limit for
a missing one, which will be considered by the enforce workflow in
the same way, keeping the existing behavior. It will merely be
reported by the calculate workflow, which is the desired change.

Change-Id: Iaab1f0d5eb0da9a667267537d86f6c70bc8db51d

doc/source/user/usage.rst

@@ -146,7 +146,10 @@ Another usage pattern is to check a limit and usage for a given
 project, outside the scope of enforcement. This may be useful in a
 reporting API to be able to expose to a user the limit and usage
 information that the enforcer would use to judge a resource
-consumption event.
+consumption event. Any limit passed to this API which is not
+registered in keystone will be considered to be zero, in keeping with
+the behavior of the enforcer assuming that "unregistered means no
+quota."
 
 .. note::
    This should ideally not be used to provide your own enforcement of

oslo_limit/limit.py

@@ -280,29 +280,21 @@ class _EnforcerUtils(object):
     def get_project_limits(self, project_id, resource_names):
         """Get all the limits for given project a resource_name list
 
-        We will raise ClaimExceedsLimit if no limit is found to ensure that
+        If a limit is not found, it will be considered to be zero
-        all clients of this library react to this situation in the same way.
+        (i.e. no quota)
 
         :param project_id:
         :param resource_names: list of resource_name strings
         :return: list of (resource_name,limit) pairs
-
-        :raises exception.ClaimExceedsLimit: if no limit is found
         """
         # Using a list to preserver the resource_name order
         project_limits = []
-        missing_limits = []
         for resource_name in resource_names:
             try:
                 limit = self._get_limit(project_id, resource_name)
-                project_limits.append((resource_name, limit))
             except _LimitNotFound:
-                missing_limits.append(resource_name)
+                limit = 0
-
+            project_limits.append((resource_name, limit))
-        if len(missing_limits) > 0:
-            over_limit_list = [exception.OverLimitInfo(name, 0, 0, 0)
-                               for name in missing_limits]
-            raise exception.ProjectOverLimit(project_id, over_limit_list)
 
         return project_limits
 

oslo_limit/tests/test_limit.py

@@ -143,6 +143,38 @@ class TestEnforcer(base.BaseTestCase):
         self.assertEqual(expected, enforcer.calculate_usage(project_id,
                                                             ['a', 'b']))
 
+    @mock.patch.object(limit._EnforcerUtils, "_get_project_limit")
+    @mock.patch.object(limit._EnforcerUtils, "_get_registered_limit")
+    def test_calculate_and_enforce_some_missing(self, mock_get_reglimit,
+                                                mock_get_limit):
+        # Registered and project limits for a and b, c is unregistered
+        reg_limits = {'a': mock.MagicMock(default_limit=10),
+                      'b': mock.MagicMock(default_limit=10)}
+        prj_limits = {('bar', 'b'): mock.MagicMock(resource_limit=6)}
+        mock_get_reglimit.side_effect = lambda r: reg_limits.get(r)
+        mock_get_limit.side_effect = lambda p, r: prj_limits.get((p, r))
+
+        # Regardless, we have usage for all three
+        mock_usage = mock.MagicMock()
+        mock_usage.return_value = {'a': 5, 'b': 5, 'c': 5}
+
+        enforcer = limit.Enforcer(mock_usage)
+
+        # When we calculate usage, we should expect the default limit
+        # of zero for the unregistered limit
+        expected = {
+            'a': limit.ProjectUsage(10, 5),
+            'b': limit.ProjectUsage(6, 5),
+            'c': limit.ProjectUsage(0, 5),
+        }
+        self.assertEqual(expected,
+                         enforcer.calculate_usage('bar', ['a', 'b', 'c']))
+
+        # Make sure that if we enforce, we get the expected behavior
+        # of c being considered to be zero
+        self.assertRaises(exception.ProjectOverLimit,
+                          enforcer.enforce, 'bar', {'a': 1, 'b': 0, 'c': 1})
+
     def test_calculate_usage_bad_params(self):
         enforcer = limit.Enforcer(mock.MagicMock())
 
@@ -216,14 +248,17 @@ class TestFlatEnforcer(base.BaseTestCase):
         self.assertEqual(0, over_a.current_usage)
         self.assertEqual(2, over_a.delta)
 
-    @mock.patch.object(limit._EnforcerUtils, "get_project_limits")
+    @mock.patch.object(limit._EnforcerUtils, "_get_project_limit")
-    def test_enforce_raises_on_missing_limit(self, mock_get_limits):
+    @mock.patch.object(limit._EnforcerUtils, "_get_registered_limit")
-        mock_usage = mock.MagicMock()
+    def test_enforce_raises_on_missing_limit(self, mock_get_reglimit,
+                                             mock_get_limit):
+        def mock_usage(*a):
+            return {'a': 1, 'b': 1}
 
         project_id = uuid.uuid4().hex
         deltas = {"a": 0, "b": 0}
-        mock_get_limits.side_effect = exception.ProjectOverLimit(
+        mock_get_reglimit.return_value = None
-            project_id, [exception.OverLimitInfo("a", 0, 0, 0)])
+        mock_get_limit.return_value = None
 
         enforcer = limit._FlatEnforcer(mock_usage)
         self.assertRaises(exception.ProjectOverLimit, enforcer.enforce,
@@ -292,24 +327,8 @@ class TestEnforcerUtils(base.BaseTestCase):
         limits = utils.get_project_limits(project_id, ["a", "b"])
         self.assertEqual([('a', 1), ('b', 2)], limits)
 
-        e = self.assertRaises(exception.ProjectOverLimit,
+        limits = utils.get_project_limits(project_id, ["c", "d"])
-                              utils.get_project_limits,
+        self.assertEqual([('c', 0), ('d', 0)], limits)
-                              project_id, ["c", "d"])
-        expected = ("Project %s is over a limit for "
-                    "[Resource c is over limit of 0 due to current usage 0 "
-                    "and delta 0, "
-                    "Resource d is over limit of 0 due to current usage 0 "
-                    "and delta 0]")
-        self.assertEqual(expected % project_id, str(e))
-        self.assertEqual(project_id, e.project_id)
-        self.assertEqual(2, len(e.over_limit_info_list))
-        over_c = e.over_limit_info_list[0]
-        self.assertEqual("c", over_c.resource_name)
-        over_d = e.over_limit_info_list[1]
-        self.assertEqual("d", over_d.resource_name)
-        self.assertEqual(0, over_d.limit)
-        self.assertEqual(0, over_d.current_usage)
-        self.assertEqual(0, over_d.delta)
 
     def test_get_limit_cache(self, cache=True):
         # No project limit and registered limit = 5