From d659eaeb4abddc45e6dea4a4b1d6019a4ecf2c3b Mon Sep 17 00:00:00 2001 From: Luis Perez Date: Wed, 25 Sep 2024 09:23:58 -0700 Subject: [PATCH 1/3] Only drop capabilities that are not added It appears that containerd (or k8s 1.24?) have changed the behavior around adding/dropping linux capabilities and added caps no longer take precedence over dropped ones --- paasta_tools/kubernetes_tools.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/paasta_tools/kubernetes_tools.py b/paasta_tools/kubernetes_tools.py index 1cb8b743b5..b1627a410c 100644 --- a/paasta_tools/kubernetes_tools.py +++ b/paasta_tools/kubernetes_tools.py @@ -1396,7 +1396,16 @@ def get_security_context(self) -> Optional[V1SecurityContext]: return V1SecurityContext(capabilities=V1Capabilities(drop=CAPS_DROP)) else: return V1SecurityContext( - capabilities=V1Capabilities(add=cap_add, drop=CAPS_DROP) + # XXX: we should probably generally work in sets, but V1Capabilities is typed as accepting + # lists of string only + capabilities=V1Capabilities( + add=cap_add, + # NOTE: this is necessary as containerd differs in behavior from dockershim: in dockershim + # dropped capabilities were overriden if the same capability was added - but in containerd + # the dropped capabilities appear to have higher priority. + # (or maybe this is a k8s behavior change?) + drop=list(set(CAPS_DROP) - set(cap_add)), + ) ) def get_kubernetes_containers( From 849179db3983c4b798272cde8439e6ffd18975cb Mon Sep 17 00:00:00 2001 From: Luis Perez Date: Wed, 25 Sep 2024 09:30:15 -0700 Subject: [PATCH 2/3] Update security context tests --- tests/test_kubernetes_tools.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_kubernetes_tools.py b/tests/test_kubernetes_tools.py index a43c1ba0a1..7b8818d350 100644 --- a/tests/test_kubernetes_tools.py +++ b/tests/test_kubernetes_tools.py @@ -1067,8 +1067,9 @@ def test_get_security_context_without_cap_add(self): def test_get_security_context_with_cap_add(self): self.deployment.config_dict["cap_add"] = ["SETGID"] + expected_dropped_caps = list(set(CAPS_DROP) - {"SETGID"}) expected_security_context = V1SecurityContext( - capabilities=V1Capabilities(add=["SETGID"], drop=CAPS_DROP) + capabilities=V1Capabilities(add=["SETGID"], drop=expected_dropped_caps) ) assert self.deployment.get_security_context() == expected_security_context From 5fa4cd1b13bd7d39bfb3f0c1803f514fe4230f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20P=C3=A9rez?= Date: Wed, 25 Sep 2024 12:34:21 -0400 Subject: [PATCH 3/3] this is definitely containerd --- paasta_tools/kubernetes_tools.py | 1 - 1 file changed, 1 deletion(-) diff --git a/paasta_tools/kubernetes_tools.py b/paasta_tools/kubernetes_tools.py index b1627a410c..a61c9cfcdb 100644 --- a/paasta_tools/kubernetes_tools.py +++ b/paasta_tools/kubernetes_tools.py @@ -1403,7 +1403,6 @@ def get_security_context(self) -> Optional[V1SecurityContext]: # NOTE: this is necessary as containerd differs in behavior from dockershim: in dockershim # dropped capabilities were overriden if the same capability was added - but in containerd # the dropped capabilities appear to have higher priority. - # (or maybe this is a k8s behavior change?) drop=list(set(CAPS_DROP) - set(cap_add)), ) )