Implement rules_exist method for quantum security group driver

Originally I thought the quantum security group driver should
not enforce rules exist on the nova-api side and instead it should
just forward the request to quantum which would return the error.
That said there is no extra cost to doing this on the nova-api side
as nova-api already queries for the group before adding the rule.

In addition, rules_exists() is used in revoke_security_group_ingress()
for the ec2 compat APIs so this needs to be implemented.

This patch moves create_security_group_rule() and rule_exists() from
nova/compute/api.py to nova/network/security_group/security_group_base.py
as the same code can be leveraged in both places.

Fixes bug 1136345

Change-Id: I444ffc2b53b30ed496b6e3250433d14f316e594d

nova/compute/api.py

@@ -2987,21 +2987,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
         self.trigger_handler('instance_remove_security_group',
                 context, instance, security_group_name)
 
-    def rule_exists(self, security_group, new_rule):
-        """Indicates whether the specified rule is already
-           defined in the given security group.
-        """
-        for rule in security_group['rules']:
-            is_duplicate = True
-            keys = ('group_id', 'cidr', 'from_port', 'to_port', 'protocol')
-            for key in keys:
-                if rule.get(key) != new_rule.get(key):
-                    is_duplicate = False
-                    break
-            if is_duplicate:
-                return rule.get('id') or True
-        return False
-
     def get_rule(self, context, id):
         self.ensure_default(context)
         try:
@@ -3094,15 +3079,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
             msg = _("Security group id should be integer")
             self.raise_invalid_property(msg)
 
-    def create_security_group_rule(self, context, security_group, new_rule):
-        if self.rule_exists(security_group, new_rule):
-            msg = (_('This rule already exists in group %s') %
-                   new_rule['parent_group_id'])
-            self.raise_group_already_exists(msg)
-        return self.add_rules(context, new_rule['parent_group_id'],
-                             security_group['name'],
-                             [new_rule])[0]
-
     def trigger_handler(self, event, *args):
         handle = getattr(self.sgh, 'trigger_%s_refresh' % event)
         handle(*args)

nova/network/security_group/quantum_driver.py

@@ -215,10 +215,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
             new_rules.append(new_rule)
         return {'security_group_rules': new_rules}
 
-    def create_security_group_rule(self, context, security_group, new_rule):
-        return self.add_rules(context, new_rule['parent_group_id'],
-                              security_group['name'], [new_rule])[0]
-
     def remove_rules(self, context, security_group, rule_ids):
         quantum = quantumv2.get_client(context)
         rule_ids = set(rule_ids)
@@ -396,10 +392,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
                     'instance': instance['uuid']})
             self.raise_not_found(msg)
 
-    def rule_exists(self, security_group, new_rule):
-        # Handled by quantum
-        pass
-
     def populate_security_groups(self, instance, security_groups):
         # Setting to emply list since we do not want to populate this field
         # in the nova database if using the quantum driver

nova/network/security_group/security_group_base.py

@@ -132,6 +132,30 @@ class SecurityGroupBase(object):
 
         return values
 
+    def create_security_group_rule(self, context, security_group, new_rule):
+        if self.rule_exists(security_group, new_rule):
+            msg = (_('This rule already exists in group %s') %
+                   new_rule['parent_group_id'])
+            self.raise_group_already_exists(msg)
+        return self.add_rules(context, new_rule['parent_group_id'],
+                             security_group['name'],
+                             [new_rule])[0]
+
+    def rule_exists(self, security_group, new_rule):
+        """Indicates whether the specified rule is already
+           defined in the given security group.
+        """
+        for rule in security_group['rules']:
+            is_duplicate = True
+            keys = ('group_id', 'cidr', 'from_port', 'to_port', 'protocol')
+            for key in keys:
+                if rule.get(key) != new_rule.get(key):
+                    is_duplicate = False
+                    break
+            if is_duplicate:
+                return rule.get('id') or True
+        return False
+
     def validate_property(self, value, property, allowed):
         pass
 
@@ -174,9 +198,6 @@ class SecurityGroupBase(object):
     def add_rules(self, context, id, name, vals):
         raise NotImplementedError()
 
-    def create_security_group_rule(self, context, security_group, new_rule):
-        raise NotImplementedError()
-
     def remove_rules(self, context, security_group, rule_ids):
         raise NotImplementedError()
 
@@ -192,9 +213,6 @@ class SecurityGroupBase(object):
     def remove_from_instance(self, context, instance, security_group_name):
         raise NotImplementedError()
 
-    def rule_exists(self, security_group, new_rule):
-        raise NotImplementedError()
-
     @staticmethod
     def raise_invalid_property(msg):
         raise NotImplementedError()

nova/tests/api/openstack/compute/contrib/test_quantum_security_groups.py

@@ -237,6 +237,7 @@ class TestQuantumSecurityGroupRulesTestCase(TestQuantumSecurityGroupsTestCase):
         id2 = '22222222-2222-2222-2222-222222222222'
         sg_template2 = test_security_groups.security_group_template(
             security_group_rules=[], id=id2)
+        self.controller_sg = security_groups.SecurityGroupController()
         quantum = get_client()
         quantum._fake_security_groups[id1] = sg_template1
         quantum._fake_security_groups[id2] = sg_template2
@@ -252,12 +253,26 @@ class TestQuantumSecurityGroupRules(
         TestQuantumSecurityGroupRulesTestCase):
 
     def test_create_add_existing_rules_by_cidr(self):
-        # Enforced by quantum
+        sg = test_security_groups.security_group_template()
-        pass
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups')
+        self.controller_sg.create(req, {'security_group': sg})
+        rule = test_security_groups.security_group_rule_template(
+            cidr='15.0.0.0/8', parent_group_id=self.sg2['id'])
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-group-rules')
+        self.controller.create(req, {'security_group_rule': rule})
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
+                          req, {'security_group_rule': rule})
 
     def test_create_add_existing_rules_by_group_id(self):
-        # Enforced by quantum
+        sg = test_security_groups.security_group_template()
-        pass
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups')
+        self.controller_sg.create(req, {'security_group': sg})
+        rule = test_security_groups.security_group_rule_template(
+            group=self.sg1['id'], parent_group_id=self.sg2['id'])
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-group-rules')
+        self.controller.create(req, {'security_group_rule': rule})
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
+                          req, {'security_group_rule': rule})
 
     def test_delete(self):
         rule = test_security_groups.security_group_rule_template(
@@ -528,11 +543,15 @@ class MockClient(object):
 
     def show_security_group(self, security_group, **_params):
         try:
-            return {'security_group':
+            sg = self._fake_security_groups[security_group]
-                    self._fake_security_groups[security_group]}
         except KeyError:
             msg = 'Security Group %s not found' % security_group
             raise q_exc.QuantumClientException(message=msg, status_code=404)
+        for security_group_rule in self._fake_security_group_rules.values():
+            if security_group_rule['security_group_id'] == sg['id']:
+                sg['security_group_rules'].append(security_group_rule)
+
+        return {'security_group': sg}
 
     def show_security_group_rule(self, security_group_rule, **_params):
         try: