From d1b6ebb4009e13ac2cf2309275a66a634e4f9171 Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Sat, 11 Jun 2011 16:42:58 -0400 Subject: [PATCH 1/7] Add ability to list ip blocks. --- nova/api/ec2/admin.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/nova/api/ec2/admin.py b/nova/api/ec2/admin.py index fcf7f674c652..37ae2e648b4b 100644 --- a/nova/api/ec2/admin.py +++ b/nova/api/ec2/admin.py @@ -369,3 +369,11 @@ class AdminController(object): 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']))} From ed3914eafa7d076fdcc03ee958f77528bcf20603 Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Sat, 11 Jun 2011 18:03:45 -0400 Subject: [PATCH 2/7] Add a method to delete provider firewall rules. --- nova/api/ec2/admin.py | 12 ++++++++++++ nova/db/api.py | 10 ++++++++++ nova/db/sqlalchemy/api.py | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/nova/api/ec2/admin.py b/nova/api/ec2/admin.py index 37ae2e648b4b..da982d8dc0b3 100644 --- a/nova/api/ec2/admin.py +++ b/nova/api/ec2/admin.py @@ -377,3 +377,15 @@ class AdminController(object): 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 + IPy.IP(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)} diff --git a/nova/db/api.py b/nova/db/api.py index 798b2881c193..62368c438504 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1004,6 +1004,16 @@ def provider_fw_rule_get_all(context): 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) + + +def provider_fw_rule_destroy(context, rule_id): + """Delete a provider firewall rule from the database.""" + return IMPL.provider_fw_rule_destroy(context) + + ################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9612e456bc0a..c8802e354586 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -2093,6 +2093,7 @@ def provider_fw_rule_create(context, rule): return fw_rule_ref +@require_admin_context def provider_fw_rule_get_all(context): session = get_session() return session.query(models.ProviderFirewallRule).\ @@ -2100,6 +2101,26 @@ def provider_fw_rule_get_all(context): 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')}) + + ################### From 0a6aeacfedfd5e666e109b54c5c03908eeb47c31 Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Sat, 11 Jun 2011 18:38:44 -0400 Subject: [PATCH 3/7] fix method chaining in database layer to pass right parameters. --- nova/db/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index 62368c438504..6ff6ec2f31ba 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1006,12 +1006,12 @@ def 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) + 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) + return IMPL.provider_fw_rule_destroy(context, rule_id) ################### From ac4baa5990c45a6a521a1786e680426ba617c65a Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Thu, 23 Jun 2011 14:13:27 -0400 Subject: [PATCH 4/7] Add test for listing provider firewall rules. --- nova/tests/test_adminapi.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nova/tests/test_adminapi.py b/nova/tests/test_adminapi.py index 7ecaf1c095ce..2b90d49e9975 100644 --- a/nova/tests/test_adminapi.py +++ b/nova/tests/test_adminapi.py @@ -87,3 +87,10 @@ class AdminApiTestCase(test.TestCase): result = self.api.block_external_addresses(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.""" + result = self.api.describe_external_address_blocks(self.context) + num = len(db.provider_fw_rule_get_all(self.context)) + # we only list IP, not tcp/udp/icmp rules + self.assertEqual(num / 3, len(result['externalIpBlockInfo'])) From 6e2ebfa1dc29e50f74f1b337d1b5349bc3c78cdc Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Thu, 23 Jun 2011 14:16:11 -0400 Subject: [PATCH 5/7] Make sure there are actually rules to test against. --- nova/tests/test_adminapi.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nova/tests/test_adminapi.py b/nova/tests/test_adminapi.py index 2b90d49e9975..4a96a3dd9f55 100644 --- a/nova/tests/test_adminapi.py +++ b/nova/tests/test_adminapi.py @@ -90,6 +90,7 @@ class AdminApiTestCase(test.TestCase): 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)) # we only list IP, not tcp/udp/icmp rules From 9a6e9a1af9359fb4a9261f59f57113f252f0d6e9 Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Thu, 23 Jun 2011 14:45:37 -0400 Subject: [PATCH 6/7] Make firewall rules tests idempotent, move IPy=>netaddr, add deltete test. --- nova/api/ec2/admin.py | 2 +- nova/tests/test_adminapi.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/nova/api/ec2/admin.py b/nova/api/ec2/admin.py index b8fc8f11498f..df7876b9d756 100644 --- a/nova/api/ec2/admin.py +++ b/nova/api/ec2/admin.py @@ -382,7 +382,7 @@ class AdminController(object): LOG.audit(_('Removing ip block from %s'), cidr, context=context) cidr = urllib.unquote(cidr).decode() # raise if invalid - IPy.IP(cidr) + 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']) diff --git a/nova/tests/test_adminapi.py b/nova/tests/test_adminapi.py index 4a96a3dd9f55..ce826fd5ba92 100644 --- a/nova/tests/test_adminapi.py +++ b/nova/tests/test_adminapi.py @@ -85,6 +85,7 @@ class AdminApiTestCase(test.TestCase): 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']) @@ -93,5 +94,18 @@ class AdminApiTestCase(test.TestCase): 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) From 51d93c5b1722bef9783cd7572c1464a084ece0aa Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Thu, 23 Jun 2011 14:52:58 -0400 Subject: [PATCH 7/7] libvirt test for deleting provider firewall rules. --- nova/tests/test_libvirt.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index ee94d3c171b0..d12e2106366f 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -1115,6 +1115,13 @@ class IptablesFirewallTestCase(test.TestCase): 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):