Merge lp:~xtoddx/nova/provider-fw-rules-delete into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

Add api methods to delete provider firewall rules.

[Brian Lamar (blamar)]

Can you add tests for verification?

review: Needs Fixing

[Brian Waldon (bcwaldon)]

13: Looks like we may be getting rid of IPy soon. Would you mind using netaddr instead?

review: Needs Fixing

[Devin Carlen (devcamcar)]

The Brians comments addressed. :) lgtm

review: Approve

[Vish Ishaya (vishvananda)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

No proposals found for merge of lp:~xtoddx/nova/provider-fw-rules-list into lp:nova.

[Brian Waldon (bcwaldon)]

Looks good.

review: Approve

=== modified file 'nova/api/ec2/admin.py'
--- nova/api/ec2/admin.py	2011-06-14 18:34:03 +0000
+++ nova/api/ec2/admin.py	2011-06-25 02:03:45 +0000
@@ -369,3 +369,23 @@
             raise exception.ApiError(_('Duplicate rule'))
         self.compute_api.trigger_provider_fw_rules_refresh(context)
         return {'status': 'OK', 'message': 'Added %s rules' % rules_added}
+
+    def describe_external_address_blocks(self, context):
+        blocks = db.provider_fw_rule_get_all(context)
+        # NOTE(todd): use a set since we have icmp/udp/tcp rules with same cidr
+        blocks = set([b.cidr for b in blocks])
+        blocks = [{'cidr': b} for b in blocks]
+        return {'externalIpBlockInfo':
+                list(sorted(blocks, key=lambda k: k['cidr']))}
+
+    def remove_external_address_block(self, context, cidr):
+        LOG.audit(_('Removing ip block from %s'), cidr, context=context)
+        cidr = urllib.unquote(cidr).decode()
+        # raise if invalid
+        netaddr.IPNetwork(cidr)
+        rules = db.provider_fw_rule_get_all_by_cidr(context, cidr)
+        for rule in rules:
+            db.provider_fw_rule_destroy(context, rule['id'])
+        if rules:
+            self.compute_api.trigger_provider_fw_rules_refresh(context)
+        return {'status': 'OK', 'message': 'Deleted %s rules' % len(rules)}

=== modified file 'nova/db/api.py'
--- nova/db/api.py	2011-06-23 17:01:18 +0000
+++ nova/db/api.py	2011-06-25 02:03:45 +0000
@@ -1044,6 +1044,16 @@
     return IMPL.provider_fw_rule_get_all(context)
 
 
+def provider_fw_rule_get_all_by_cidr(context, cidr):
+    """Get all provider-level firewall rules."""
+    return IMPL.provider_fw_rule_get_all_by_cidr(context, cidr)
+
+
+def provider_fw_rule_destroy(context, rule_id):
+    """Delete a provider firewall rule from the database."""
+    return IMPL.provider_fw_rule_destroy(context, rule_id)
+
+
 ###################
 
 

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-06-23 17:01:18 +0000
+++ nova/db/sqlalchemy/api.py	2011-06-25 02:03:45 +0000
@@ -2189,6 +2189,7 @@
     return fw_rule_ref
 
 
+@require_admin_context
 def provider_fw_rule_get_all(context):
     session = get_session()
     return session.query(models.ProviderFirewallRule).\
@@ -2196,6 +2197,26 @@
                    all()
 
 
+@require_admin_context
+def provider_fw_rule_get_all_by_cidr(context, cidr):
+    session = get_session()
+    return session.query(models.ProviderFirewallRule).\
+                   filter_by(deleted=can_read_deleted(context)).\
+                   filter_by(cidr=cidr).\
+                   all()
+
+
+@require_admin_context
+def provider_fw_rule_destroy(context, rule_id):
+    session = get_session()
+    with session.begin():
+        session.query(models.ProviderFirewallRule).\
+                filter_by(id=rule_id).\
+                update({'deleted': True,
+                        'deleted_at': utils.utcnow(),
+                        'updated_at': literal_column('updated_at')})
+
+
 ###################
 
 

=== modified file 'nova/tests/test_adminapi.py'
--- nova/tests/test_adminapi.py	2011-06-23 17:59:26 +0000
+++ nova/tests/test_adminapi.py	2011-06-25 02:03:45 +0000
@@ -85,5 +85,27 @@
     def test_block_external_ips(self):
         """Make sure provider firewall rules are created."""
         result = self.api.block_external_addresses(self.context, '1.1.1.1/32')
+        self.api.remove_external_address_block(self.context, '1.1.1.1/32')
         self.assertEqual('OK', result['status'])
         self.assertEqual('Added 3 rules', result['message'])
+
+    def test_list_blocked_ips(self):
+        """Make sure we can see the external blocks that exist."""
+        self.api.block_external_addresses(self.context, '1.1.1.2/32')
+        result = self.api.describe_external_address_blocks(self.context)
+        num = len(db.provider_fw_rule_get_all(self.context))
+        self.api.remove_external_address_block(self.context, '1.1.1.2/32')
+        # we only list IP, not tcp/udp/icmp rules
+        self.assertEqual(num / 3, len(result['externalIpBlockInfo']))
+
+    def test_remove_ip_block(self):
+        """Remove ip blocks."""
+        result = self.api.block_external_addresses(self.context, '1.1.1.3/32')
+        self.assertEqual('OK', result['status'])
+        num0 = len(db.provider_fw_rule_get_all(self.context))
+        result = self.api.remove_external_address_block(self.context,
+                                                        '1.1.1.3/32')
+        self.assertEqual('OK', result['status'])
+        self.assertEqual('Deleted 3 rules', result['message'])
+        num1 = len(db.provider_fw_rule_get_all(self.context))
+        self.assert_(num1 < num0)

=== modified file 'nova/tests/test_libvirt.py'
--- nova/tests/test_libvirt.py	2011-06-11 19:14:46 +0000
+++ nova/tests/test_libvirt.py	2011-06-25 02:03:45 +0000
@@ -1115,6 +1115,13 @@
                 provjump_rules.append(rule)
         self.assertEqual(1, len(provjump_rules))
 
+        # remove a rule from the db, cast to compute to refresh rule
+        db.provider_fw_rule_destroy(admin_ctxt, provider_fw1['id'])
+        self.fw.refresh_provider_fw_rules()
+        rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules
+                      if rule.chain == 'provider']
+        self.assertEqual(1, len(rules))
+
 
 class NWFilterTestCase(test.TestCase):
     def setUp(self):