Add maximum number of l7rules per l7policy

For reasons of pure practicality, it is not a good idea to allow tenants
to create arbitrarily long lists of l7rules on their l7policies. After a
brief discussion we decided that 50 rules is good limit, especially
given that we expect most practical uses of L7 functionality to entail 1
to 4 rules per policy at most.

This commit also fixes a minor bug I noticed in the L7 rule API tests,
and cleans up the L7Rule API rule response specification.

Change-Id: I28b8161e85b9e86d4c44be3d48cbf94a3ce631f3
Closes-Bug: 1549100

octavia/api/v1/controllers/l7rule.py

@@ -72,6 +72,13 @@ class L7RuleController(base.BaseController):
             raise exceptions.ImmutableObject(resource=db_lb._name(),
                                              id=self.load_balancer_id)
 
+    def _check_l7policy_max_rules(self, session):
+        """Checks to make sure the L7Policy doesn't have too many rules."""
+        count = self.repositories.l7rule.count(
+            session, l7policy_id=self.l7policy_id)
+        if count >= constants.MAX_L7RULES_PER_L7POLICY:
+            raise exceptions.TooManyL7RulesOnL7Policy(id=self.l7policy_id)
+
     @wsme_pecan.wsexpose(l7rule_types.L7RuleResponse,
                          body=l7rule_types.L7RulePOST, status_code=202)
     def post(self, l7rule):
@@ -81,6 +88,7 @@ class L7RuleController(base.BaseController):
         except Exception as e:
             raise exceptions.L7RuleValidation(error=e)
         context = pecan.request.context.get('octavia_context')
+        self._check_l7policy_max_rules(context.session)
         l7rule_dict = db_prepare.create_l7rule(l7rule.to_dict(),
                                                self.l7policy_id)
         self._test_lb_and_listener_statuses(context.session)

octavia/api/v1/types/l7rule.py

@@ -21,10 +21,10 @@ from octavia.common import constants
 class L7RuleResponse(base.BaseType):
     """Defines which attributes are to be shown on any response."""
     id = wtypes.wsattr(wtypes.UuidType())
-    type = wtypes.wsattr(wtypes.StringType(max_length=255))
-    compare_type = wtypes.wsattr(wtypes.StringType(max_length=255))
-    key = wtypes.wsattr(wtypes.StringType(max_length=255))
-    value = wtypes.wsattr(wtypes.StringType(max_length=255))
+    type = wtypes.wsattr(wtypes.StringType())
+    compare_type = wtypes.wsattr(wtypes.StringType())
+    key = wtypes.wsattr(wtypes.StringType())
+    value = wtypes.wsattr(wtypes.StringType())
     invert = wtypes.wsattr(bool)
 
 

octavia/common/constants.py

@@ -112,6 +112,10 @@ SUPPORTED_L7POLICY_ACTIONS = (L7POLICY_ACTION_REJECT,
 # definitely be the case with 2147483647
 MAX_POLICY_POSITION = 2147483647
 
+# Testing showed haproxy config failed to parse after more than
+# 53 rules per policy
+MAX_L7RULES_PER_L7POLICY = 50
+
 URL_REGEX = (r'\Ahttp[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*(),]|'
              r'(?:%[0-9a-fA-F][0-9a-fA-F]))+')
 

octavia/common/exceptions.py

@@ -150,6 +150,11 @@ class ImmutableObject(APIException):
     code = 409
 
 
+class TooManyL7RulesOnL7Policy(APIException):
+    message = _("Too many rules on L7 policy %(id)s")
+    code = 409
+
+
 class ComputeBuildException(OctaviaException):
     message = _LE('Failed to build compute instance.')
 

octavia/db/prepare.py

@@ -93,6 +93,9 @@ def create_l7policy(l7policy_dict, lb_id, listener_id):
         l7policy_dict['redirect_pool'] = prepped_pool
         l7policy_dict['redirect_pool_id'] = prepped_pool['id']
     if l7policy_dict.get('l7rules'):
+        if (len(l7policy_dict.get('l7rules')) >
+                constants.MAX_L7RULES_PER_L7POLICY):
+            raise exceptions.TooManyL7RulesOnL7Policy(id=l7policy_dict['id'])
         prepped_l7rules = []
         for l7rule_dict in l7policy_dict.get('l7rules'):
             try:

octavia/tests/functional/api/v1/test_l7rule.py

@@ -228,7 +228,19 @@ class TestL7Rule(base.BaseAPITest):
                 'type': constants.L7RULE_TYPE_PATH,
                 'compare_type': constants.L7RULE_COMPARE_TYPE_STARTS_WITH,
                 'value': '/api'}
-        self.post(path, body, status=409, expect_errors=True)
+        self.post(path, body, status=409)
+
+    def test_create_too_many_rules(self):
+        for i in range(0, constants.MAX_L7RULES_PER_L7POLICY):
+            self.create_l7rule(
+                self.lb.get('id'), self.listener.get('id'),
+                self.l7policy.get('id'), constants.L7RULE_TYPE_PATH,
+                constants.L7RULE_COMPARE_TYPE_STARTS_WITH, '/api')
+            self.set_lb_status(self.lb.get('id'), constants.ACTIVE)
+        body = {'type': constants.L7RULE_TYPE_PATH,
+                'compare_type': constants.L7RULE_COMPARE_TYPE_STARTS_WITH,
+                'value': '/api'}
+        self.post(self.l7rules_path, body, status=409)
 
     def test_bad_create(self):
         l7rule = {'name': 'test1'}