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/11.0.7
Brian Haley 2 months ago
parent
commit
a9bc8ab1e1

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

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

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

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

Loading…
Cancel
Save