Browse Source

Fix KeyError in OVS firewall

When merging port ranges, the code never assumed the
conjunction ID might not be present in the set due to
already being removed.

In this case there were two security groups, both using
the same remote security group, but the first security
group does not define a port range and the second one does.
Or more generally, the first SG port range is a subset
of the second, as no port-range means the full range.

Change-Id: I17ab643abbd2ec21eda4ae1dfb9abf2d4b0657f2
Closes-bug: #1813007
(cherry picked from commit 18c578aa10)
tags/14.0.1
Brian Haley 2 months ago
parent
commit
f6652f0ee6

+ 11
- 5
neutron/agent/linux/openvswitch_firewall/rules.py View File

@@ -121,26 +121,32 @@ def merge_port_ranges(rule_conj_list):
121 121
     # item means a removal.
122 122
     result = []
123 123
     rule_tmpl = rule_conj_list[0][0]
124
-    cur_conj = set()
124
+    cur_conj = {}
125 125
     cur_range_min = None
126 126
     for port, m, conj_id in port_ranges:
127 127
         if m == 'min':
128
+            if conj_id in cur_conj:
129
+                cur_conj[conj_id] += 1
130
+                continue
128 131
             if cur_conj and cur_range_min != port:
129 132
                 rule = rule_tmpl.copy()
130 133
                 rule['port_range_min'] = cur_range_min
131 134
                 rule['port_range_max'] = port - 1
132
-                result.append((rule, list(cur_conj)))
135
+                result.append((rule, list(cur_conj.keys())))
133 136
             cur_range_min = port
134
-            cur_conj.add(conj_id)
137
+            cur_conj[conj_id] = 1
135 138
         else:
139
+            if cur_conj[conj_id] > 1:
140
+                cur_conj[conj_id] -= 1
141
+                continue
136 142
             if cur_range_min <= port:
137 143
                 rule = rule_tmpl.copy()
138 144
                 rule['port_range_min'] = cur_range_min
139 145
                 rule['port_range_max'] = port
140
-                result.append((rule, list(cur_conj)))
146
+                result.append((rule, list(cur_conj.keys())))
141 147
                 # The next port range without 'port' starts from (port + 1)
142 148
                 cur_range_min = port + 1
143
-            cur_conj.remove(conj_id)
149
+            del cur_conj[conj_id]
144 150
 
145 151
     if (len(result) == 1 and result[0][0]['port_range_min'] == 1 and
146 152
             result[0][0]['port_range_max'] == 65535):

+ 11
- 2
neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py View File

@@ -420,8 +420,8 @@ class TestMergeRules(base.BaseTestCase):
420 420
         self.assertEqual(len(expected), len(result))
421 421
         for (range_min, range_max, conj_ids), result1 in zip(
422 422
                 expected, result):
423
-            self.assertEqual(range_min, result1[0]['port_range_min'])
424
-            self.assertEqual(range_max, result1[0]['port_range_max'])
423
+            self.assertEqual(range_min, result1[0].get('port_range_min'))
424
+            self.assertEqual(range_max, result1[0].get('port_range_max'))
425 425
             self.assertEqual(conj_ids, set(result1[1]))
426 426
 
427 427
     def test__assert_mergeable_rules(self):
@@ -489,6 +489,15 @@ class TestMergeRules(base.BaseTestCase):
489 489
                 (30, 40, {10, 12, 4}),
490 490
                 (41, 65535, {10, 12})], result)
491 491
 
492
+    def test_merge_port_ranges_no_port_ranges_same_conj_id(self):
493
+        result = rules.merge_port_ranges(
494
+            [(dict(self.rule_tmpl), 10),
495
+             (dict(self.rule_tmpl), 12),
496
+             (dict([('port_range_min', 30), ('port_range_max', 30)] +
497
+                   self.rule_tmpl), 10)])
498
+        self._test_merge_port_ranges_helper([
499
+                (None, None, {10, 12})], result)
500
+
492 501
     def test_merge_port_ranges_nonoverlapping(self):
493 502
         result = rules.merge_port_ranges(
494 503
             [(dict([('port_range_min', 30), ('port_range_max', 40)] +

Loading…
Cancel
Save