From 631364aa356c72a1a4379e09ddbe583fdd9ce479 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
(cherry picked from commit 4bfe85db0b3aa04360ae4b05d0a8ecf34f83853e)
(cherry picked from commit b7c7bb1839d8357fd29789aaf1e3de05d3e2f59d)
---
 kuryr_kubernetes/controller/drivers/utils.py  |  9 +++++---
 .../unit/controller/drivers/test_utils.py     | 23 +++++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/kuryr_kubernetes/controller/drivers/utils.py b/kuryr_kubernetes/controller/drivers/utils.py
index 88f5fd7fe..80f657bc9 100644
--- a/kuryr_kubernetes/controller/drivers/utils.py
+++ b/kuryr_kubernetes/controller/drivers/utils.py
@@ -351,14 +351,17 @@ 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'}))