From 85473ced07d06c42f7a544537dcca1251fd8e278 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Mon, 3 Nov 2014 14:33:17 -0600 Subject: [PATCH 01/14] Cameron and Ed | Add rule to security group on EC2 when not first rule in group --- rule_refresher.py | 31 +++++++++++++++ tests/test_rule_refresher.py | 74 ++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 rule_refresher.py create mode 100644 tests/test_rule_refresher.py diff --git a/rule_refresher.py b/rule_refresher.py new file mode 100644 index 0000000..813bc58 --- /dev/null +++ b/rule_refresher.py @@ -0,0 +1,31 @@ +class RuleRefresher: + + def __init__(self, openstack_group_manager, ec2_conn, rule_comparator): + self.openstack_group_manager = openstack_group_manager + self.ec2_conn = ec2_conn + self.rule_comparator = rule_comparator + + def refresh(self, openstack_instance): + openstack_group = self.openstack_group_manager.list()[0] + openstack_rules = openstack_group.rules + + ec2_group = self.ec2_conn.get_all_security_groups()[0] + ec2_rules = ec2_group.rules + + for openstack_rule in openstack_rules: + same_rule_exists_on_ec2 = False + for ec2_rule in ec2_rules: + if self.rule_comparator.rules_are_equal(openstack_rule, ec2_rule): + same_rule_exists_on_ec2 = True + break + + if not same_rule_exists_on_ec2: + self.ec2_conn.authorize_security_group( + group_name=openstack_group.name, + ip_protocol=openstack_rule['ip_protocol'], + from_port=openstack_rule['from_port'], + to_port=openstack_rule['to_port'], + cidr_ip=openstack_rule['ip_range']['cidr'] + ) + + diff --git a/tests/test_rule_refresher.py b/tests/test_rule_refresher.py new file mode 100644 index 0000000..a49c0ff --- /dev/null +++ b/tests/test_rule_refresher.py @@ -0,0 +1,74 @@ +import unittest + +import boto + +from boto.ec2 import EC2Connection +from mock import Mock +import novaclient +from novaclient.v1_1.servers import Server + +from nova.virt.ec2.rule_refresher import RuleRefresher +from nova.virt.ec2.rule_comparator import RuleComparator + +GROUP_NAME = 'secGroup' + +class TestRuleRefresher(unittest.TestCase): + def setUp(self): + self.new_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}} + + self.openstack_group = Mock() + self.openstack_group.name = GROUP_NAME + + self.ec2_group = Mock() + self.ec2_group.name = GROUP_NAME + + self.openstack_instance = Mock() + self.openstack_instance.security_groups = [{'name': GROUP_NAME}] + + self.openstack_group_manager = Mock(spec=novaclient.v1_1.security_groups.SecurityGroupManager) + self.openstack_group_manager.list.return_value = [self.openstack_group] + + self.ec2_connection = Mock(spec=EC2Connection) + self.ec2_connection.get_all_security_groups.return_value = [self.ec2_group] + + self.rule_comparator = Mock(spec=RuleComparator) + + self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection, self.rule_comparator) + + def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self): + self.openstack_group.rules = [self.new_rule] + self.ec2_group.rules = [] + + self.rule_refresher.refresh(self.openstack_instance) + + self.ec2_connection.authorize_security_group.assert_called_once_with( + group_name=GROUP_NAME, + ip_protocol='abc', + from_port=1111, + to_port=2222, + cidr_ip="1.2.3.4/55" + ) + + def test_should_add_rule_to_ec2_security_group_when_other_rule_already_on_both(self): + existing_rule = {'ip_protocol': 'hi', 'from_port': 3333, 'to_port': 4444, 'ip_range': {'cidr': '6.7.8.9/00'}} + existing_ec2_rule = {'attribute': 'value'} + + self.openstack_group.rules = [ + existing_rule, + self.new_rule + ] + self.ec2_group.rules = [existing_ec2_rule] + + def mock_rules_are_equal(openstack_rule, ec2_rule): + return openstack_rule == existing_rule + self.rule_comparator.rules_are_equal.side_effect = mock_rules_are_equal + + self.rule_refresher.refresh(self.openstack_instance) + + self.ec2_connection.authorize_security_group.assert_called_once_with( + group_name=GROUP_NAME, + ip_protocol=self.new_rule['ip_protocol'], + from_port=self.new_rule['from_port'], + to_port=self.new_rule['to_port'], + cidr_ip=self.new_rule['ip_range']['cidr'] + ) \ No newline at end of file From 6f2fc65bf48bacc30f31c4fbb91e12eb8499ff14 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Mon, 3 Nov 2014 17:05:37 -0600 Subject: [PATCH 02/14] Ed & Cameron | Common rule class for comparing openstack and ec2 rules --- ec2_rule_transformer.py | 18 ++++++++++++++++++ openstack_rule_transformer.py | 15 +++++++++++++++ rule.py | 16 ++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 ec2_rule_transformer.py create mode 100644 openstack_rule_transformer.py create mode 100644 rule.py diff --git a/ec2_rule_transformer.py b/ec2_rule_transformer.py new file mode 100644 index 0000000..64bb25f --- /dev/null +++ b/ec2_rule_transformer.py @@ -0,0 +1,18 @@ +from copy import deepcopy +from rule import Rule + + +class EC2RuleTransformer: + + def __init__(self, ec2_connection): + self.ec2_connection = ec2_connection + + def to_rule(self, ec2_rule): + rule_args = deepcopy(ec2_rule) + + if ec2_rule.grants[0].cidr_ip: + rule_args['ip_range'] = ec2_rule.grants[0].cidr_ip + else: + group_id = ec2_rule.grants[0].group_id + rule_args['group_name'] = self.ec2_connection.get_all_security_groups(group_ids=group_id)[0] + return Rule(**ec2_rule) \ No newline at end of file diff --git a/openstack_rule_transformer.py b/openstack_rule_transformer.py new file mode 100644 index 0000000..562e0c8 --- /dev/null +++ b/openstack_rule_transformer.py @@ -0,0 +1,15 @@ +from copy import deepcopy +from rule import Rule + + +class OpenStackRuleTransformer: + + def to_rule(self, openstack_rule): + rule_args = deepcopy(openstack_rule) + + if 'cidr' in openstack_rule['ip_range']: + rule_args['ip_range'] = openstack_rule['ip_range']['cidr'] + else: + rule_args['group_name'] = openstack_rule['group']['name'] + + return Rule(**openstack_rule) \ No newline at end of file diff --git a/rule.py b/rule.py new file mode 100644 index 0000000..fc01ac8 --- /dev/null +++ b/rule.py @@ -0,0 +1,16 @@ +class Rule: + def __init__(self, ip_protocol, from_port, to_port, ip_range=None, group_name=None): + self.ip_protocol = ip_protocol + self.from_port = from_port + self.to_port = to_port + self.ip_range = ip_range + self.group_name = group_name + + def __key(self): + return self.ip_protocol, self.from_port, self.to_port, self.ip_range, self.group_name + + def __eq__(self, other): + return self.__key() == other.__key() + + def __hash__(self): + return hash(self.__key()) \ No newline at end of file From d1265791817225a72f9485905d05c3b638226204 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Tue, 4 Nov 2014 11:35:04 -0600 Subject: [PATCH 03/14] Cameron & Ed | Test EC2 rule transformer --- ec2_rule_transformer.py | 8 +++--- tests/test_ec2_rule_transformer.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 tests/test_ec2_rule_transformer.py diff --git a/ec2_rule_transformer.py b/ec2_rule_transformer.py index 64bb25f..d6731f8 100644 --- a/ec2_rule_transformer.py +++ b/ec2_rule_transformer.py @@ -8,11 +8,13 @@ class EC2RuleTransformer: self.ec2_connection = ec2_connection def to_rule(self, ec2_rule): - rule_args = deepcopy(ec2_rule) + rule_args = deepcopy(vars(ec2_rule)) + del rule_args['grants'] if ec2_rule.grants[0].cidr_ip: rule_args['ip_range'] = ec2_rule.grants[0].cidr_ip else: group_id = ec2_rule.grants[0].group_id - rule_args['group_name'] = self.ec2_connection.get_all_security_groups(group_ids=group_id)[0] - return Rule(**ec2_rule) \ No newline at end of file + rule_args['group_name'] = self.ec2_connection.get_all_security_groups(group_ids=group_id)[0].name + + return Rule(**rule_args) \ No newline at end of file diff --git a/tests/test_ec2_rule_transformer.py b/tests/test_ec2_rule_transformer.py new file mode 100644 index 0000000..c407e47 --- /dev/null +++ b/tests/test_ec2_rule_transformer.py @@ -0,0 +1,40 @@ +from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer +from mock import Mock +from fake_ec2_rule_builder import FakeEC2RuleBuilder + +import unittest + + + +class TestEC2RuleTransformer(unittest.TestCase): + + def setUp(self): + self.ec2_connection = Mock() + self.ec2_rule_transformer = EC2RuleTransformer(self.ec2_connection) + + def test_should_copy_ip_protocol_and_port_attributes(self): + ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build() + + rule = self.ec2_rule_transformer.to_rule(ec2_rule) + + self.assertEqual(rule.ip_protocol, ec2_rule.ip_protocol) + self.assertEqual(rule.from_port, ec2_rule.from_port) + self.assertEqual(rule.to_port, ec2_rule.to_port) + + def test_should_copy_ip_range_attribute_from_grant(self): + ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().with_ip_range('5.6.7.8/90').build() + + rule = self.ec2_rule_transformer.to_rule(ec2_rule) + + self.assertEqual(rule.ip_range, ec2_rule.grants[0].cidr_ip) + + def test_should_set_group_name_from_grant(self): + ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().with_allowed_security_group_id(123).build() + + ec2_group = Mock() + ec2_group.name = 'secGroup' + self.ec2_connection.get_all_security_groups.return_value = [ec2_group] + + rule = self.ec2_rule_transformer.to_rule(ec2_rule) + + self.assertEqual(rule.group_name, 'secGroup') \ No newline at end of file From 3751616fbfa7e583c088c46f6834f8dae0e7ba80 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Tue, 4 Nov 2014 13:39:03 -0600 Subject: [PATCH 04/14] Ed & Cameron | Refactoring: replace rule comparator with transformers and rule class equality method in rule refresher --- openstack_rule_transformer.py | 2 +- rule_refresher.py | 43 +++++++++--------- tests/test_ec2_rule_transformer.py | 13 ++++-- tests/test_rule_refresher.py | 71 +++++++++++++++++++++++------- 4 files changed, 86 insertions(+), 43 deletions(-) diff --git a/openstack_rule_transformer.py b/openstack_rule_transformer.py index 562e0c8..b78ed4e 100644 --- a/openstack_rule_transformer.py +++ b/openstack_rule_transformer.py @@ -2,7 +2,7 @@ from copy import deepcopy from rule import Rule -class OpenStackRuleTransformer: +class OpenstackRuleTransformer: def to_rule(self, openstack_rule): rule_args = deepcopy(openstack_rule) diff --git a/rule_refresher.py b/rule_refresher.py index 813bc58..45477e0 100644 --- a/rule_refresher.py +++ b/rule_refresher.py @@ -1,31 +1,28 @@ class RuleRefresher: - def __init__(self, openstack_group_manager, ec2_conn, rule_comparator): + def __init__(self, openstack_group_manager, ec2_conn, openstack_rule_transformer, ec2_rule_transformer): self.openstack_group_manager = openstack_group_manager self.ec2_conn = ec2_conn - self.rule_comparator = rule_comparator + self.openstack_rule_transformer = openstack_rule_transformer + self.ec2_rule_transformer = ec2_rule_transformer def refresh(self, openstack_instance): - openstack_group = self.openstack_group_manager.list()[0] - openstack_rules = openstack_group.rules - - ec2_group = self.ec2_conn.get_all_security_groups()[0] - ec2_rules = ec2_group.rules - - for openstack_rule in openstack_rules: - same_rule_exists_on_ec2 = False - for ec2_rule in ec2_rules: - if self.rule_comparator.rules_are_equal(openstack_rule, ec2_rule): - same_rule_exists_on_ec2 = True - break - - if not same_rule_exists_on_ec2: - self.ec2_conn.authorize_security_group( - group_name=openstack_group.name, - ip_protocol=openstack_rule['ip_protocol'], - from_port=openstack_rule['from_port'], - to_port=openstack_rule['to_port'], - cidr_ip=openstack_rule['ip_range']['cidr'] - ) + for group_dict in openstack_instance.security_groups: + openstack_group = [group for group in self.openstack_group_manager.list() if group.name == group_dict['name']][0] + ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0] + for openstack_rule in openstack_group.rules: + same_rule_exists_on_ec2 = False + for ec2_rule in ec2_group.rules: + if self.openstack_rule_transformer.to_rule(openstack_rule) == self.ec2_rule_transformer.to_rule(ec2_rule): + same_rule_exists_on_ec2 = True + break + if not same_rule_exists_on_ec2: + self.ec2_conn.authorize_security_group( + group_name=openstack_group.name, + ip_protocol=openstack_rule['ip_protocol'], + from_port=openstack_rule['from_port'], + to_port=openstack_rule['to_port'], + cidr_ip=openstack_rule['ip_range']['cidr'] + ) \ No newline at end of file diff --git a/tests/test_ec2_rule_transformer.py b/tests/test_ec2_rule_transformer.py index c407e47..b136dfe 100644 --- a/tests/test_ec2_rule_transformer.py +++ b/tests/test_ec2_rule_transformer.py @@ -5,20 +5,25 @@ from fake_ec2_rule_builder import FakeEC2RuleBuilder import unittest - class TestEC2RuleTransformer(unittest.TestCase): def setUp(self): self.ec2_connection = Mock() self.ec2_rule_transformer = EC2RuleTransformer(self.ec2_connection) - def test_should_copy_ip_protocol_and_port_attributes(self): + def test_should_copy_ip_protocol(self): ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build() - rule = self.ec2_rule_transformer.to_rule(ec2_rule) - self.assertEqual(rule.ip_protocol, ec2_rule.ip_protocol) + + def test_should_copy_from_port(self): + ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build() + rule = self.ec2_rule_transformer.to_rule(ec2_rule) self.assertEqual(rule.from_port, ec2_rule.from_port) + + def test_should_copy_to_port(self): + ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build() + rule = self.ec2_rule_transformer.to_rule(ec2_rule) self.assertEqual(rule.to_port, ec2_rule.to_port) def test_should_copy_ip_range_attribute_from_grant(self): diff --git a/tests/test_rule_refresher.py b/tests/test_rule_refresher.py index a49c0ff..8b118f9 100644 --- a/tests/test_rule_refresher.py +++ b/tests/test_rule_refresher.py @@ -8,13 +8,15 @@ import novaclient from novaclient.v1_1.servers import Server from nova.virt.ec2.rule_refresher import RuleRefresher -from nova.virt.ec2.rule_comparator import RuleComparator +from nova.virt.ec2.openstack_rule_transformer import OpenstackRuleTransformer +from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer +from nova.virt.ec2.rule import Rule GROUP_NAME = 'secGroup' class TestRuleRefresher(unittest.TestCase): def setUp(self): - self.new_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}} + self.existing_new_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}} self.openstack_group = Mock() self.openstack_group.name = GROUP_NAME @@ -31,12 +33,14 @@ class TestRuleRefresher(unittest.TestCase): self.ec2_connection = Mock(spec=EC2Connection) self.ec2_connection.get_all_security_groups.return_value = [self.ec2_group] - self.rule_comparator = Mock(spec=RuleComparator) + self.openstack_rule_transformer = Mock(spec=OpenstackRuleTransformer) + self.ec2_rule_transformer = Mock(spec=EC2RuleTransformer) - self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection, self.rule_comparator) + self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection, + self.openstack_rule_transformer, self.ec2_rule_transformer) def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self): - self.openstack_group.rules = [self.new_rule] + self.openstack_group.rules = [self.existing_new_rule] self.ec2_group.rules = [] self.rule_refresher.refresh(self.openstack_instance) @@ -50,25 +54,62 @@ class TestRuleRefresher(unittest.TestCase): ) def test_should_add_rule_to_ec2_security_group_when_other_rule_already_on_both(self): - existing_rule = {'ip_protocol': 'hi', 'from_port': 3333, 'to_port': 4444, 'ip_range': {'cidr': '6.7.8.9/00'}} + existing_openstack_rule = {'ip_protocol': 'hi', 'from_port': 3333, 'to_port': 4444, 'ip_range': {'cidr': '6.7.8.9/00'}} existing_ec2_rule = {'attribute': 'value'} + existing_transformed_rule = Rule('sdfg', 5, 6, '7.7.7.7/77') + new_transformed_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') self.openstack_group.rules = [ - existing_rule, - self.new_rule + existing_openstack_rule, + self.existing_new_rule ] self.ec2_group.rules = [existing_ec2_rule] - def mock_rules_are_equal(openstack_rule, ec2_rule): - return openstack_rule == existing_rule - self.rule_comparator.rules_are_equal.side_effect = mock_rules_are_equal + def mock_openstack_to_rule(openstack_rule): + return existing_transformed_rule if openstack_rule == existing_openstack_rule else new_transformed_rule + + def mock_ec2_to_rule(ec2_rule): + if ec2_rule == existing_ec2_rule: + return existing_transformed_rule + + self.openstack_rule_transformer.to_rule.side_effect = mock_openstack_to_rule + self.ec2_rule_transformer.to_rule.side_effect = mock_ec2_to_rule self.rule_refresher.refresh(self.openstack_instance) self.ec2_connection.authorize_security_group.assert_called_once_with( group_name=GROUP_NAME, - ip_protocol=self.new_rule['ip_protocol'], - from_port=self.new_rule['from_port'], - to_port=self.new_rule['to_port'], - cidr_ip=self.new_rule['ip_range']['cidr'] + ip_protocol=self.existing_new_rule['ip_protocol'], + from_port=self.existing_new_rule['from_port'], + to_port=self.existing_new_rule['to_port'], + cidr_ip=self.existing_new_rule['ip_range']['cidr'] + ) + + def test_should_add_rule_to_corresponding_ec2_group_when_other_groups_present(self): + openstack_group2 = Mock() + openstack_group2.name = "group2" + ec2_group2 = Mock() + ec2_group2.rules = [] + self.ec2_group.rules = [] + + self.openstack_group.rules = [self.existing_new_rule] + openstack_group2.rules = [] + self.openstack_instance.security_groups = [{'name': GROUP_NAME}, {'name': openstack_group2.name}] + + self.openstack_group_manager.list.return_value = [openstack_group2, self.openstack_group] + + def mock_get_all_security_groups(groupnames=None): + if groupnames == ec2_group2.name: + return [ec2_group2] + return [self.ec2_group] + self.ec2_connection.get_all_security_groups.side_effect = mock_get_all_security_groups + + self.rule_refresher.refresh(self.openstack_instance) + + self.ec2_connection.authorize_security_group.assert_called_once_with( + group_name=GROUP_NAME, + ip_protocol=self.existing_new_rule['ip_protocol'], + from_port=self.existing_new_rule['from_port'], + to_port=self.existing_new_rule['to_port'], + cidr_ip=self.existing_new_rule['ip_range']['cidr'] ) \ No newline at end of file From e5ae501335201ceefbacf0c9a7a11fdac2c448a7 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Tue, 4 Nov 2014 14:59:41 -0600 Subject: [PATCH 05/14] Ed & Cameron | Refactoring: extract rule comparison between openstack and ec2 groups to common group class --- ec2_group_transformer.py | 4 ++ group.py | 4 ++ openstack_group_transformer.py | 3 + rule_refresher.py | 35 +++++----- tests/test_rule_refresher.py | 114 +++++++++++++-------------------- 5 files changed, 75 insertions(+), 85 deletions(-) create mode 100644 ec2_group_transformer.py create mode 100644 group.py create mode 100644 openstack_group_transformer.py diff --git a/ec2_group_transformer.py b/ec2_group_transformer.py new file mode 100644 index 0000000..5674d42 --- /dev/null +++ b/ec2_group_transformer.py @@ -0,0 +1,4 @@ +class EC2GroupTransformer: + + def to_group(self, ec2_group): + pass \ No newline at end of file diff --git a/group.py b/group.py new file mode 100644 index 0000000..978cfd5 --- /dev/null +++ b/group.py @@ -0,0 +1,4 @@ +class Group: + + def rule_diff(self, other_group): + pass \ No newline at end of file diff --git a/openstack_group_transformer.py b/openstack_group_transformer.py new file mode 100644 index 0000000..c1b2ad2 --- /dev/null +++ b/openstack_group_transformer.py @@ -0,0 +1,3 @@ +class OpenstackGroupTransformer: + def to_group(self, openstack_group): + pass \ No newline at end of file diff --git a/rule_refresher.py b/rule_refresher.py index 45477e0..73d7fd9 100644 --- a/rule_refresher.py +++ b/rule_refresher.py @@ -1,28 +1,29 @@ class RuleRefresher: - def __init__(self, openstack_group_manager, ec2_conn, openstack_rule_transformer, ec2_rule_transformer): + def __init__(self, openstack_group_manager, ec2_conn, openstack_group_transformer, ec2_group_transformer): self.openstack_group_manager = openstack_group_manager self.ec2_conn = ec2_conn - self.openstack_rule_transformer = openstack_rule_transformer - self.ec2_rule_transformer = ec2_rule_transformer + self.openstack_group_transformer = openstack_group_transformer + self.ec2_group_transformer = ec2_group_transformer def refresh(self, openstack_instance): for group_dict in openstack_instance.security_groups: openstack_group = [group for group in self.openstack_group_manager.list() if group.name == group_dict['name']][0] + transformed_openstack_group = self.openstack_group_transformer.to_group(openstack_group) + ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0] + transformed_ec2_group = self.ec2_group_transformer.to_group(ec2_group) - for openstack_rule in openstack_group.rules: - same_rule_exists_on_ec2 = False - for ec2_rule in ec2_group.rules: - if self.openstack_rule_transformer.to_rule(openstack_rule) == self.ec2_rule_transformer.to_rule(ec2_rule): - same_rule_exists_on_ec2 = True - break + rules_in_openstack_group_not_in_ec2 = transformed_openstack_group.rule_diff(transformed_ec2_group) - if not same_rule_exists_on_ec2: - self.ec2_conn.authorize_security_group( - group_name=openstack_group.name, - ip_protocol=openstack_rule['ip_protocol'], - from_port=openstack_rule['from_port'], - to_port=openstack_rule['to_port'], - cidr_ip=openstack_rule['ip_range']['cidr'] - ) \ No newline at end of file + for rule in rules_in_openstack_group_not_in_ec2: + self._create_rule_on_ec2(openstack_group, rule) + + def _create_rule_on_ec2(self, openstack_group, rule): + self.ec2_conn.authorize_security_group( + group_name=openstack_group.name, + ip_protocol=rule.ip_protocol, + from_port=rule.from_port, + to_port=rule.to_port, + cidr_ip=rule.ip_range + ) \ No newline at end of file diff --git a/tests/test_rule_refresher.py b/tests/test_rule_refresher.py index 8b118f9..e4bddee 100644 --- a/tests/test_rule_refresher.py +++ b/tests/test_rule_refresher.py @@ -1,115 +1,93 @@ import unittest -import boto - from boto.ec2 import EC2Connection from mock import Mock import novaclient from novaclient.v1_1.servers import Server from nova.virt.ec2.rule_refresher import RuleRefresher -from nova.virt.ec2.openstack_rule_transformer import OpenstackRuleTransformer -from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer from nova.virt.ec2.rule import Rule +from nova.virt.ec2.ec2_group_transformer import EC2GroupTransformer +from nova.virt.ec2.openstack_group_transformer import OpenstackGroupTransformer +from nova.virt.ec2.group import Group GROUP_NAME = 'secGroup' +OTHER_GROUP_NAME = "otherSecGroup" class TestRuleRefresher(unittest.TestCase): def setUp(self): - self.existing_new_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}} - - self.openstack_group = Mock() - self.openstack_group.name = GROUP_NAME - - self.ec2_group = Mock() - self.ec2_group.name = GROUP_NAME + self.new_openstack_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}} self.openstack_instance = Mock() self.openstack_instance.security_groups = [{'name': GROUP_NAME}] + self.openstack_group = Mock() + self.openstack_group.name = GROUP_NAME self.openstack_group_manager = Mock(spec=novaclient.v1_1.security_groups.SecurityGroupManager) self.openstack_group_manager.list.return_value = [self.openstack_group] + self.ec2_group = Mock() + self.ec2_group.name = GROUP_NAME self.ec2_connection = Mock(spec=EC2Connection) self.ec2_connection.get_all_security_groups.return_value = [self.ec2_group] - self.openstack_rule_transformer = Mock(spec=OpenstackRuleTransformer) - self.ec2_rule_transformer = Mock(spec=EC2RuleTransformer) + self.openstack_group_transformer = Mock(spec=OpenstackGroupTransformer) + self.ec2_group_transformer = Mock(spec=EC2GroupTransformer) self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection, - self.openstack_rule_transformer, self.ec2_rule_transformer) + self.openstack_group_transformer, self.ec2_group_transformer) def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self): - self.openstack_group.rules = [self.existing_new_rule] - self.ec2_group.rules = [] + new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') + transformed_openstack_group = Mock(spec=Group) + transformed_openstack_group.rule_diff.return_value = [new_rule] + self.openstack_group_transformer.to_group.return_value = transformed_openstack_group self.rule_refresher.refresh(self.openstack_instance) self.ec2_connection.authorize_security_group.assert_called_once_with( group_name=GROUP_NAME, - ip_protocol='abc', - from_port=1111, - to_port=2222, - cidr_ip="1.2.3.4/55" - ) - - def test_should_add_rule_to_ec2_security_group_when_other_rule_already_on_both(self): - existing_openstack_rule = {'ip_protocol': 'hi', 'from_port': 3333, 'to_port': 4444, 'ip_range': {'cidr': '6.7.8.9/00'}} - existing_ec2_rule = {'attribute': 'value'} - existing_transformed_rule = Rule('sdfg', 5, 6, '7.7.7.7/77') - new_transformed_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') - - self.openstack_group.rules = [ - existing_openstack_rule, - self.existing_new_rule - ] - self.ec2_group.rules = [existing_ec2_rule] - - def mock_openstack_to_rule(openstack_rule): - return existing_transformed_rule if openstack_rule == existing_openstack_rule else new_transformed_rule - - def mock_ec2_to_rule(ec2_rule): - if ec2_rule == existing_ec2_rule: - return existing_transformed_rule - - self.openstack_rule_transformer.to_rule.side_effect = mock_openstack_to_rule - self.ec2_rule_transformer.to_rule.side_effect = mock_ec2_to_rule - - self.rule_refresher.refresh(self.openstack_instance) - - self.ec2_connection.authorize_security_group.assert_called_once_with( - group_name=GROUP_NAME, - ip_protocol=self.existing_new_rule['ip_protocol'], - from_port=self.existing_new_rule['from_port'], - to_port=self.existing_new_rule['to_port'], - cidr_ip=self.existing_new_rule['ip_range']['cidr'] + ip_protocol=new_rule.ip_protocol, + from_port=new_rule.from_port, + to_port=new_rule.to_port, + cidr_ip=new_rule.ip_range ) def test_should_add_rule_to_corresponding_ec2_group_when_other_groups_present(self): - openstack_group2 = Mock() - openstack_group2.name = "group2" - ec2_group2 = Mock() - ec2_group2.rules = [] - self.ec2_group.rules = [] + openstack_group_with_new_rule = Mock() + openstack_group_with_new_rule.name = OTHER_GROUP_NAME + other_ec2_group = Mock() - self.openstack_group.rules = [self.existing_new_rule] - openstack_group2.rules = [] - self.openstack_instance.security_groups = [{'name': GROUP_NAME}, {'name': openstack_group2.name}] + self.openstack_instance.security_groups = [{'name': GROUP_NAME}, {'name': OTHER_GROUP_NAME}] - self.openstack_group_manager.list.return_value = [openstack_group2, self.openstack_group] + self.openstack_group_manager.list.return_value = [openstack_group_with_new_rule, self.openstack_group] def mock_get_all_security_groups(groupnames=None): - if groupnames == ec2_group2.name: - return [ec2_group2] + if groupnames == other_ec2_group.name: + return [other_ec2_group] return [self.ec2_group] self.ec2_connection.get_all_security_groups.side_effect = mock_get_all_security_groups + new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') + transformed_openstack_group = Mock(spec=Group) + transformed_openstack_group.rule_diff.return_value = [] + transformed_openstack_group_with_new_rule = Mock(spec=Group) + transformed_openstack_group_with_new_rule.rule_diff.return_value = [new_rule] + + def mock_openstack_to_group(openstack_group): + if openstack_group == self.openstack_group: + return transformed_openstack_group + else: + return transformed_openstack_group_with_new_rule + self.openstack_group_transformer.to_group.side_effect = mock_openstack_to_group + + self.rule_refresher.refresh(self.openstack_instance) self.ec2_connection.authorize_security_group.assert_called_once_with( - group_name=GROUP_NAME, - ip_protocol=self.existing_new_rule['ip_protocol'], - from_port=self.existing_new_rule['from_port'], - to_port=self.existing_new_rule['to_port'], - cidr_ip=self.existing_new_rule['ip_range']['cidr'] + group_name=OTHER_GROUP_NAME, + ip_protocol=new_rule.ip_protocol, + from_port=new_rule.from_port, + to_port=new_rule.to_port, + cidr_ip=new_rule.ip_range ) \ No newline at end of file From c34f2fac8f3e2fa4ed4d995ce3ded4d2e34397f1 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 13:54:23 -0600 Subject: [PATCH 06/14] Ed & Cameron | Extract getting transformed rules from openstack and ec2 groups into rule services, extract getting openstack group into group service --- ec2_rule_service.py | 9 +++ openstack_group_service.py | 7 +++ openstack_rule_service.py | 9 +++ rule_refresher.py | 28 ++++----- tests/test_ec2_rule_service.py | 50 +++++++++++++++ tests/test_openstack_group_service.py | 27 +++++++++ tests/test_openstack_rule_service.py | 52 ++++++++++++++++ tests/test_rule_refresher.py | 87 +++++++++------------------ 8 files changed, 196 insertions(+), 73 deletions(-) create mode 100644 ec2_rule_service.py create mode 100644 openstack_group_service.py create mode 100644 openstack_rule_service.py create mode 100644 tests/test_ec2_rule_service.py create mode 100644 tests/test_openstack_group_service.py create mode 100644 tests/test_openstack_rule_service.py diff --git a/ec2_rule_service.py b/ec2_rule_service.py new file mode 100644 index 0000000..ab07ff7 --- /dev/null +++ b/ec2_rule_service.py @@ -0,0 +1,9 @@ +class EC2RuleService: + + def __init__(self, ec2_connection, ec2_rule_transformer): + self.ec2_connection = ec2_connection + self.ec2_rule_transformer = ec2_rule_transformer + + def get_rules_for_group(self, group_name): + group = self.ec2_connection.get_all_security_groups(groupnames=group_name)[0] + return set([self.ec2_rule_transformer.to_rule(rule) for rule in group.rules]) \ No newline at end of file diff --git a/openstack_group_service.py b/openstack_group_service.py new file mode 100644 index 0000000..0b578f8 --- /dev/null +++ b/openstack_group_service.py @@ -0,0 +1,7 @@ +class OpenstackGroupService(): + + def __init__(self, security_group_manager): + self.security_group_manager = security_group_manager + + def get_group(self, group_name): + return [group for group in self.security_group_manager.list() if group.name == group_name][0] \ No newline at end of file diff --git a/openstack_rule_service.py b/openstack_rule_service.py new file mode 100644 index 0000000..37765fe --- /dev/null +++ b/openstack_rule_service.py @@ -0,0 +1,9 @@ +class OpenstackRuleService: + def __init__(self, group_service, openstack_rule_transformer): + self.group_service = group_service + self.openstack_rule_transformer = openstack_rule_transformer + + def get_rules_for_group(self, group_name): + openstack_group = self.group_service.get_group(group_name) + return set([self.openstack_rule_transformer.to_rule(rule) for rule in openstack_group.rules]) + # return self.group_service.get_group(group_name).rules \ No newline at end of file diff --git a/rule_refresher.py b/rule_refresher.py index 73d7fd9..0e64921 100644 --- a/rule_refresher.py +++ b/rule_refresher.py @@ -1,27 +1,27 @@ class RuleRefresher: - def __init__(self, openstack_group_manager, ec2_conn, openstack_group_transformer, ec2_group_transformer): - self.openstack_group_manager = openstack_group_manager + def __init__(self, ec2_conn, openstack_rule_service, ec2_rule_service): self.ec2_conn = ec2_conn - self.openstack_group_transformer = openstack_group_transformer - self.ec2_group_transformer = ec2_group_transformer + self.openstack_rule_service = openstack_rule_service + self.ec2_rule_service = ec2_rule_service def refresh(self, openstack_instance): for group_dict in openstack_instance.security_groups: - openstack_group = [group for group in self.openstack_group_manager.list() if group.name == group_dict['name']][0] - transformed_openstack_group = self.openstack_group_transformer.to_group(openstack_group) + # openstack_group = [group for group in self.openstack_group_manager.list() if group.name == group_dict['name']][0] + # transformed_openstack_group = self.openstack_group_transformer.to_group(openstack_group) + # ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0] + # transformed_ec2_group = self.ec2_group_transformer.to_group(ec2_group) - ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0] - transformed_ec2_group = self.ec2_group_transformer.to_group(ec2_group) + # TODO: transform openstack rules before finding difference + openstack_rules = self.openstack_rule_service.get_rules_for_group(group_dict['name']) + ec2_rules = self.ec2_rule_service.get_rules_for_group(group_dict['name']) - rules_in_openstack_group_not_in_ec2 = transformed_openstack_group.rule_diff(transformed_ec2_group) + for rule in openstack_rules - ec2_rules: + self._create_rule_on_ec2(group_dict['name'], rule) - for rule in rules_in_openstack_group_not_in_ec2: - self._create_rule_on_ec2(openstack_group, rule) - - def _create_rule_on_ec2(self, openstack_group, rule): + def _create_rule_on_ec2(self, group_name, rule): self.ec2_conn.authorize_security_group( - group_name=openstack_group.name, + group_name=group_name, ip_protocol=rule.ip_protocol, from_port=rule.from_port, to_port=rule.to_port, diff --git a/tests/test_ec2_rule_service.py b/tests/test_ec2_rule_service.py new file mode 100644 index 0000000..d50adcd --- /dev/null +++ b/tests/test_ec2_rule_service.py @@ -0,0 +1,50 @@ +import unittest + +from boto.ec2 import EC2Connection +from boto.ec2.securitygroup import SecurityGroup +from mock import Mock, call + +from nova.virt.ec2.ec2_rule_service import EC2RuleService +from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer +from nova.virt.ec2.tests.fake_ec2_rule_builder import FakeEC2RuleBuilder + +class TestEC2RuleService(unittest.TestCase): + + def setUp(self): + self.security_group = Mock(spec=SecurityGroup) + self.security_group.name = "secGroup" + + self.ec2_connection = Mock(spec=EC2Connection) + self.ec2_connection.get_all_security_groups.return_value = [self.security_group] + self.ec2_rule_transformer = Mock(spec=EC2RuleTransformer) + + self.ec2_rule_service = EC2RuleService(self.ec2_connection, self.ec2_rule_transformer) + + def test_should_get_security_group_from_ec2_connection(self): + self.security_group.rules = [] + + self.ec2_rule_service.get_rules_for_group(self.security_group.name) + + self.ec2_connection.get_all_security_groups.assert_called_once_with(groupnames=self.security_group.name) + + def test_should_transform_rules_from_security_group(self): + first_rule = Mock() + second_rule = Mock() + self.security_group.rules = [first_rule, second_rule] + + self.ec2_rule_service.get_rules_for_group(self.security_group.name) + + self.ec2_rule_transformer.to_rule.assert_has_calls([call(first_rule), call(second_rule)]) + + def test_should_return_transformed_security_group_rules(self): + first_rule = Mock() + second_rule = Mock() + self.security_group.rules = [first_rule, second_rule] + + first_transformed_rule = Mock() + second_transformed_rule = Mock() + self.ec2_rule_transformer.to_rule.side_effect = [first_transformed_rule, second_transformed_rule] + + actual_rules = self.ec2_rule_service.get_rules_for_group(self.security_group.name) + + self.assertEqual(actual_rules, set([first_transformed_rule, second_transformed_rule])) \ No newline at end of file diff --git a/tests/test_openstack_group_service.py b/tests/test_openstack_group_service.py new file mode 100644 index 0000000..362e209 --- /dev/null +++ b/tests/test_openstack_group_service.py @@ -0,0 +1,27 @@ +import unittest +from mock import Mock +from novaclient.v1_1.security_groups import SecurityGroupManager +from nova.virt.ec2.openstack_group_service import OpenstackGroupService + + +class TestOpenstackGroupService(unittest.TestCase): + + def setUp(self): + self.security_group_manager = Mock(spec=SecurityGroupManager) + self.openstack_group_service = OpenstackGroupService(self.security_group_manager) + + def test_should_get_group_from_nova_security_group_manager(self): + security_group = Mock() + security_group.name = 'secGroup' + self.security_group_manager.list.return_value = [security_group] + + self.assertEqual(self.openstack_group_service.get_group(security_group.name), security_group) + + def test_should_get_group_from_nova_security_group_manager_when_multiple_groups_present(self): + security_group1 = Mock() + security_group1.name = 'secGroup' + security_group2 = Mock() + security_group2.name = 'otherGroup' + self.security_group_manager.list.return_value = [security_group1, security_group2] + + self.assertEqual(self.openstack_group_service.get_group(security_group2.name), security_group2) \ No newline at end of file diff --git a/tests/test_openstack_rule_service.py b/tests/test_openstack_rule_service.py new file mode 100644 index 0000000..3297338 --- /dev/null +++ b/tests/test_openstack_rule_service.py @@ -0,0 +1,52 @@ +import unittest + +from mock import Mock, call +from novaclient.v1_1.security_groups import SecurityGroup + +from nova.virt.ec2.openstack_rule_service import OpenstackRuleService +from nova.virt.ec2.openstack_group_service import OpenstackGroupService +from nova.virt.ec2.openstack_rule_transformer import OpenstackRuleTransformer + + +class TestOpenstackRuleService(unittest.TestCase): + def setUp(self): + self.security_group = Mock(spec=SecurityGroup) + self.security_group.name = "secGroup" + + self.openstack_group_service = Mock(OpenstackGroupService) + self.openstack_group_service.get_group.return_value = self.security_group + self.openstack_rule_transformer = Mock(OpenstackRuleTransformer) + + self.openstack_rule_service = OpenstackRuleService( + self.openstack_group_service, + self.openstack_rule_transformer + ) + + def test_should_get_security_group_from_group_service(self): + self.security_group.rules = [] + + self.openstack_rule_service.get_rules_for_group(self.security_group.name) + + self.openstack_group_service.get_group.assert_called_once_with(self.security_group.name) + + def test_should_transform_rules_from_security_group(self): + first_rule = Mock() + second_rule = Mock() + self.security_group.rules = [first_rule, second_rule] + + self.openstack_rule_service.get_rules_for_group(self.security_group.name) + + self.openstack_rule_transformer.to_rule.assert_has_calls([call(first_rule), call(second_rule)]) + + def test_should_return_transformed_security_group_rules(self): + first_rule = Mock() + second_rule = Mock() + self.security_group.rules = [first_rule, second_rule] + + first_transformed_rule = Mock() + second_transformed_rule = Mock() + self.openstack_rule_transformer.to_rule.side_effect = [first_transformed_rule, second_transformed_rule] + + actual_rules = self.openstack_rule_service.get_rules_for_group(self.security_group.name) + + self.assertEqual(actual_rules, set([first_transformed_rule, second_transformed_rule])) diff --git a/tests/test_rule_refresher.py b/tests/test_rule_refresher.py index e4bddee..04db6c1 100644 --- a/tests/test_rule_refresher.py +++ b/tests/test_rule_refresher.py @@ -2,92 +2,61 @@ import unittest from boto.ec2 import EC2Connection from mock import Mock -import novaclient -from novaclient.v1_1.servers import Server from nova.virt.ec2.rule_refresher import RuleRefresher + from nova.virt.ec2.rule import Rule -from nova.virt.ec2.ec2_group_transformer import EC2GroupTransformer -from nova.virt.ec2.openstack_group_transformer import OpenstackGroupTransformer -from nova.virt.ec2.group import Group +from nova.virt.ec2.openstack_rule_service import OpenstackRuleService +from nova.virt.ec2.ec2_rule_service import EC2RuleService GROUP_NAME = 'secGroup' OTHER_GROUP_NAME = "otherSecGroup" class TestRuleRefresher(unittest.TestCase): def setUp(self): - self.new_openstack_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}} - + self.new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') self.openstack_instance = Mock() - self.openstack_instance.security_groups = [{'name': GROUP_NAME}] - self.openstack_group = Mock() - self.openstack_group.name = GROUP_NAME - self.openstack_group_manager = Mock(spec=novaclient.v1_1.security_groups.SecurityGroupManager) - self.openstack_group_manager.list.return_value = [self.openstack_group] + self.ec2_connection = Mock(EC2Connection) + self.openstack_rule_service = Mock(OpenstackRuleService) + self.ec2_rule_service = Mock(EC2RuleService) - self.ec2_group = Mock() - self.ec2_group.name = GROUP_NAME - self.ec2_connection = Mock(spec=EC2Connection) - self.ec2_connection.get_all_security_groups.return_value = [self.ec2_group] - - self.openstack_group_transformer = Mock(spec=OpenstackGroupTransformer) - self.ec2_group_transformer = Mock(spec=EC2GroupTransformer) - - self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection, - self.openstack_group_transformer, self.ec2_group_transformer) + self.rule_refresher = RuleRefresher( + self.ec2_connection, + self.openstack_rule_service, + self.ec2_rule_service + ) def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self): - new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') - transformed_openstack_group = Mock(spec=Group) - transformed_openstack_group.rule_diff.return_value = [new_rule] - self.openstack_group_transformer.to_group.return_value = transformed_openstack_group + self.openstack_instance.security_groups = [{'name': GROUP_NAME}] + + self.openstack_rule_service.get_rules_for_group.return_value = set([self.new_rule]) + self.ec2_rule_service.get_rules_for_group.return_value = set() self.rule_refresher.refresh(self.openstack_instance) self.ec2_connection.authorize_security_group.assert_called_once_with( group_name=GROUP_NAME, - ip_protocol=new_rule.ip_protocol, - from_port=new_rule.from_port, - to_port=new_rule.to_port, - cidr_ip=new_rule.ip_range + ip_protocol=self.new_rule.ip_protocol, + from_port=self.new_rule.from_port, + to_port=self.new_rule.to_port, + cidr_ip=self.new_rule.ip_range ) def test_should_add_rule_to_corresponding_ec2_group_when_other_groups_present(self): - openstack_group_with_new_rule = Mock() - openstack_group_with_new_rule.name = OTHER_GROUP_NAME - other_ec2_group = Mock() - self.openstack_instance.security_groups = [{'name': GROUP_NAME}, {'name': OTHER_GROUP_NAME}] - self.openstack_group_manager.list.return_value = [openstack_group_with_new_rule, self.openstack_group] - - def mock_get_all_security_groups(groupnames=None): - if groupnames == other_ec2_group.name: - return [other_ec2_group] - return [self.ec2_group] - self.ec2_connection.get_all_security_groups.side_effect = mock_get_all_security_groups - - new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') - transformed_openstack_group = Mock(spec=Group) - transformed_openstack_group.rule_diff.return_value = [] - transformed_openstack_group_with_new_rule = Mock(spec=Group) - transformed_openstack_group_with_new_rule.rule_diff.return_value = [new_rule] - - def mock_openstack_to_group(openstack_group): - if openstack_group == self.openstack_group: - return transformed_openstack_group - else: - return transformed_openstack_group_with_new_rule - self.openstack_group_transformer.to_group.side_effect = mock_openstack_to_group - + def mock_get_rules_for_openstack_group(group_name): + return set() if group_name == GROUP_NAME else set([self.new_rule]) + self.openstack_rule_service.get_rules_for_group.side_effect = mock_get_rules_for_openstack_group + self.ec2_rule_service.get_rules_for_group.return_value = set() self.rule_refresher.refresh(self.openstack_instance) self.ec2_connection.authorize_security_group.assert_called_once_with( group_name=OTHER_GROUP_NAME, - ip_protocol=new_rule.ip_protocol, - from_port=new_rule.from_port, - to_port=new_rule.to_port, - cidr_ip=new_rule.ip_range + ip_protocol=self.new_rule.ip_protocol, + from_port=self.new_rule.from_port, + to_port=self.new_rule.to_port, + cidr_ip=self.new_rule.ip_range ) \ No newline at end of file From 6d4ec264a5bdb9977b4a2267e7abbd1e89a5960b Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 14:00:03 -0600 Subject: [PATCH 07/14] Ed & Cameron | Move unit tests to separate directory --- tests/unit/__init__.py | 0 tests/{ => unit}/fake_ec2_rule_builder.py | 0 tests/{ => unit}/test_ec2_rule_service.py | 2 +- tests/{ => unit}/test_ec2_rule_transformer.py | 9 +++++---- tests/{ => unit}/test_openstack_group_service.py | 2 ++ tests/{ => unit}/test_openstack_rule_service.py | 0 tests/{ => unit}/test_rule_comparator.py | 0 tests/{ => unit}/test_rule_refresher.py | 1 - 8 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 tests/unit/__init__.py rename tests/{ => unit}/fake_ec2_rule_builder.py (100%) rename tests/{ => unit}/test_ec2_rule_service.py (94%) rename tests/{ => unit}/test_ec2_rule_transformer.py (96%) rename tests/{ => unit}/test_openstack_group_service.py (96%) rename tests/{ => unit}/test_openstack_rule_service.py (100%) rename tests/{ => unit}/test_rule_comparator.py (100%) rename tests/{ => unit}/test_rule_refresher.py (99%) diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/fake_ec2_rule_builder.py b/tests/unit/fake_ec2_rule_builder.py similarity index 100% rename from tests/fake_ec2_rule_builder.py rename to tests/unit/fake_ec2_rule_builder.py diff --git a/tests/test_ec2_rule_service.py b/tests/unit/test_ec2_rule_service.py similarity index 94% rename from tests/test_ec2_rule_service.py rename to tests/unit/test_ec2_rule_service.py index d50adcd..6acecf9 100644 --- a/tests/test_ec2_rule_service.py +++ b/tests/unit/test_ec2_rule_service.py @@ -6,7 +6,7 @@ from mock import Mock, call from nova.virt.ec2.ec2_rule_service import EC2RuleService from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer -from nova.virt.ec2.tests.fake_ec2_rule_builder import FakeEC2RuleBuilder + class TestEC2RuleService(unittest.TestCase): diff --git a/tests/test_ec2_rule_transformer.py b/tests/unit/test_ec2_rule_transformer.py similarity index 96% rename from tests/test_ec2_rule_transformer.py rename to tests/unit/test_ec2_rule_transformer.py index b136dfe..335af04 100644 --- a/tests/test_ec2_rule_transformer.py +++ b/tests/unit/test_ec2_rule_transformer.py @@ -1,9 +1,10 @@ -from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer -from mock import Mock -from fake_ec2_rule_builder import FakeEC2RuleBuilder - import unittest +from mock import Mock + +from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer +from fake_ec2_rule_builder import FakeEC2RuleBuilder + class TestEC2RuleTransformer(unittest.TestCase): diff --git a/tests/test_openstack_group_service.py b/tests/unit/test_openstack_group_service.py similarity index 96% rename from tests/test_openstack_group_service.py rename to tests/unit/test_openstack_group_service.py index 362e209..21404f9 100644 --- a/tests/test_openstack_group_service.py +++ b/tests/unit/test_openstack_group_service.py @@ -1,6 +1,8 @@ import unittest + from mock import Mock from novaclient.v1_1.security_groups import SecurityGroupManager + from nova.virt.ec2.openstack_group_service import OpenstackGroupService diff --git a/tests/test_openstack_rule_service.py b/tests/unit/test_openstack_rule_service.py similarity index 100% rename from tests/test_openstack_rule_service.py rename to tests/unit/test_openstack_rule_service.py diff --git a/tests/test_rule_comparator.py b/tests/unit/test_rule_comparator.py similarity index 100% rename from tests/test_rule_comparator.py rename to tests/unit/test_rule_comparator.py diff --git a/tests/test_rule_refresher.py b/tests/unit/test_rule_refresher.py similarity index 99% rename from tests/test_rule_refresher.py rename to tests/unit/test_rule_refresher.py index 04db6c1..4513a39 100644 --- a/tests/test_rule_refresher.py +++ b/tests/unit/test_rule_refresher.py @@ -4,7 +4,6 @@ from boto.ec2 import EC2Connection from mock import Mock from nova.virt.ec2.rule_refresher import RuleRefresher - from nova.virt.ec2.rule import Rule from nova.virt.ec2.openstack_rule_service import OpenstackRuleService from nova.virt.ec2.ec2_rule_service import EC2RuleService From a8f09b1ccb0c070561ce9a5fea71399430701664 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 14:49:02 -0600 Subject: [PATCH 08/14] Cameron & Ed | Extract looping over groups and refreshing their rules to GroupRuleRefresher --- ec2driver.py | 65 +++++++------------ group_rule_refresher.py | 22 +++++++ instance_rule_refresher.py | 11 ++++ rule_refresher.py | 29 --------- ...resher.py => test_group_rule_refresher.py} | 28 ++------ tests/unit/test_instance_rule_refresher.py | 21 ++++++ 6 files changed, 80 insertions(+), 96 deletions(-) create mode 100644 group_rule_refresher.py create mode 100644 instance_rule_refresher.py delete mode 100644 rule_refresher.py rename tests/unit/{test_rule_refresher.py => test_group_rule_refresher.py} (52%) create mode 100644 tests/unit/test_instance_rule_refresher.py diff --git a/ec2driver.py b/ec2driver.py index ada1465..f0c9c44 100644 --- a/ec2driver.py +++ b/ec2driver.py @@ -25,6 +25,8 @@ from boto.exception import EC2ResponseError from boto.regioninfo import RegionInfo from oslo.config import cfg from novaclient.v1_1 import client +from ec2_rule_service import EC2RuleService +from ec2_rule_transformer import EC2RuleTransformer from ec2driver_config import * from nova import block_device @@ -39,8 +41,13 @@ from nova.openstack.common import loopingcall from nova.virt import driver from nova.virt import virtapi from credentials import get_nova_creds +from instance_rule_refresher import InstanceRuleRefresher +from openstack_group_service import OpenstackGroupService +from openstack_rule_service import OpenstackRuleService +from openstack_rule_transformer import OpenstackRuleTransformer import rule_comparator +from group_rule_refresher import GroupRuleRefresher LOG = logging.getLogger(__name__) @@ -156,7 +163,20 @@ class EC2Driver(driver.ComputeDriver): aws_region, aws_access_key_id=CONF.ec2driver.ec2_access_key_id, aws_secret_access_key=CONF.ec2driver.ec2_secret_access_key) self.security_group_lock = Lock() - self.rule_comparator = rule_comparator.RuleComparator(self.ec2_conn) + + self.instance_rule_refresher = InstanceRuleRefresher( + GroupRuleRefresher( + ec2_connection=self.ec2_conn, + openstack_rule_service=OpenstackRuleService( + group_service=OpenstackGroupService(self.nova.security_groups), + openstack_rule_transformer=OpenstackRuleTransformer() + ), + ec2_rule_service=EC2RuleService( + ec2_connection=self.ec2_conn, + ec2_rule_transformer=EC2RuleTransformer() + ) + ) + ) if not '_EC2_NODES' in globals(): set_nodes([CONF.host]) @@ -720,48 +740,7 @@ class EC2Driver(driver.ComputeDriver): # TODO: lock for case when group is associated with multiple instances [Cameron & Ed] - openstack_instance = self.nova.servers.get(instance['id']) - - for group_dict in openstack_instance.security_groups: - - openstack_group =\ - [group for group in self.nova.security_groups.list() if group.name == group_dict['name']][0] - - ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0] - - for openstack_rule in openstack_group.rules: - equivalent_rule_found_in_ec2 = False - for ec2_rule in ec2_group.rules: - if self.rule_comparator.rules_are_equal(openstack_rule, ec2_rule): - equivalent_rule_found_in_ec2 = True - break - - if not equivalent_rule_found_in_ec2: - self.ec2_conn.authorize_security_group( - group_name=ec2_group.name, - ip_protocol=openstack_rule['ip_protocol'], - from_port=openstack_rule['from_port'], - to_port=openstack_rule['to_port'], - src_security_group_name=self._get_allowed_group_name_from_openstack_rule_if_present(openstack_rule), - cidr_ip=self._get_allowed_ip_range_from_openstack_rule_if_present(openstack_rule) - ) - - for ec2_rule in ec2_group.rules: - equivalent_rule_found_in_openstack = False - for openstack_rule in openstack_group.rules: - if self.rule_comparator.rules_are_equal(openstack_rule, ec2_rule): - equivalent_rule_found_in_openstack = True - break - - if not equivalent_rule_found_in_openstack: - self.ec2_conn.revoke_security_group( - group_name=ec2_group.name, - ip_protocol=ec2_rule.ip_protocol, - from_port=ec2_rule.from_port, - to_port=ec2_rule.to_port, - cidr_ip=ec2_rule.grants[0].cidr_ip, - src_security_group_group_id=ec2_rule.grants[0].group_id - ) + self.instance_rule_refresher.refresh(self.nova.servers.get(instance['id'])) return diff --git a/group_rule_refresher.py b/group_rule_refresher.py new file mode 100644 index 0000000..e8dafbb --- /dev/null +++ b/group_rule_refresher.py @@ -0,0 +1,22 @@ +class GroupRuleRefresher: + + def __init__(self, ec2_connection, openstack_rule_service, ec2_rule_service): + self.ec2_conn = ec2_connection + self.openstack_rule_service = openstack_rule_service + self.ec2_rule_service = ec2_rule_service + + def refresh(self, group_name): + openstack_rules = self.openstack_rule_service.get_rules_for_group(group_name) + ec2_rules = self.ec2_rule_service.get_rules_for_group(group_name) + + for rule in openstack_rules - ec2_rules: + self._create_rule_on_ec2(group_name, rule) + + def _create_rule_on_ec2(self, group_name, rule): + self.ec2_conn.authorize_security_group( + group_name=group_name, + ip_protocol=rule.ip_protocol, + from_port=rule.from_port, + to_port=rule.to_port, + cidr_ip=rule.ip_range + ) \ No newline at end of file diff --git a/instance_rule_refresher.py b/instance_rule_refresher.py new file mode 100644 index 0000000..03a0d70 --- /dev/null +++ b/instance_rule_refresher.py @@ -0,0 +1,11 @@ +class InstanceRuleRefresher: + + def __init__(self, group_rule_refresher): + self.group_rule_refresher = group_rule_refresher + + def refresh(self, instance): + for group_name in self._get_group_names(instance): + self.group_rule_refresher.refresh(group_name) + + def _get_group_names(self, instance): + return [group['name'] for group in instance.security_groups] diff --git a/rule_refresher.py b/rule_refresher.py deleted file mode 100644 index 0e64921..0000000 --- a/rule_refresher.py +++ /dev/null @@ -1,29 +0,0 @@ -class RuleRefresher: - - def __init__(self, ec2_conn, openstack_rule_service, ec2_rule_service): - self.ec2_conn = ec2_conn - self.openstack_rule_service = openstack_rule_service - self.ec2_rule_service = ec2_rule_service - - def refresh(self, openstack_instance): - for group_dict in openstack_instance.security_groups: - # openstack_group = [group for group in self.openstack_group_manager.list() if group.name == group_dict['name']][0] - # transformed_openstack_group = self.openstack_group_transformer.to_group(openstack_group) - # ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0] - # transformed_ec2_group = self.ec2_group_transformer.to_group(ec2_group) - - # TODO: transform openstack rules before finding difference - openstack_rules = self.openstack_rule_service.get_rules_for_group(group_dict['name']) - ec2_rules = self.ec2_rule_service.get_rules_for_group(group_dict['name']) - - for rule in openstack_rules - ec2_rules: - self._create_rule_on_ec2(group_dict['name'], rule) - - def _create_rule_on_ec2(self, group_name, rule): - self.ec2_conn.authorize_security_group( - group_name=group_name, - ip_protocol=rule.ip_protocol, - from_port=rule.from_port, - to_port=rule.to_port, - cidr_ip=rule.ip_range - ) \ No newline at end of file diff --git a/tests/unit/test_rule_refresher.py b/tests/unit/test_group_rule_refresher.py similarity index 52% rename from tests/unit/test_rule_refresher.py rename to tests/unit/test_group_rule_refresher.py index 4513a39..1166a71 100644 --- a/tests/unit/test_rule_refresher.py +++ b/tests/unit/test_group_rule_refresher.py @@ -3,7 +3,7 @@ import unittest from boto.ec2 import EC2Connection from mock import Mock -from nova.virt.ec2.rule_refresher import RuleRefresher +from nova.virt.ec2.group_rule_refresher import GroupRuleRefresher from nova.virt.ec2.rule import Rule from nova.virt.ec2.openstack_rule_service import OpenstackRuleService from nova.virt.ec2.ec2_rule_service import EC2RuleService @@ -11,7 +11,7 @@ from nova.virt.ec2.ec2_rule_service import EC2RuleService GROUP_NAME = 'secGroup' OTHER_GROUP_NAME = "otherSecGroup" -class TestRuleRefresher(unittest.TestCase): +class TestGroupRuleRefresher(unittest.TestCase): def setUp(self): self.new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') self.openstack_instance = Mock() @@ -20,19 +20,17 @@ class TestRuleRefresher(unittest.TestCase): self.openstack_rule_service = Mock(OpenstackRuleService) self.ec2_rule_service = Mock(EC2RuleService) - self.rule_refresher = RuleRefresher( + self.group_rule_refresher = GroupRuleRefresher( self.ec2_connection, self.openstack_rule_service, self.ec2_rule_service ) def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self): - self.openstack_instance.security_groups = [{'name': GROUP_NAME}] - self.openstack_rule_service.get_rules_for_group.return_value = set([self.new_rule]) self.ec2_rule_service.get_rules_for_group.return_value = set() - self.rule_refresher.refresh(self.openstack_instance) + self.group_rule_refresher.refresh(GROUP_NAME) self.ec2_connection.authorize_security_group.assert_called_once_with( group_name=GROUP_NAME, @@ -40,22 +38,4 @@ class TestRuleRefresher(unittest.TestCase): from_port=self.new_rule.from_port, to_port=self.new_rule.to_port, cidr_ip=self.new_rule.ip_range - ) - - def test_should_add_rule_to_corresponding_ec2_group_when_other_groups_present(self): - self.openstack_instance.security_groups = [{'name': GROUP_NAME}, {'name': OTHER_GROUP_NAME}] - - def mock_get_rules_for_openstack_group(group_name): - return set() if group_name == GROUP_NAME else set([self.new_rule]) - self.openstack_rule_service.get_rules_for_group.side_effect = mock_get_rules_for_openstack_group - self.ec2_rule_service.get_rules_for_group.return_value = set() - - self.rule_refresher.refresh(self.openstack_instance) - - self.ec2_connection.authorize_security_group.assert_called_once_with( - group_name=OTHER_GROUP_NAME, - ip_protocol=self.new_rule.ip_protocol, - from_port=self.new_rule.from_port, - to_port=self.new_rule.to_port, - cidr_ip=self.new_rule.ip_range ) \ No newline at end of file diff --git a/tests/unit/test_instance_rule_refresher.py b/tests/unit/test_instance_rule_refresher.py new file mode 100644 index 0000000..57b1727 --- /dev/null +++ b/tests/unit/test_instance_rule_refresher.py @@ -0,0 +1,21 @@ +import unittest +from mock import Mock, call +from nova.virt.ec2.group_rule_refresher import GroupRuleRefresher +from nova.virt.ec2.instance_rule_refresher import InstanceRuleRefresher + + +class TestInstanceRuleRefresher(unittest.TestCase): + + def test_should_call_group_rule_refresher_on_every_group_for_instance(self): + + group_rule_refresher = Mock(spec=GroupRuleRefresher) + + instance = Mock() + first_group = {'name': 'firstGroup'} + second_group = {'name': 'secondGroup'} + instance.security_groups = [first_group, second_group] + + instance_rule_refresher = InstanceRuleRefresher(group_rule_refresher) + instance_rule_refresher.refresh(instance) + + group_rule_refresher.refresh.assert_has_calls([call(first_group['name']), call(second_group['name'])]) \ No newline at end of file From 9826b0778ccfa1dce9906e16d217207b27f2d0b8 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 14:53:39 -0600 Subject: [PATCH 09/14] Ed & Cameron | Move integration tests to separate directory --- tests/integration/__init__.py | 0 tests/{ => integration}/ec2_test_base.py | 4 ++-- tests/{ => integration}/test_ec2driver.py | 7 ++++--- tests/{ => integration}/test_security_groups.py | 5 +++-- 4 files changed, 9 insertions(+), 7 deletions(-) create mode 100644 tests/integration/__init__.py rename tests/{ => integration}/ec2_test_base.py (96%) rename tests/{ => integration}/test_ec2driver.py (99%) rename tests/{ => integration}/test_security_groups.py (97%) diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/ec2_test_base.py b/tests/integration/ec2_test_base.py similarity index 96% rename from tests/ec2_test_base.py rename to tests/integration/ec2_test_base.py index a459b53..4416df8 100644 --- a/tests/ec2_test_base.py +++ b/tests/integration/ec2_test_base.py @@ -7,8 +7,8 @@ import datetime from novaclient.v1_1 import client from boto import ec2 -from ..credentials import get_nova_creds -from ..ec2driver_config import * +from ...credentials import get_nova_creds +from ...ec2driver_config import * class EC2TestBase(unittest.TestCase): diff --git a/tests/test_ec2driver.py b/tests/integration/test_ec2driver.py similarity index 99% rename from tests/test_ec2driver.py rename to tests/integration/test_ec2driver.py index 717753a..890b39e 100644 --- a/tests/test_ec2driver.py +++ b/tests/integration/test_ec2driver.py @@ -1,10 +1,11 @@ import unittest import urllib2 -from boto.exception import EC2ResponseError +from boto.exception import EC2ResponseError from novaclient.v1_1 import client -from ..ec2driver_config import * -from ec2_test_base import EC2TestBase + +from ...ec2driver_config import * +from tests.integration.ec2_test_base import EC2TestBase class TestEC2Driver(EC2TestBase): diff --git a/tests/test_security_groups.py b/tests/integration/test_security_groups.py similarity index 97% rename from tests/test_security_groups.py rename to tests/integration/test_security_groups.py index 04517a4..f916750 100644 --- a/tests/test_security_groups.py +++ b/tests/integration/test_security_groups.py @@ -2,8 +2,9 @@ from time import sleep import unittest from random import randint -from ..ec2driver_config import * -from ec2_test_base import EC2TestBase +from ...ec2driver_config import * +from tests.integration.ec2_test_base import EC2TestBase + class TestSecurityGroups(EC2TestBase): From 693c165a73d8073c594f5b06162966c0678de2f2 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 15:26:48 -0600 Subject: [PATCH 10/14] Cameron & Ed | Fix EC2RuleTransformer constructor use, updated security group rule integration test --- ec2driver.py | 2 +- tests/integration/test_security_groups.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ec2driver.py b/ec2driver.py index f0c9c44..822424b 100644 --- a/ec2driver.py +++ b/ec2driver.py @@ -173,7 +173,7 @@ class EC2Driver(driver.ComputeDriver): ), ec2_rule_service=EC2RuleService( ec2_connection=self.ec2_conn, - ec2_rule_transformer=EC2RuleTransformer() + ec2_rule_transformer=EC2RuleTransformer(self.ec2_conn) ) ) ) diff --git a/tests/integration/test_security_groups.py b/tests/integration/test_security_groups.py index f916750..3f3dd29 100644 --- a/tests/integration/test_security_groups.py +++ b/tests/integration/test_security_groups.py @@ -3,7 +3,7 @@ import unittest from random import randint from ...ec2driver_config import * -from tests.integration.ec2_test_base import EC2TestBase +from ec2_test_base import EC2TestBase class TestSecurityGroups(EC2TestBase): @@ -36,9 +36,9 @@ class TestSecurityGroups(EC2TestBase): updated_matching_ec2_security_group = self._wait_for_ec2_group_to_have_no_instances(self.security_group) self.assertEqual(updated_matching_ec2_security_group.instances(), []) - - def test_should_add_rule_to_ec2_security_group_when_group_is_added_to_an_instance(self): - + + @unittest.skipIf(os.environ.get('MOCK_EC2'), 'Not supported by moto') + def test_should_add_rule_to_ec2_security_group_when_rule_is_added_to_openstack_group_associated_with_instance(self): security_group_rule = self.nova.security_group_rules.create( parent_group_id=self.security_group.id, ip_protocol='tcp', @@ -47,13 +47,13 @@ class TestSecurityGroups(EC2TestBase): cidr='0.0.0.0/0' ) - updated_security_group = self.nova.security_groups.get(self.security_group.id) - ec2_security_group = self.ec2_conn.get_all_security_groups(groupnames=self.security_group.name)[0] ec2_rule = ec2_security_group.rules[0] - self.assertEqual(ec2_rule.ip_protocol, security_group_rule.ip_protocol) - #etc + self.assertEqual(ec2_rule.ip_protocol, security_group_rule['ip_protocol']) + self.assertEqual(ec2_rule.from_port, security_group_rule['from_port']) + self.assertEqual(ec2_rule.to_port, security_group_rule['to_port']) + self.assertEqual(ec2_rule.grants[0].cidr_ip, security_group_rule['ip_range']['cidr']) def _destroy_security_group(self): print "Cleanup: Destroying security group" From 9ef0e8cb01b942efc434d94127d330e2709f1e85 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 17:06:24 -0600 Subject: [PATCH 11/14] Cameron & Ed | Remove unused arguments from OpenStack and EC2 rules in rule transformers --- ec2_rule_transformer.py | 8 +++++-- openstack_rule_transformer.py | 9 +++++++- tests/unit/fake_ec2_rule_builder.py | 5 +++-- tests/unit/test_openstack_rule_transformer.py | 22 +++++++++++++++++++ 4 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_openstack_rule_transformer.py diff --git a/ec2_rule_transformer.py b/ec2_rule_transformer.py index d6731f8..a2e57ea 100644 --- a/ec2_rule_transformer.py +++ b/ec2_rule_transformer.py @@ -9,7 +9,7 @@ class EC2RuleTransformer: def to_rule(self, ec2_rule): rule_args = deepcopy(vars(ec2_rule)) - del rule_args['grants'] + self._delete_unused_rule_args(rule_args) if ec2_rule.grants[0].cidr_ip: rule_args['ip_range'] = ec2_rule.grants[0].cidr_ip @@ -17,4 +17,8 @@ class EC2RuleTransformer: group_id = ec2_rule.grants[0].group_id rule_args['group_name'] = self.ec2_connection.get_all_security_groups(group_ids=group_id)[0].name - return Rule(**rule_args) \ No newline at end of file + return Rule(**rule_args) + + def _delete_unused_rule_args(self, rule_args): + del rule_args['grants'] + del rule_args['parent'] \ No newline at end of file diff --git a/openstack_rule_transformer.py b/openstack_rule_transformer.py index b78ed4e..5a84cb9 100644 --- a/openstack_rule_transformer.py +++ b/openstack_rule_transformer.py @@ -7,9 +7,16 @@ class OpenstackRuleTransformer: def to_rule(self, openstack_rule): rule_args = deepcopy(openstack_rule) + self._delete_unused_rule_args(rule_args) + if 'cidr' in openstack_rule['ip_range']: rule_args['ip_range'] = openstack_rule['ip_range']['cidr'] else: rule_args['group_name'] = openstack_rule['group']['name'] - return Rule(**openstack_rule) \ No newline at end of file + return Rule(**rule_args) + + def _delete_unused_rule_args(self, rule_args): + del rule_args['group'] + del rule_args['parent_group_id'] + del rule_args['id'] \ No newline at end of file diff --git a/tests/unit/fake_ec2_rule_builder.py b/tests/unit/fake_ec2_rule_builder.py index 1dd43d6..bf6ccb1 100644 --- a/tests/unit/fake_ec2_rule_builder.py +++ b/tests/unit/fake_ec2_rule_builder.py @@ -2,7 +2,7 @@ from collections import namedtuple class FakeEC2RuleBuilder(): - EC2Rule = namedtuple('EC2Rule', 'ip_protocol from_port to_port grants') + EC2Rule = namedtuple('EC2Rule', 'ip_protocol from_port to_port grants parent') GroupOrCIDR = namedtuple('GroupOrCIDR', 'cidr_ip group_id') def __init__(self): @@ -11,6 +11,7 @@ class FakeEC2RuleBuilder(): self.to_port = '3333' self.ip_range = '0.0.0.0/0' self.allowed_security_group_id = None + self.parent = None @staticmethod def an_ec2_rule(): @@ -40,4 +41,4 @@ class FakeEC2RuleBuilder(): def build(self): grants = [self.GroupOrCIDR(self.ip_range, self.allowed_security_group_id)] - return self.EC2Rule(self.ip_protocol, self.from_port, self.to_port, grants) + return self.EC2Rule(self.ip_protocol, self.from_port, self.to_port, grants, self.parent) diff --git a/tests/unit/test_openstack_rule_transformer.py b/tests/unit/test_openstack_rule_transformer.py new file mode 100644 index 0000000..6256455 --- /dev/null +++ b/tests/unit/test_openstack_rule_transformer.py @@ -0,0 +1,22 @@ +import unittest + +from nova.virt.ec2.openstack_rule_transformer import OpenstackRuleTransformer + + +class TestEC2RuleTransformer(unittest.TestCase): + + def setUp(self): + self.openstack_rule_transformer = OpenstackRuleTransformer() + + def test_should_copy_to_port(self): + openstack_rule = { + 'ip_protocol': 'abc', + 'from_port': 123, + 'to_port': 456, + 'group': {}, + 'parent_group_id': 55, + 'ip_range': {'cidr': '9.8.7.6/55'}, + 'id': 18 + } + rule = self.openstack_rule_transformer.to_rule(openstack_rule) + self.assertEqual(rule.to_port, openstack_rule['to_port']) \ No newline at end of file From fc3e9907e75299e3fb55b93aa32503cf159133b3 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Wed, 5 Nov 2014 17:07:29 -0600 Subject: [PATCH 12/14] Ed & Cameron | Remove rules from security groups on EC2 in GroupRuleRefresher --- group_rule_refresher.py | 27 ++++++++++++++++++++----- tests/unit/test_group_rule_refresher.py | 26 ++++++++++++++++++------ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/group_rule_refresher.py b/group_rule_refresher.py index e8dafbb..b473731 100644 --- a/group_rule_refresher.py +++ b/group_rule_refresher.py @@ -6,13 +6,30 @@ class GroupRuleRefresher: self.ec2_rule_service = ec2_rule_service def refresh(self, group_name): - openstack_rules = self.openstack_rule_service.get_rules_for_group(group_name) - ec2_rules = self.ec2_rule_service.get_rules_for_group(group_name) + openstack_rules = self.openstack_rule_service.get_rules_for_group(group_name) + ec2_rules = self.ec2_rule_service.get_rules_for_group(group_name) - for rule in openstack_rules - ec2_rules: - self._create_rule_on_ec2(group_name, rule) + self._add_rules_to_ec2(ec2_rules, group_name, openstack_rules) + self._remove_rules_from_ec2(ec2_rules, group_name, openstack_rules) - def _create_rule_on_ec2(self, group_name, rule): + def _add_rules_to_ec2(self, ec2_rules, group_name, openstack_rules): + for rule in openstack_rules - ec2_rules: + self._add_rule_on_ec2(group_name, rule) + + def _remove_rules_from_ec2(self, ec2_rules, group_name, openstack_rules): + for rule in ec2_rules - openstack_rules: + self._remove_rule_from_ec2(group_name, rule) + + def _remove_rule_from_ec2(self, group_name, rule): + self.ec2_conn.revoke_security_group( + group_name=group_name, + ip_protocol=rule.ip_protocol, + from_port=rule.from_port, + to_port=rule.to_port, + cidr_ip=rule.ip_range + ) + + def _add_rule_on_ec2(self, group_name, rule): self.ec2_conn.authorize_security_group( group_name=group_name, ip_protocol=rule.ip_protocol, diff --git a/tests/unit/test_group_rule_refresher.py b/tests/unit/test_group_rule_refresher.py index 1166a71..9d4aabe 100644 --- a/tests/unit/test_group_rule_refresher.py +++ b/tests/unit/test_group_rule_refresher.py @@ -13,7 +13,7 @@ OTHER_GROUP_NAME = "otherSecGroup" class TestGroupRuleRefresher(unittest.TestCase): def setUp(self): - self.new_rule = Rule('hjkl', 7, 8, '9.9.9.9/99') + self.rule = Rule('hjkl', 7, 8, '9.9.9.9/99') self.openstack_instance = Mock() self.ec2_connection = Mock(EC2Connection) @@ -27,15 +27,29 @@ class TestGroupRuleRefresher(unittest.TestCase): ) def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self): - self.openstack_rule_service.get_rules_for_group.return_value = set([self.new_rule]) + self.openstack_rule_service.get_rules_for_group.return_value = set([self.rule]) self.ec2_rule_service.get_rules_for_group.return_value = set() self.group_rule_refresher.refresh(GROUP_NAME) self.ec2_connection.authorize_security_group.assert_called_once_with( group_name=GROUP_NAME, - ip_protocol=self.new_rule.ip_protocol, - from_port=self.new_rule.from_port, - to_port=self.new_rule.to_port, - cidr_ip=self.new_rule.ip_range + ip_protocol=self.rule.ip_protocol, + from_port=self.rule.from_port, + to_port=self.rule.to_port, + cidr_ip=self.rule.ip_range + ) + + def test_should_remove_rule_from_ec2_security_group_when_rule_not_associated_with_group_on_openstack(self): + self.openstack_rule_service.get_rules_for_group.return_value = set() + self.ec2_rule_service.get_rules_for_group.return_value = set([self.rule]) + + self.group_rule_refresher.refresh(GROUP_NAME) + + self.ec2_connection.revoke_security_group.assert_called_once_with( + group_name=GROUP_NAME, + ip_protocol=self.rule.ip_protocol, + from_port=self.rule.from_port, + to_port=self.rule.to_port, + cidr_ip=self.rule.ip_range ) \ No newline at end of file From 36649ae1fc60832eff8d7077be5722385297d4a5 Mon Sep 17 00:00:00 2001 From: cameron-r Date: Thu, 6 Nov 2014 10:27:53 -0600 Subject: [PATCH 13/14] Cameron & Ed | Delete more unused rule EC2 rule args in EC2RuleTransformer, fix TestOpenstackRuleTransformer class name --- ec2_rule_transformer.py | 5 ++++- tests/unit/fake_ec2_rule_builder.py | 8 ++++++-- tests/unit/test_openstack_rule_transformer.py | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ec2_rule_transformer.py b/ec2_rule_transformer.py index a2e57ea..a83c63c 100644 --- a/ec2_rule_transformer.py +++ b/ec2_rule_transformer.py @@ -21,4 +21,7 @@ class EC2RuleTransformer: def _delete_unused_rule_args(self, rule_args): del rule_args['grants'] - del rule_args['parent'] \ No newline at end of file + del rule_args['parent'] + del rule_args['item'] + del rule_args['ipRanges'] + del rule_args['groups'] \ No newline at end of file diff --git a/tests/unit/fake_ec2_rule_builder.py b/tests/unit/fake_ec2_rule_builder.py index bf6ccb1..c0a0fdd 100644 --- a/tests/unit/fake_ec2_rule_builder.py +++ b/tests/unit/fake_ec2_rule_builder.py @@ -2,7 +2,7 @@ from collections import namedtuple class FakeEC2RuleBuilder(): - EC2Rule = namedtuple('EC2Rule', 'ip_protocol from_port to_port grants parent') + EC2Rule = namedtuple('EC2Rule', 'ip_protocol from_port to_port grants parent item ipRanges groups') GroupOrCIDR = namedtuple('GroupOrCIDR', 'cidr_ip group_id') def __init__(self): @@ -12,6 +12,9 @@ class FakeEC2RuleBuilder(): self.ip_range = '0.0.0.0/0' self.allowed_security_group_id = None self.parent = None + self.item = '\n' + self.ip_ranges = '\n' + self.groups = '' @staticmethod def an_ec2_rule(): @@ -41,4 +44,5 @@ class FakeEC2RuleBuilder(): def build(self): grants = [self.GroupOrCIDR(self.ip_range, self.allowed_security_group_id)] - return self.EC2Rule(self.ip_protocol, self.from_port, self.to_port, grants, self.parent) + return self.EC2Rule(self.ip_protocol, self.from_port, self.to_port, grants, + self.parent, self.item, self.ip_ranges, self.groups) diff --git a/tests/unit/test_openstack_rule_transformer.py b/tests/unit/test_openstack_rule_transformer.py index 6256455..887a3b2 100644 --- a/tests/unit/test_openstack_rule_transformer.py +++ b/tests/unit/test_openstack_rule_transformer.py @@ -3,7 +3,7 @@ import unittest from nova.virt.ec2.openstack_rule_transformer import OpenstackRuleTransformer -class TestEC2RuleTransformer(unittest.TestCase): +class TestOpenstackRuleTransformer(unittest.TestCase): def setUp(self): self.openstack_rule_transformer = OpenstackRuleTransformer() From b5baec56655d55c104ab0ca96dcd8b905eefc45d Mon Sep 17 00:00:00 2001 From: cameron-r Date: Thu, 6 Nov 2014 11:31:51 -0600 Subject: [PATCH 14/14] Ed & Cameron | Copy from_port and to_port as strings in EC2RuleTransformer, refactoring transformers --- ec2_rule_transformer.py | 15 +++++---------- openstack_rule_transformer.py | 13 ++++--------- tests/unit/test_openstack_rule_transformer.py | 17 +++++++++++++++-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/ec2_rule_transformer.py b/ec2_rule_transformer.py index a83c63c..5f32a3a 100644 --- a/ec2_rule_transformer.py +++ b/ec2_rule_transformer.py @@ -8,8 +8,10 @@ class EC2RuleTransformer: self.ec2_connection = ec2_connection def to_rule(self, ec2_rule): - rule_args = deepcopy(vars(ec2_rule)) - self._delete_unused_rule_args(rule_args) + rule_args = {} + rule_args['ip_protocol'] = ec2_rule.ip_protocol + rule_args['from_port'] = ec2_rule.from_port + rule_args['to_port'] = ec2_rule.to_port if ec2_rule.grants[0].cidr_ip: rule_args['ip_range'] = ec2_rule.grants[0].cidr_ip @@ -17,11 +19,4 @@ class EC2RuleTransformer: group_id = ec2_rule.grants[0].group_id rule_args['group_name'] = self.ec2_connection.get_all_security_groups(group_ids=group_id)[0].name - return Rule(**rule_args) - - def _delete_unused_rule_args(self, rule_args): - del rule_args['grants'] - del rule_args['parent'] - del rule_args['item'] - del rule_args['ipRanges'] - del rule_args['groups'] \ No newline at end of file + return Rule(**rule_args) \ No newline at end of file diff --git a/openstack_rule_transformer.py b/openstack_rule_transformer.py index 5a84cb9..300198a 100644 --- a/openstack_rule_transformer.py +++ b/openstack_rule_transformer.py @@ -3,11 +3,11 @@ from rule import Rule class OpenstackRuleTransformer: - def to_rule(self, openstack_rule): - rule_args = deepcopy(openstack_rule) - - self._delete_unused_rule_args(rule_args) + rule_args = {} + rule_args['ip_protocol'] = openstack_rule['ip_protocol'] + rule_args['from_port'] = str(openstack_rule['from_port']) + rule_args['to_port'] = str(openstack_rule['to_port']) if 'cidr' in openstack_rule['ip_range']: rule_args['ip_range'] = openstack_rule['ip_range']['cidr'] @@ -15,8 +15,3 @@ class OpenstackRuleTransformer: rule_args['group_name'] = openstack_rule['group']['name'] return Rule(**rule_args) - - def _delete_unused_rule_args(self, rule_args): - del rule_args['group'] - del rule_args['parent_group_id'] - del rule_args['id'] \ No newline at end of file diff --git a/tests/unit/test_openstack_rule_transformer.py b/tests/unit/test_openstack_rule_transformer.py index 887a3b2..b86466c 100644 --- a/tests/unit/test_openstack_rule_transformer.py +++ b/tests/unit/test_openstack_rule_transformer.py @@ -8,7 +8,7 @@ class TestOpenstackRuleTransformer(unittest.TestCase): def setUp(self): self.openstack_rule_transformer = OpenstackRuleTransformer() - def test_should_copy_to_port(self): + def test_should_copy_to_port_as_string(self): openstack_rule = { 'ip_protocol': 'abc', 'from_port': 123, @@ -19,4 +19,17 @@ class TestOpenstackRuleTransformer(unittest.TestCase): 'id': 18 } rule = self.openstack_rule_transformer.to_rule(openstack_rule) - self.assertEqual(rule.to_port, openstack_rule['to_port']) \ No newline at end of file + self.assertEqual(rule.to_port, str(openstack_rule['to_port'])) + + def test_should_copy_from_port_as_string(self): + openstack_rule = { + 'ip_protocol': 'abc', + 'from_port': 123, + 'to_port': 456, + 'group': {}, + 'parent_group_id': 55, + 'ip_range': {'cidr': '9.8.7.6/55'}, + 'id': 18 + } + rule = self.openstack_rule_transformer.to_rule(openstack_rule) + self.assertEqual(rule.from_port, str(openstack_rule['from_port'])) \ No newline at end of file