From 1a65b5fa09915e3cf7590130fef4ede95d9af88b Mon Sep 17 00:00:00 2001 From: Jordi Pique Date: Sun, 25 Aug 2024 12:41:30 +0200 Subject: [PATCH] Ability to set multiple webhooks on the same path If we define different webhooks with the same value on the "path" field, then these webhooks will be called in sequence when the server receives a request for the given path. The resulting "accept" responses will be ANDed and the patches will be concatenated. Notice that a given webhook will see the patches from the previous webhooks already applied to the object that it must inspect. --- README.md | 2 + generic_k8s_webhook/http_server.py | 51 ++++++++----- generic_k8s_webhook/webhook.py | 5 +- tests/http_server_test.py | 3 +- tests/http_server_test_data/test_case_4.yaml | 75 ++++++++++++++++++++ 5 files changed, 112 insertions(+), 24 deletions(-) create mode 100644 tests/http_server_test_data/test_case_4.yaml diff --git a/README.md b/README.md index b1bb988..bb9f0bf 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,8 @@ actions: ``` +If more than one webhook have the same path, they will be called in order. The `accept` responses are ANDed and the `patch` responses are concatenated. Notice that a given webhook will receive the payload already modified by all the previous webhooks that have the same path. + The syntax of the `condition` can be found in [Defining a condition](#defining-a-condition). The syntax of the patch can be found in [Defining a patch](#defining-a-patch). ### Testing the `GenericWebhookConfig` file is correct diff --git a/generic_k8s_webhook/http_server.py b/generic_k8s_webhook/http_server.py index 2863e64..689d409 100644 --- a/generic_k8s_webhook/http_server.py +++ b/generic_k8s_webhook/http_server.py @@ -79,28 +79,33 @@ def do_POST(self): def _do_post(self): logging.info(f"Processing request from {self.address_string()}") - request_served = False - for webhook in self.CONFIG_LOADER.get_webhooks(): - if self._get_path() == webhook.path: - content_length = int(self.headers["Content-Length"]) - raw_body = self.rfile.read(content_length) - body = json.loads(raw_body) - request = body["request"] - - uid = request["uid"] - accept, patch = webhook.process_manifest(request["object"]) - response = self._generate_response(uid, accept, patch) - - self.send_response(200) - self.end_headers() - self.wfile.write(json.dumps(response).encode("utf-8")) - - request_served = True + webhook_paths = [webhook.path for webhook in self.CONFIG_LOADER.get_webhooks()] - if not request_served: + # The path in the url is not defined in this server + if self._get_path() not in webhook_paths: self.send_response(400) self.end_headers() - logging.error(f"Wrong path {self.path}") + logging.error(f"Wrong path {self.path} Not defined") + return + + request = self._get_body_request() + uid = request["uid"] + # Calling in order all the webhooks that have the target path. They all must set accept=True to + # accept the request. The patches are concatenated and applied for the next call to "process_manifest" + final_patch = jsonpatch.JsonPatch([]) + for webhook in self.CONFIG_LOADER.get_webhooks(): + if self._get_path() == webhook.path: + # The call to the current webhook needs a json object that has been updated by the previous patches + patched_object = final_patch.apply(request["object"]) + accept, patch = webhook.process_manifest(patched_object) + final_patch = jsonpatch.JsonPatch(list(final_patch) + list(patch)) + if not accept: + break + + response = self._generate_response(uid, accept, final_patch) + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(response).encode("utf-8")) def _generate_response(self, uid: str, accept: bool, patch: jsonpatch.JsonPatch) -> dict: response = { @@ -122,6 +127,14 @@ def _get_path(self) -> str: parsed_url = urlparse(self.path) return parsed_url.path + def _get_body_request(self) -> dict: + """Returns the "request" field of the body of the current request""" + content_length = int(self.headers["Content-Length"]) + raw_body = self.rfile.read(content_length) + body = json.loads(raw_body) + request = body["request"] + return request + class Server: def __init__( # pylint: disable=too-many-arguments diff --git a/generic_k8s_webhook/webhook.py b/generic_k8s_webhook/webhook.py index 19d4b2e..a16553d 100644 --- a/generic_k8s_webhook/webhook.py +++ b/generic_k8s_webhook/webhook.py @@ -14,9 +14,6 @@ def check_condition(self, manifest: dict) -> bool: return self.condition.get_value([manifest]) def get_patches(self, json_payload: dict) -> jsonpatch.JsonPatch | None: - if not self.list_jpatch_op: - return None - # 1. Generate a json patch specific for the json_payload # 2. Update the json_payload based on that patch # 3. Extract the raw patch, so we can merge later all the patches into a single JsonPatch object @@ -42,4 +39,4 @@ def process_manifest(self, manifest: dict) -> tuple[bool, jsonpatch.JsonPatch | return action.accept, patches # If no condition is met, we'll accept the manifest without any patch - return True, None + return True, jsonpatch.JsonPatch([]) diff --git a/tests/http_server_test.py b/tests/http_server_test.py index fe33f28..9dca3ce 100644 --- a/tests/http_server_test.py +++ b/tests/http_server_test.py @@ -18,7 +18,8 @@ @pytest.mark.parametrize( ("name_test", "req", "webhook_config", "expected_response"), load_test_case(os.path.join(HTTP_SERVER_TEST_DATA_DIR, "test_case_1.yaml")) - + load_test_case(os.path.join(HTTP_SERVER_TEST_DATA_DIR, "test_case_3.yaml")), + + load_test_case(os.path.join(HTTP_SERVER_TEST_DATA_DIR, "test_case_3.yaml")) + + load_test_case(os.path.join(HTTP_SERVER_TEST_DATA_DIR, "test_case_4.yaml")), ) def test_http_server(name_test, req, webhook_config, expected_response, tmp_path): webhook_config_file = tmp_path / "webhook_config.yaml" diff --git a/tests/http_server_test_data/test_case_4.yaml b/tests/http_server_test_data/test_case_4.yaml new file mode 100644 index 0000000..2052275 --- /dev/null +++ b/tests/http_server_test_data/test_case_4.yaml @@ -0,0 +1,75 @@ +request: + path: /my-webhook + body: + apiVersion: admission.k8s.io/v1 + kind: AdmissionReview + request: + uid: "1234" + object: + apiVersion: v1 + kind: ServiceAccount + metadata: + name: experimental-descheduler + namespace: kube-system + labels: + app.kubernetes.io/name: descheduler + app.kubernetes.io/version: "0.27.1" + +webhook_config: + apiVersion: generic-webhook/v1beta1 + kind: GenericWebhookConfig + webhooks: + - name: my-webhook-1 + path: /my-webhook + actions: + # Refuse the request if the name is "random-name" + - condition: .metadata.name == "random-name" + accept: false + # Otherwise, accept it and add a label + - accept: true + patch: + - op: add + path: .metadata.labels + value: + myLabel: myValue + + - name: my-webhook-2 + path: /my-webhook + actions: + # Add another label if myLabel is present. This only happens if the previous + # call to my-webhook-1 went through the second action + - condition: .metadata.labels.myLabel == "myValue" + patch: + - op: add + path: .metadata.labels + value: + otherLabel: otherValue + +cases: + - patches: [] + expected_response: + apiVersion: admission.k8s.io/v1 + kind: AdmissionReview + response: + uid: "1234" + allowed: True + patchType: JSONPatch + patch: + - op: add + path: /metadata/labels + value: + myLabel: myValue + - op: add + path: /metadata/labels + value: + otherLabel: otherValue + + - patches: + - key: [request, body, request, object, metadata, name] + value: random-name + expected_response: + apiVersion: admission.k8s.io/v1 + kind: AdmissionReview + response: + uid: "1234" + allowed: False