Fix broken k8s_object_rules tests. (#302)

Most of the problems occur in resource-matching logic that tries to find the right ES event to compare the expected evaluation against, which is harder to do for K8S resources than other resource types.

Inserting a per-testcase UUID into the K8S object's metadata that is being tested allows for much simpler resource-matching than the recursion logic in the `dict_contains` utility.

Author: yashtewari

tests/commonlib/docker_wrapper.py

@@ -29,4 +29,3 @@ def exec_command(self, container_name: str, command: str, param_value: str, reso
             raise ValueError(f'Failed to execute command: {command_f, output}')
 
         return output.decode().strip()
-

tests/commonlib/kubernetes.py

@@ -15,6 +15,7 @@
 RESOURCE_POD = 'Pod'
 RESOURCE_SERVICE_ACCOUNT = 'ServiceAccount'
 
+
 class KubernetesHelper:
 
     def __init__(self, is_in_cluster_config: bool = False):
@@ -192,15 +193,15 @@ def patch_resources(self, resource_type: str, **kwargs):
 
         patch_body = kwargs.pop('body')
 
-        pod = self.get_resource(resource_type, **kwargs)
+        self.get_resource(resource_type, **kwargs)
         self.delete_resources(resource_type=resource_type, **kwargs)
         deleted = self.wait_for_resource(resource_type=resource_type, status_list=['DELETED'], **kwargs)
 
         if not deleted:
-            raise ValueError(f'could not delete Pod: {kwargs}')
+            raise ValueError(f'could not delete {resource_type}: {kwargs}')
 
         return self.create_patched_resource(resource_type, patch_body)
-    
+
     def create_patched_resource(self, patch_resource_type, patch_body):
         """
         """
@@ -225,17 +226,17 @@ def create_patched_resource(self, patch_resource_type, patch_body):
             done = self.wait_for_resource(resource_type=resource_type, status_list=["RUNNING", "ADDED"], **relevant_metadata)
             if done:
                 patched_resource = created_resource
-            
+
             break
-        
+
         return patched_resource
-    
-    def patch_resource_body(self, body: Union[list,dict], patch: Union[list,dict]) -> Union[list,dict]:
+
+    def patch_resource_body(self, body: Union[list, dict], patch: Union[list, dict]) -> Union[list, dict]:
         """
         """
         if type(body) != type(patch):
             raise ValueError(f'Cannot compare {type(body)}: {body} with {type(patch)}: {patch}')
-        
+
         if isinstance(body, dict):
             for key, val in patch.items():
                 if key not in body:
@@ -245,23 +246,23 @@ def patch_resource_body(self, body: Union[list,dict], patch: Union[list,dict]) -
                         body[key] = self.patch_resource_body(body[key], val)
                     else:
                         body[key] = val
-        
+
         elif isinstance(body, list):
-            for i, val in enumerate(body):
-                if i >= len(patch):
+            for i, val in enumerate(patch):
+                if i >= len(body):
                     break
 
                 if isinstance(val, list) or isinstance(val, dict):
                     body[i] = self.patch_resource_body(body[i], val)
                 else:
                     body[i] = val
-            
+
             if len(patch) > len(body):
                 body += val[len(patch):]
-                
+
         else:
             raise ValueError(f'Invalid body {body} of type {type(body)}')
-        
+
         return body
 
     def list_resources(self, resource_type: str, **kwargs):

tests/deploy/mock-pod.yml

@@ -16,7 +16,7 @@ spec:
         - "3600"
       securityContext:
         privileged: false
-        allowPrivilegeEscalation: false
+        allowPrivilegeEscalation: true
         capabilities:
           add: [ "NET_ADMIN", "SYS_TIME" ]
 ---

tests/product/tests/data/k8s_object/k8s_object_rules.py

@@ -94,7 +94,7 @@
     resource_type='Pod',
     resource_body={
         'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE},
-        'spec': {'serviceAccount': DEFAULT, 'namespace': DEFAULT},
+        'spec': {'serviceAccount': DEFAULT},
     },
     expected=RULE_FAIL_STATUS,
 )
@@ -104,7 +104,7 @@
     resource_type='Pod',
     resource_body={
         'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE},
-        'spec': {'serviceAccountName': DEFAULT, 'namespace': DEFAULT},
+        'spec': {'serviceAccountName': DEFAULT},
     },
     expected=RULE_FAIL_STATUS,
 )
@@ -122,9 +122,6 @@
 cis_5_1_5 = {
     "5.1.5 ServiceAccount.Name == default and automountServiceAccountToken == true":
         cis_5_1_5_service_account,
-}
-
-cis_5_1_5_skip = {
     '5.1.5 Pod.serviceAccount == default': cis_5_1_5_pod_serviceAccount,
     '5.1.5 Pod.serviceAccountName == default': cis_5_1_5_pod_serviceAccountName,
 }
@@ -179,7 +176,7 @@
 
 # CIS 5.2.2
 cis_5_2_2_pod_fail = KubeTestCase(
-    rule_tag='CIS 5.2.3',
+    rule_tag='CIS 5.2.2',
     resource_type='Pod',
     resource_body={
         'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE},
@@ -196,7 +193,7 @@
 )
 
 cis_5_2_2_pod_pass = KubeTestCase(
-    rule_tag='CIS 5.2.3',
+    rule_tag='CIS 5.2.2',
     resource_type='Pod',
     resource_body={
         'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE},
@@ -239,9 +236,7 @@
 )
 
 cis_5_2_3 = {
-    '5.2.3 Pod.spec.hostPID == true': cis_5_2_3_pod_fail
-}
-cis_5_2_3_skip = {
+    '5.2.3 Pod.spec.hostPID == true': cis_5_2_3_pod_fail,
     '5.2.3 Pod.spec.hostPID == false': cis_5_2_3_pod_pass,
 }
 
@@ -267,10 +262,7 @@
 )
 
 cis_5_2_4 = {
-    '5.2.4 Pod.spec.hostIPC == true': cis_5_2_4_pod_fail
-}
-
-cis_5_2_4_skip = {
+    '5.2.4 Pod.spec.hostIPC == true': cis_5_2_4_pod_fail,
     '5.2.4 Pod.spec.hostIPC == false': cis_5_2_4_pod_pass
 }
 
@@ -296,12 +288,8 @@
 )
 
 cis_5_2_5 = {
-    '5.2.5 Pod.spec.hostNetwork == true': cis_5_2_5_pod_fail
-
-}
-
-cis_5_2_5_skip = {
-    '5.2.5 Pod.spec.hostNetwork == false': cis_5_2_5_pod_pass
+    '5.2.5 Pod.spec.hostNetwork == true': cis_5_2_5_pod_fail,
+    '5.2.5 Pod.spec.hostNetwork == false': cis_5_2_5_pod_pass,
 }
 
 # CIS 5.2.6

tests/product/tests/test_k8s_objects_rules.py

@@ -3,12 +3,12 @@
 This module verifies correctness of retrieved findings by manipulating audit and remediation actions
 """
 from datetime import datetime
+import uuid
 
 import pytest
 
-# from product.tests.data.k8s_object.k8s_object_rules import *
 from product.tests.data.k8s_object import k8s_object_rules as k8s_tc
-from commonlib.utils import get_ES_evaluation, get_resource_identifier
+from commonlib.utils import get_ES_evaluation
 from commonlib.framework.reporting import skip_param_case, SkipReportData
 
 
@@ -22,22 +22,15 @@
         *k8s_tc.cis_5_2_3.values(),
         *k8s_tc.cis_5_2_4.values(),
         *k8s_tc.cis_5_2_5.values(),
-        *skip_param_case(skip_list=[*k8s_tc.cis_5_1_5_skip.values(),
-                                    *k8s_tc.cis_5_2_2.values(),
-                                    *k8s_tc.cis_5_2_3_skip.values(),
-                                    *k8s_tc.cis_5_2_4_skip.values(),
-                                    *k8s_tc.cis_5_2_5_skip.values(),
-                                    *k8s_tc.cis_5_2_6.values(),
-                                    *k8s_tc.cis_5_2_7.values(),
-                                    *k8s_tc.cis_5_2_8.values()
-                                    ],
+        *k8s_tc.cis_5_2_2.values(),
+        *k8s_tc.cis_5_2_6.values(),
+        *k8s_tc.cis_5_2_8.values(),
+        *skip_param_case(skip_list=[*k8s_tc.cis_5_2_7.values()],
                          data_to_report=SkipReportData(
-                             url_title="security-team: #4312",
-                             url_link="https://github.com/elastic/security-team/issues/4312",
-                             skip_reason="known issue: broken k8s object tests"
+                             url_title="security-team: #4540",
+                             url_link="https://github.com/elastic/security-team/issues/4540",
+                             skip_reason="Known issue: incorrect implementation"
                          ))
-        # *k8s_tc.cis_5_2_9.values(), - TODO: cases are not implemented
-        # *k8s_tc.cis_5_2_10.values() - TODO: cases are not implemented
     ],
     ids=[
         *k8s_tc.cis_5_1_3.keys(),
@@ -46,11 +39,7 @@
         *k8s_tc.cis_5_2_3.keys(),
         *k8s_tc.cis_5_2_4.keys(),
         *k8s_tc.cis_5_2_5.keys(),
-        *k8s_tc.cis_5_1_5_skip.keys(),
         *k8s_tc.cis_5_2_2.keys(),
-        *k8s_tc.cis_5_2_3_skip.keys(),
-        *k8s_tc.cis_5_2_4_skip.keys(),
-        *k8s_tc.cis_5_2_5_skip.keys(),
         *k8s_tc.cis_5_2_6.keys(),
         *k8s_tc.cis_5_2_7.keys(),
         *k8s_tc.cis_5_2_8.keys(),
@@ -82,6 +71,11 @@ def test_kube_resource_patch(elastic_client, test_env, rule_tag, resource_type,
 
     assert resource, f"Resource {resource_type} not found"
 
+    test_resource_id = str(uuid.uuid4())
+
+    labels = metadata.setdefault('labels', {})
+    labels['test_resource_id'] = test_resource_id
+
     # patch resource
     resource = k8s_client.patch_resources(
         resource_type=resource_type,
@@ -92,13 +86,19 @@ def test_kube_resource_patch(elastic_client, test_env, rule_tag, resource_type,
         raise ValueError(
             f'Could not patch resource type {resource_type}:'
             f' {relevant_metadata} with patch {resource_body}')
-    
+
+    def match_resource(eval_resource):
+        try:
+            return eval_resource.metadata.labels.test_resource_id == test_resource_id
+        except AttributeError:
+            return False
+
     evaluation = get_ES_evaluation(
         elastic_client=elastic_client,
         timeout=agent_config.findings_timeout,
         rule_tag=rule_tag,
         exec_timestamp=datetime.utcnow(),
-        resource_identifier=get_resource_identifier(resource_body),
+        resource_identifier=match_resource,
     )
 
     assert evaluation is not None, f"No evaluation for rule {rule_tag} could be found"

tests/product/tests/test_process_controller_manager_rules.py

@@ -48,7 +48,7 @@ def test_process_controller_manager(elastic_client,
     # Wait for process reboot
     # TODO: Implement a more optimal way of waiting
     time.sleep(60)
-    
+
     evaluation = get_ES_evaluation(
         elastic_client=elastic_client,
         timeout=cloudbeat_agent.findings_timeout,

tests/product/tests/test_process_etcd_rules.py

@@ -44,7 +44,7 @@ def test_process_etcd(elastic_client,
     api_client.edit_process_file(container_name=node.metadata.name,
                                  dictionary=dictionary,
                                  resource=resource)
-    
+
     # Wait for process reboot
     # TODO: Implement a more optimal way of waiting
     time.sleep(60)