Browse Source

Ed & Cameron | Refactoring: replace rule comparator with transformers and rule class equality method in rule refresher

changes/85/139985/1
cameron-r 4 years ago
parent
commit
3751616fbf
4 changed files with 86 additions and 43 deletions
  1. 1
    1
      openstack_rule_transformer.py
  2. 20
    23
      rule_refresher.py
  3. 9
    4
      tests/test_ec2_rule_transformer.py
  4. 56
    15
      tests/test_rule_refresher.py

+ 1
- 1
openstack_rule_transformer.py View File

@@ -2,7 +2,7 @@ from copy import deepcopy
2 2
 from rule import Rule
3 3
 
4 4
 
5
-class OpenStackRuleTransformer:
5
+class OpenstackRuleTransformer:
6 6
 
7 7
     def to_rule(self, openstack_rule):
8 8
         rule_args = deepcopy(openstack_rule)

+ 20
- 23
rule_refresher.py View File

@@ -1,31 +1,28 @@
1 1
 class RuleRefresher:
2 2
 
3
-    def __init__(self, openstack_group_manager, ec2_conn, rule_comparator):
3
+    def __init__(self, openstack_group_manager, ec2_conn, openstack_rule_transformer, ec2_rule_transformer):
4 4
         self.openstack_group_manager = openstack_group_manager
5 5
         self.ec2_conn = ec2_conn
6
-        self.rule_comparator = rule_comparator
6
+        self.openstack_rule_transformer = openstack_rule_transformer
7
+        self.ec2_rule_transformer = ec2_rule_transformer
7 8
 
8 9
     def refresh(self, openstack_instance):
9
-        openstack_group = self.openstack_group_manager.list()[0]
10
-        openstack_rules = openstack_group.rules
11
-
12
-        ec2_group = self.ec2_conn.get_all_security_groups()[0]
13
-        ec2_rules = ec2_group.rules
14
-
15
-        for openstack_rule in openstack_rules:
16
-            same_rule_exists_on_ec2 = False
17
-            for ec2_rule in ec2_rules:
18
-                if self.rule_comparator.rules_are_equal(openstack_rule, ec2_rule):
19
-                    same_rule_exists_on_ec2 = True
20
-                    break
21
-
22
-            if not same_rule_exists_on_ec2:
23
-                self.ec2_conn.authorize_security_group(
24
-                    group_name=openstack_group.name,
25
-                    ip_protocol=openstack_rule['ip_protocol'],
26
-                    from_port=openstack_rule['from_port'],
27
-                    to_port=openstack_rule['to_port'],
28
-                    cidr_ip=openstack_rule['ip_range']['cidr']
29
-                )
10
+        for group_dict in openstack_instance.security_groups:
11
+            openstack_group = [group for group in self.openstack_group_manager.list() if group.name == group_dict['name']][0]
12
+            ec2_group = self.ec2_conn.get_all_security_groups(groupnames=group_dict['name'])[0]
30 13
 
14
+            for openstack_rule in openstack_group.rules:
15
+                same_rule_exists_on_ec2 = False
16
+                for ec2_rule in ec2_group.rules:
17
+                    if self.openstack_rule_transformer.to_rule(openstack_rule) == self.ec2_rule_transformer.to_rule(ec2_rule):
18
+                        same_rule_exists_on_ec2 = True
19
+                        break
31 20
 
21
+                if not same_rule_exists_on_ec2:
22
+                    self.ec2_conn.authorize_security_group(
23
+                        group_name=openstack_group.name,
24
+                        ip_protocol=openstack_rule['ip_protocol'],
25
+                        from_port=openstack_rule['from_port'],
26
+                        to_port=openstack_rule['to_port'],
27
+                        cidr_ip=openstack_rule['ip_range']['cidr']
28
+                    )

+ 9
- 4
tests/test_ec2_rule_transformer.py View File

@@ -5,20 +5,25 @@ from fake_ec2_rule_builder import FakeEC2RuleBuilder
5 5
 import unittest
6 6
 
7 7
 
8
-
9 8
 class TestEC2RuleTransformer(unittest.TestCase):
10 9
 
11 10
     def setUp(self):
12 11
         self.ec2_connection = Mock()
13 12
         self.ec2_rule_transformer = EC2RuleTransformer(self.ec2_connection)
14 13
 
15
-    def test_should_copy_ip_protocol_and_port_attributes(self):
14
+    def test_should_copy_ip_protocol(self):
16 15
         ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build()
17
-
18 16
         rule = self.ec2_rule_transformer.to_rule(ec2_rule)
19
-
20 17
         self.assertEqual(rule.ip_protocol, ec2_rule.ip_protocol)
18
+
19
+    def test_should_copy_from_port(self):
20
+        ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build()
21
+        rule = self.ec2_rule_transformer.to_rule(ec2_rule)
21 22
         self.assertEqual(rule.from_port, ec2_rule.from_port)
23
+
24
+    def test_should_copy_to_port(self):
25
+        ec2_rule = FakeEC2RuleBuilder.an_ec2_rule().build()
26
+        rule = self.ec2_rule_transformer.to_rule(ec2_rule)
22 27
         self.assertEqual(rule.to_port, ec2_rule.to_port)
23 28
 
24 29
     def test_should_copy_ip_range_attribute_from_grant(self):

+ 56
- 15
tests/test_rule_refresher.py View File

@@ -8,13 +8,15 @@ import novaclient
8 8
 from novaclient.v1_1.servers import Server
9 9
 
10 10
 from nova.virt.ec2.rule_refresher import RuleRefresher
11
-from nova.virt.ec2.rule_comparator import RuleComparator
11
+from nova.virt.ec2.openstack_rule_transformer import OpenstackRuleTransformer
12
+from nova.virt.ec2.ec2_rule_transformer import EC2RuleTransformer
13
+from nova.virt.ec2.rule import Rule
12 14
 
13 15
 GROUP_NAME = 'secGroup'
14 16
 
15 17
 class TestRuleRefresher(unittest.TestCase):
16 18
     def setUp(self):
17
-        self.new_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}}
19
+        self.existing_new_rule = {'ip_protocol': 'abc', 'from_port': 1111, 'to_port': 2222, 'ip_range': {'cidr': '1.2.3.4/55'}}
18 20
 
19 21
         self.openstack_group = Mock()
20 22
         self.openstack_group.name = GROUP_NAME
@@ -31,12 +33,14 @@ class TestRuleRefresher(unittest.TestCase):
31 33
         self.ec2_connection = Mock(spec=EC2Connection)
32 34
         self.ec2_connection.get_all_security_groups.return_value = [self.ec2_group]
33 35
 
34
-        self.rule_comparator = Mock(spec=RuleComparator)
36
+        self.openstack_rule_transformer = Mock(spec=OpenstackRuleTransformer)
37
+        self.ec2_rule_transformer = Mock(spec=EC2RuleTransformer)
35 38
 
36
-        self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection, self.rule_comparator)
39
+        self.rule_refresher = RuleRefresher(self.openstack_group_manager, self.ec2_connection,
40
+                                            self.openstack_rule_transformer, self.ec2_rule_transformer)
37 41
 
38 42
     def test_should_add_rule_to_ec2_security_group_when_rule_associated_with_group_on_openstack(self):
39
-        self.openstack_group.rules = [self.new_rule]
43
+        self.openstack_group.rules = [self.existing_new_rule]
40 44
         self.ec2_group.rules = []
41 45
 
42 46
         self.rule_refresher.refresh(self.openstack_instance)
@@ -50,25 +54,62 @@ class TestRuleRefresher(unittest.TestCase):
50 54
         )
51 55
 
52 56
     def test_should_add_rule_to_ec2_security_group_when_other_rule_already_on_both(self):
53
-        existing_rule = {'ip_protocol': 'hi', 'from_port': 3333, 'to_port': 4444, 'ip_range': {'cidr': '6.7.8.9/00'}}
57
+        existing_openstack_rule = {'ip_protocol': 'hi', 'from_port': 3333, 'to_port': 4444, 'ip_range': {'cidr': '6.7.8.9/00'}}
54 58
         existing_ec2_rule = {'attribute': 'value'}
59
+        existing_transformed_rule = Rule('sdfg', 5, 6, '7.7.7.7/77')
60
+        new_transformed_rule = Rule('hjkl', 7, 8, '9.9.9.9/99')
55 61
 
56 62
         self.openstack_group.rules = [
57
-            existing_rule,
58
-            self.new_rule
63
+            existing_openstack_rule,
64
+            self.existing_new_rule
59 65
         ]
60 66
         self.ec2_group.rules = [existing_ec2_rule]
61 67
 
62
-        def mock_rules_are_equal(openstack_rule, ec2_rule):
63
-            return openstack_rule == existing_rule
64
-        self.rule_comparator.rules_are_equal.side_effect = mock_rules_are_equal
68
+        def mock_openstack_to_rule(openstack_rule):
69
+            return existing_transformed_rule if openstack_rule == existing_openstack_rule else new_transformed_rule
70
+
71
+        def mock_ec2_to_rule(ec2_rule):
72
+            if ec2_rule == existing_ec2_rule:
73
+                return existing_transformed_rule
74
+
75
+        self.openstack_rule_transformer.to_rule.side_effect = mock_openstack_to_rule
76
+        self.ec2_rule_transformer.to_rule.side_effect = mock_ec2_to_rule
77
+
78
+        self.rule_refresher.refresh(self.openstack_instance)
79
+
80
+        self.ec2_connection.authorize_security_group.assert_called_once_with(
81
+            group_name=GROUP_NAME,
82
+            ip_protocol=self.existing_new_rule['ip_protocol'],
83
+            from_port=self.existing_new_rule['from_port'],
84
+            to_port=self.existing_new_rule['to_port'],
85
+            cidr_ip=self.existing_new_rule['ip_range']['cidr']
86
+        )
87
+
88
+    def test_should_add_rule_to_corresponding_ec2_group_when_other_groups_present(self):
89
+        openstack_group2 = Mock()
90
+        openstack_group2.name = "group2"
91
+        ec2_group2 = Mock()
92
+        ec2_group2.rules = []
93
+        self.ec2_group.rules = []
94
+
95
+        self.openstack_group.rules = [self.existing_new_rule]
96
+        openstack_group2.rules = []
97
+        self.openstack_instance.security_groups = [{'name': GROUP_NAME}, {'name': openstack_group2.name}]
98
+
99
+        self.openstack_group_manager.list.return_value = [openstack_group2, self.openstack_group]
100
+
101
+        def mock_get_all_security_groups(groupnames=None):
102
+            if groupnames == ec2_group2.name:
103
+                return [ec2_group2]
104
+            return [self.ec2_group]
105
+        self.ec2_connection.get_all_security_groups.side_effect = mock_get_all_security_groups
65 106
 
66 107
         self.rule_refresher.refresh(self.openstack_instance)
67 108
 
68 109
         self.ec2_connection.authorize_security_group.assert_called_once_with(
69 110
             group_name=GROUP_NAME,
70
-            ip_protocol=self.new_rule['ip_protocol'],
71
-            from_port=self.new_rule['from_port'],
72
-            to_port=self.new_rule['to_port'],
73
-            cidr_ip=self.new_rule['ip_range']['cidr']
111
+            ip_protocol=self.existing_new_rule['ip_protocol'],
112
+            from_port=self.existing_new_rule['from_port'],
113
+            to_port=self.existing_new_rule['to_port'],
114
+            cidr_ip=self.existing_new_rule['ip_range']['cidr']
74 115
         )

Loading…
Cancel
Save