From 4bfe85db0b3aa04360ae4b05d0a8ecf34f83853e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Dulko?= <mdulko@redhat.com>
Date: Mon, 9 Nov 2020 18:37:16 +0100
Subject: [PATCH] Handle None or {} labels in match_selector()

match_selector() has a bug causing it to return True for any
podSelector with any matchLabel and pods without labels at all. This is
totally incorrect, casues issues like applying NPs to pods that should
not be affected by them and probably others.

Change-Id: I47d7e61787675252cf16a9b4ae51871d8a31dc0a
Closes-Bug: 1903067
Closes-Bug: 1903572
---
 kuryr_kubernetes/controller/drivers/utils.py  |  7 +++---
 .../unit/controller/drivers/test_utils.py     | 23 +++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/kuryr_kubernetes/controller/drivers/utils.py b/kuryr_kubernetes/controller/drivers/utils.py
index dc66ecebc..a54846349 100644
--- a/kuryr_kubernetes/controller/drivers/utils.py
+++ b/kuryr_kubernetes/controller/drivers/utils.py
@@ -410,14 +410,15 @@ def match_labels(crd_labels, labels):
 def match_selector(selector, labels):
     if selector is None:
         return True
+    if labels is None:
+        labels = {}
     crd_labels = selector.get('matchLabels', None)
     crd_expressions = selector.get('matchExpressions', None)
 
     match_exp = match_lb = True
     if crd_expressions:
-        match_exp = match_expressions(crd_expressions,
-                                      labels)
-    if crd_labels and labels:
+        match_exp = match_expressions(crd_expressions, labels)
+    if crd_labels:
         match_lb = match_labels(crd_labels, labels)
     return match_exp and match_lb
 
diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py
index 8a22b982c..52147e4cd 100644
--- a/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py
+++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py
@@ -63,3 +63,26 @@ class TestUtils(test_base.TestCase):
 
     def test_get_network_id_empty(self):
         self.assertRaises(exceptions.IntegrityError, utils.get_network_id, {})
+
+    def test_match_selector(self):
+        self.assertFalse(
+            utils.match_selector({'matchLabels': {'app': 'demo'}}, None))
+        self.assertFalse(
+            utils.match_selector({'matchLabels': {'app': 'demo'}}, {}))
+        self.assertFalse(
+            utils.match_selector({'matchLabels': {'app': 'demo'}},
+                                 {'app': 'foobar'}))
+        self.assertTrue(
+            utils.match_selector({'matchLabels': {'app': 'demo'}},
+                                 {'app': 'demo'}))
+        self.assertTrue(
+            utils.match_selector({'matchLabels': {'app': 'demo'}},
+                                 {'app': 'demo', 'foo': 'bar'}))
+        self.assertTrue(
+            utils.match_selector({'matchLabels': {'app': 'demo',
+                                                  'foo': 'bar'}},
+                                 {'app': 'demo', 'foo': 'bar'}))
+        self.assertFalse(
+            utils.match_selector({'matchLabels': {'app': 'demo',
+                                                  'foo': 'bar'}},
+                                 {'app': 'demo'}))