Makes security group rules with the newer version of the ec2 api and correctly supports boto 2.0

3 files changed, 66 insertions, 29 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 10720a804..0294c09c5 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -539,15 +539,18 @@ class CloudController(object):
             return rules
         if 'ip_ranges' in kwargs:
             rules = self._cidr_args_split(kwargs)
+        else:
+            rules = [kwargs]
         finalset = []
         for rule in rules:
             if 'groups' in rule:
                 groups_values = self._groups_args_split(rule)
                 for groups_value in groups_values:
-                    finalset.append(groups_value)
+                    final = self._rule_dict_last_step(context, **groups_value)
+                    finalset.append(final)
             else:
-                if rule:
-                    finalset.append(rule)
+                final = self._rule_dict_last_step(context, **rule)
+                finalset.append(final)
         return finalset
 
     def _cidr_args_split(self, kwargs):
@@ -590,6 +593,9 @@ class CloudController(object):
                     db.security_group_get_by_name(context.elevated(),
                                                   source_project_id,
                                                   source_security_group_name)
+            notfound = exception.SecurityGroupNotFound
+            if not source_security_group:
+                raise notfound(security_group_id=source_security_group_name)
             values['group_id'] = source_security_group['id']
         elif cidr_ip:
             # If this fails, it throws an exception. This is what we want.
@@ -628,7 +634,7 @@ class CloudController(object):
         for rule in security_group.rules:
             if 'group_id' in values:
                 if rule['group_id'] == values['group_id']:
-                    return True
+                    return rule['id']
             else:
                 is_duplicate = True
                 for key in ('cidr', 'from_port', 'to_port', 'protocol'):
@@ -636,7 +642,7 @@ class CloudController(object):
                         is_duplicate = False
                         break
                 if is_duplicate:
-                    return True
+                    return rule['id']
         return False
 
     def revoke_security_group_ingress(self, context, group_name=None,
@@ -659,22 +665,30 @@ class CloudController(object):
 
         msg = "Revoke security group ingress %s"
         LOG.audit(_(msg), security_group['name'], context=context)
+        prevalues = []
+        try:
+            prevalues = kwargs['ip_permissions']
+        except KeyError:
+            prevalues.append(kwargs)
+        rule_id = None
+        for values in prevalues:
+            rulesvalues = self._rule_args_to_dict(context, values)
+            if not rulesvalues:
+                err = "%s Not enough parameters to build a valid rule"
+                raise exception.ApiError(_(err % rulesvalues))
 
-        criteria = self._rule_args_to_dict(context, kwargs)[0]
-        if criteria is None:
-            raise exception.ApiError(_("Not enough parameters to build a "
-                                       "valid rule."))
-
-        for rule in security_group.rules:
-            match = True
-            for (k, v) in criteria.iteritems():
-                if getattr(rule, k, False) != v:
-                    match = False
-            if match:
-                db.security_group_rule_destroy(context, rule['id'])
-                self.compute_api.trigger_security_group_rules_refresh(context,
-                                        security_group_id=security_group['id'])
-                return True
+            for values_for_rule in rulesvalues:
+                values_for_rule['parent_group_id'] = security_group.id
+                rule_id = self._security_group_rule_exists(security_group,
+                                                           values_for_rule)
+                if rule_id:
+                    db.security_group_rule_destroy(context, rule_id)
+        if rule_id:
+            # NOTE(vish): we removed a rule, so refresh
+            self.compute_api.trigger_security_group_rules_refresh(
+                    context,
+                    security_group_id=security_group['id'])
+            return True
         raise exception.ApiError(_("No rule for the specified parameters."))
 
     # TODO(soren): This has only been tested with Boto as the client.
@@ -721,15 +735,17 @@ class CloudController(object):
                 postvalues.append(values_for_rule)
 
         for values_for_rule in postvalues:
-            security_group_rule = db.security_group_rule_create(context,
-                                                               values_for_rule)
+            security_group_rule = db.security_group_rule_create(
+                    context,
+                    values_for_rule)
 
-        self.compute_api.trigger_security_group_rules_refresh(context,
-                                  security_group_id=security_group['id'])
+        if postvalues:
+            self.compute_api.trigger_security_group_rules_refresh(
+                    context,
+                    security_group_id=security_group['id'])
+            return True
 
-        group = db.security_group_get_by_name(context, context.project_id,
-                                              security_group['name'])
-        return True
+        raise exception.ApiError(_("No rule for the specified parameters."))
 
     def _get_source_project_id(self, context, source_security_group_owner_id):
         if source_security_group_owner_id:
diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py
index 26ac5ff24..fe7fd8402 100644
--- a/nova/tests/test_api.py
+++ b/nova/tests/test_api.py
@@ -213,7 +213,11 @@ class ApiEc2TestCase(test.TestCase):
         self.http = FakeHttplibConnection(
                 self.app, '%s:8773' % (self.host), False)
         # pylint: disable=E1103
-        self.ec2.new_http_connection(host, is_secure).AndReturn(self.http)
+        if boto.Version >= '2':
+            self.ec2.new_http_connection(host or '%s:8773' % (self.host),
+                is_secure).AndReturn(self.http)
+        else:
+            self.ec2.new_http_connection(host, is_secure).AndReturn(self.http)
         return self.http
 
     def test_return_valid_isoformat(self):
diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py
index 136082cc1..f87edc407 100644
--- a/nova/tests/test_cloud.py
+++ b/nova/tests/test_cloud.py
@@ -287,7 +287,7 @@ class CloudTestCase(test.TestCase):
                                       'ip_protocol': u'tcp'}]}
         self.assertTrue(authz(self.context, group_name=sec['name'], **kwargs))
 
-    def test_authorize_security_group_ingress_ip_permissions_groups(self):
+    def test_authorize_security_group_fail_missing_source_group(self):
         kwargs = {'project_id': self.context.project_id, 'name': 'test'}
         sec = db.security_group_create(self.context, kwargs)
         authz = self.cloud.authorize_security_group_ingress
@@ -295,6 +295,23 @@ class CloudTestCase(test.TestCase):
                   'ip_ranges':{'1': {'cidr_ip': u'0.0.0.0/0'},
                                 '2': {'cidr_ip': u'10.10.10.10/32'}},
                   'groups': {'1': {'user_id': u'someuser',
+                                   'group_name': u'somegroup1'}},
+                  'ip_protocol': u'tcp'}]}
+        self.assertRaises(exception.SecurityGroupNotFound, authz,
+                          self.context, group_name=sec['name'], **kwargs)
+
+    def test_authorize_security_group_ingress_ip_permissions_groups(self):
+        kwargs = {'project_id': self.context.project_id, 'name': 'test'}
+        sec = db.security_group_create(self.context,
+                                       {'project_id': 'someuser',
+                                        'name': 'somegroup1'})
+        sec = db.security_group_create(self.context,
+                                       {'project_id': 'someuser',
+                                        'name': 'othergroup2'})
+        sec = db.security_group_create(self.context, kwargs)
+        authz = self.cloud.authorize_security_group_ingress
+        kwargs = {'ip_permissions': [{'to_port': 81, 'from_port': 81,
+                  'groups': {'1': {'user_id': u'someuser',
                                    'group_name': u'somegroup1'},
                              '2': {'user_id': u'someuser',
                                    'group_name': u'othergroup2'}},