From 8c7af4165e6d48783e93118fc21aa961b2735a00 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Mon, 19 Aug 2024 14:23:03 -0400 Subject: [PATCH 01/11] Add resources for assign-class-label mutating webhook Signed-off-by: Isaiah Stapleton --- .gitignore | 5 + .../src/python/assign-class-label/Dockerfile | 11 ++ .../src/python/assign-class-label/mutate.py | 106 ++++++++++++++++++ .../assign-class-label/requirements.txt | 4 + docker/src/python/assign-class-label/wsgi.py | 4 + webhooks/assign-class-label/deployment.yaml | 39 +++++++ .../assign-class-label/kustomization.yaml | 13 +++ webhooks/assign-class-label/role.yaml | 8 ++ webhooks/assign-class-label/rolebinding.yaml | 12 ++ webhooks/assign-class-label/service.yaml | 12 ++ .../assign-class-label/serviceaccount.yaml | 5 + .../assign-class-label/webhook-config.yaml | 25 +++++ 12 files changed, 244 insertions(+) create mode 100644 docker/src/python/assign-class-label/Dockerfile create mode 100644 docker/src/python/assign-class-label/mutate.py create mode 100644 docker/src/python/assign-class-label/requirements.txt create mode 100644 docker/src/python/assign-class-label/wsgi.py create mode 100644 webhooks/assign-class-label/deployment.yaml create mode 100644 webhooks/assign-class-label/kustomization.yaml create mode 100644 webhooks/assign-class-label/role.yaml create mode 100644 webhooks/assign-class-label/rolebinding.yaml create mode 100644 webhooks/assign-class-label/service.yaml create mode 100644 webhooks/assign-class-label/serviceaccount.yaml create mode 100644 webhooks/assign-class-label/webhook-config.yaml diff --git a/.gitignore b/.gitignore index 68bc17f..3817e2b 100644 --- a/.gitignore +++ b/.gitignore @@ -158,3 +158,8 @@ cython_debug/ # and can be added to the global gitignore or merged into this file. For a more nuclear # option (not recommended) you can uncomment the following to ignore the entire idea folder. #.idea/ + + +webhook.crt +webhook.key +webhooks/assign-class-label/secret.yaml \ No newline at end of file diff --git a/docker/src/python/assign-class-label/Dockerfile b/docker/src/python/assign-class-label/Dockerfile new file mode 100644 index 0000000..b4a8a80 --- /dev/null +++ b/docker/src/python/assign-class-label/Dockerfile @@ -0,0 +1,11 @@ +FROM python:3.9-slim + +WORKDIR /app + +COPY . /app + +RUN pip install -r requirements.txt + +EXPOSE 8443 + +CMD ["gunicorn", "wsgi:webhook", "--log-level=info", "--workers", "3", "--bind", "0.0.0.0:8443", "--keyfile", "/certs/webhook.key", "--certfile", "/certs/webhook.crt"] diff --git a/docker/src/python/assign-class-label/mutate.py b/docker/src/python/assign-class-label/mutate.py new file mode 100644 index 0000000..ca699aa --- /dev/null +++ b/docker/src/python/assign-class-label/mutate.py @@ -0,0 +1,106 @@ +import logging +import json +import base64 +import os +from flask import Flask, request, jsonify +from kubernetes import config, client +from openshift.dynamic import DynamicClient + +webhook = Flask(__name__) + +LOG = logging.getLogger(__name__) +logging.basicConfig(level=logging.INFO) + +try: + config.load_incluster_config() +except config.ConfigException as e: + LOG.error("Could not configure Kubernetes client: %s", str(e)) + exit(1) + +k8s_client = client.ApiClient() +dyn_client = DynamicClient(k8s_client) + + +def assign_class_label(pod, groups): + # Extract pod metadata + pod_metadata = pod.get('metadata', {}) + pod_labels = pod_metadata.get('labels', {}) + pod_user = pod_labels.get("opendatahub.io/user", None) + + # Iterate through classes + for group in groups: + try: + # Get users in group (class) + group_resource = dyn_client.resources.get(api_version='user.openshift.io/v1', kind='Group') + group_obj = group_resource.get(name=group) + group_obj_dict = group_obj.to_dict() + group_users = group_obj_dict.get('users', []) + + # Check if group has no users + if not group_users: + LOG.warning(f"Group {group} has no users or users attribute is not a list.") + continue + + # Compare users in the groups (classes) with the pod user + if pod_user in group_users: + LOG.info(f"Assigning class label: {group} to user {pod_user}") + return group + except Exception as e: + LOG.error(f"Error fetching group {group}: {str(e)}") + continue + + return None + +@webhook.route('/mutate', methods=['POST']) +def mutate_pod(): + # Grab pod for mutation + request_info = request.get_json() + uid = request_info["request"]["uid"] + pod = request_info["request"]["object"] + + groups = os.environ.get("GROUPS").split(",") + + if not groups: + LOG.error("GROUPS environment variables are required.") + exit(1) + + # Grab class that the pod user belongs to + class_label = assign_class_label(pod, groups) + + # If user not in any class, return without modifications + if not class_label: + return jsonify({ + "apiVersion": "admission.k8s.io/v1", + "kind": "AdmissionReview", + "response": { + "uid": uid, + "allowed": True, + "status": {"message": "No class label assigned."} + } + }) + + # Generate JSON Patch to add class label + patch = [{ + "op": "add", + "path": "/metadata/labels/class", + "value": class_label + }] + + # Encode patch as base64 for response + patch_base64 = base64.b64encode(json.dumps(patch).encode('utf-8')).decode('utf-8') + + # Return webhook response that includes the patch to add class label + return jsonify({ + "apiVersion": "admission.k8s.io/v1", + "kind": "AdmissionReview", + "response": { + "uid": uid, + "allowed": True, + "patchType": "JSONPatch", + "patch": patch_base64 + } + }) + +if __name__ == "__main__": + logging.basicConfig(level="INFO") + webhook.run(host='0.0.0.0', port=8443) diff --git a/docker/src/python/assign-class-label/requirements.txt b/docker/src/python/assign-class-label/requirements.txt new file mode 100644 index 0000000..35f5f44 --- /dev/null +++ b/docker/src/python/assign-class-label/requirements.txt @@ -0,0 +1,4 @@ +flask +kubernetes +openshift +gunicorn diff --git a/docker/src/python/assign-class-label/wsgi.py b/docker/src/python/assign-class-label/wsgi.py new file mode 100644 index 0000000..4d9cb82 --- /dev/null +++ b/docker/src/python/assign-class-label/wsgi.py @@ -0,0 +1,4 @@ +from mutate import webhook + +if __name__ == "__main__": + webhook.run() \ No newline at end of file diff --git a/webhooks/assign-class-label/deployment.yaml b/webhooks/assign-class-label/deployment.yaml new file mode 100644 index 0000000..2ac394e --- /dev/null +++ b/webhooks/assign-class-label/deployment.yaml @@ -0,0 +1,39 @@ +kind: Deployment +apiVersion: apps/v1 +metadata: + name: assign-class-label-webhook + labels: + app: assign-class-label +spec: + replicas: 1 + selector: + matchLabels: + app: assign-class-label + template: + metadata: + labels: + app: assign-class-label + spec: + containers: + - name: assign-class-label + image: quay.io/rh-ee-istaplet/ope-webhooks:assign-class-label-webhook + imagePullPolicy: Always + ports: + - containerPort: 443 + volumeMounts: + - name: cert + mountPath: /certs + readOnly: true + resources: + limits: + cpu: 500m + memory: 512Mi + env: + # EDIT VALUE HERE BEFORE RUNNING, must be comma separated + - name: GROUPS + value: "cs210,cs506,ee440" + serviceAccountName: webhook-sa + volumes: + - name: cert + secret: + secretName: webhook-cert diff --git a/webhooks/assign-class-label/kustomization.yaml b/webhooks/assign-class-label/kustomization.yaml new file mode 100644 index 0000000..dd142d6 --- /dev/null +++ b/webhooks/assign-class-label/kustomization.yaml @@ -0,0 +1,13 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +namespace: rhods-notebooks +commonLabels: + app: ope-class-mutating-webhook + +resources: + - deployment.yaml + - service.yaml + - webhook-config.yaml + - serviceaccount.yaml + - role.yaml + - rolebinding.yaml diff --git a/webhooks/assign-class-label/role.yaml b/webhooks/assign-class-label/role.yaml new file mode 100644 index 0000000..5640af6 --- /dev/null +++ b/webhooks/assign-class-label/role.yaml @@ -0,0 +1,8 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: ope-webhook-role +rules: +- apiGroups: ["user.openshift.io"] + resources: ["pods", "groups"] + verbs: ["get", "list", "watch", "patch"] diff --git a/webhooks/assign-class-label/rolebinding.yaml b/webhooks/assign-class-label/rolebinding.yaml new file mode 100644 index 0000000..48cebf2 --- /dev/null +++ b/webhooks/assign-class-label/rolebinding.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: ope-webhook-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: ope-webhook-role +subjects: +- kind: ServiceAccount + name: webhook-sa + namespace: rhods-notebooks diff --git a/webhooks/assign-class-label/service.yaml b/webhooks/assign-class-label/service.yaml new file mode 100644 index 0000000..fa6f57c --- /dev/null +++ b/webhooks/assign-class-label/service.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: Service +metadata: + name: assign-class-label-webhook +spec: + selector: + app: assign-class-label-webhook + ports: + - name: https + protocol: TCP + port: 443 + targetPort: 8443 diff --git a/webhooks/assign-class-label/serviceaccount.yaml b/webhooks/assign-class-label/serviceaccount.yaml new file mode 100644 index 0000000..86983aa --- /dev/null +++ b/webhooks/assign-class-label/serviceaccount.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: webhook-sa + namespace: rhods-notebooks diff --git a/webhooks/assign-class-label/webhook-config.yaml b/webhooks/assign-class-label/webhook-config.yaml new file mode 100644 index 0000000..5b17a87 --- /dev/null +++ b/webhooks/assign-class-label/webhook-config.yaml @@ -0,0 +1,25 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: assign-class-label-webhook +webhooks: +- name: assign-class-label-webhook.nerc.com + clientConfig: + service: + namespace: rhods-notebooks + name: assign-class-label-webhook + path: /mutate + caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVRRENDQXlpZ0F3SUJBZ0lVU3JnWEl4OUpON21wY3FnLzRsMWg5bFZNMFBNd0RRWUpLb1pJaHZjTkFRRUwKQlFBd2daQXhDekFKQmdOVkJBWVRBbFZUTVJZd0ZBWURWUVFJREExTllYTnpZV05vZFhOelpYUnpNUTh3RFFZRApWUVFIREFaQ2IzTjBiMjR4RURBT0JnTlZCQW9NQjFKbFpDQklZWFF4RVRBUEJnTlZCQXNNQ0ZKbGMyVmhjbU5vCk1ROHdEUVlEVlFRRERBWkpjMkZwWVdneElqQWdCZ2txaGtpRzl3MEJDUUVXRTJsemRHRndiR1YwUUhKbFpHaGgKZEM1amIyMHdIaGNOTWpRd09ERTVNVGN4TnpReFdoY05NamN3TmpBNU1UY3hOelF4V2pDQmtERUxNQWtHQTFVRQpCaE1DVlZNeEZqQVVCZ05WQkFnTURVMWhjM05oWTJoMWMzTmxkSE14RHpBTkJnTlZCQWNNQmtKdmMzUnZiakVRCk1BNEdBMVVFQ2d3SFVtVmtJRWhoZERFUk1BOEdBMVVFQ3d3SVVtVnpaV0Z5WTJneER6QU5CZ05WQkFNTUJrbHoKWVdsaGFERWlNQ0FHQ1NxR1NJYjNEUUVKQVJZVGFYTjBZWEJzWlhSQWNtVmthR0YwTG1OdmJUQ0NBU0l3RFFZSgpLb1pJaHZjTkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFJbEJnaXZMVVZWWENmOTIyV2JHN3Fwcm9MZUgvODlQCkNVbHJLSTFISUNYRW1tZTRKRHJvQzRIV1c0QW4vS1VoVW5pNkpWbHhjaE02dWJMeU5sNjBmRVRERWJBeHgzWVUKa1RHa01JNGxWS0VlVGlpcFJVQUdwQjg0VXBEbi82cnpwQUJqT2xoakFmMUNZcnhzSVJvc1o2L1ZVcG9veTdOZApiYllIdlFTMmd1N2IwODAyS2YrWko2bUpPbGlIcUdtVjRqVlRJK0RPem15RXlhMmhwSXpHMnZOQ0JIcDJVZXN2CkdaMXJ0dE8xOWs3K1AyQXdoZFVUMk14Z2NEeUdaSjF5N2ZuR25JbXczRGtEL2FKWUp3bklHaWhaMnNSZTlsbjgKbGVsRVN3YWx3S2NPTlZRcnB3QlJzdC8ySXdhdlpyd1dNQVg0cUpEeks3dWJyc3FJdkZZcUh6MENBd0VBQWFPQgpqekNCakRBZEJnTlZIUTRFRmdRVXpDWUswU2tkbm02S0lNbnFiSGF1QnlSOWxrb3dId1lEVlIwakJCZ3dGb0FVCnpDWUswU2tkbm02S0lNbnFiSGF1QnlSOWxrb3dEd1lEVlIwVEFRSC9CQVV3QXdFQi96QTVCZ05WSFJFRU1qQXcKZ2k1aGMzTnBaMjR0WTJ4aGMzTXRiR0ZpWld3dGQyVmlhRzl2YXk1eWFHOWtjeTF1YjNSbFltOXZhM011YzNaagpNQTBHQ1NxR1NJYjNEUUVCQ3dVQUE0SUJBUUI3eVpGM1k5ZWFkTDR5Y1R3cHJsem5JT2IreUJncGxkOFdlNndqCisvWE9XVnlXb09rM1JYT0VadWoxUE8xcWpHY0lJOFpQc05xamJmNVhOd1hkaUlsSm1KdDJaQVFWWkpqY0dkMmQKS2RRNXF1Wk44Z2ZFS0pwWHJUVWlIZ3VQQjdCNmd5dGVBYWlyaXl4RDllZmgzcG1HTGJaU0xscXlRcVBrUFZ6UwpUeXBIbDNPVHllaE8wKy82MWh0MnE1TVpYamd4Rk1CQTlqNXZKd2JUMlVxWFllSGFOOTlqcU0rTW10Zks2NUUxCjkxQUY2bndmQys0Z1FNS0tUQ0hpVkFWb09KakJBVjZyOXkydSs5a0xyNE9LUkFaWEF4alI3dFl1YitsQmJYZUsKSHpOdnBYajBLZmpBaG9BNkdPUW1MVG01UGRNZjN2dWhlS1FFdmp1L3lldzZTMlk5Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K + rules: + - operations: ["CREATE"] + apiGroups: [""] + apiVersions: ["v1"] + resources: ["pods"] + namespaceSelector: + matchExpressions: + - key: kubernetes.io/metadata.name + operator: In + values: + - rhods-notebooks + sideEffects: None + admissionReviewVersions: ["v1"] From e1219e7ee83a30cbd3f8cd5832f1c25ec8c300f5 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Mon, 19 Aug 2024 15:20:52 -0400 Subject: [PATCH 02/11] Update README to include info about assign-class-label webhook Signed-off-by: Isaiah Stapleton --- README.md | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f542d99..df7e673 100644 --- a/README.md +++ b/README.md @@ -100,4 +100,71 @@ This script is used to retrieve the URL for a particular notebook associated wit ``` Enter the notebook name: xxx URL for notebook xxx: xxx - ``` \ No newline at end of file + ``` + +## Webhooks + +### assign-class-label + +This script is used to add labels to the pod of a user denoting which class they belong to (class="classname"). This allows us to differentiate between users of different classes running in the same namespace. This also allows us to create validating [gatekeeper policies](https://github.com/OCP-on-NERC/gatekeeper) for each class. + +Before using the assign-class-label webhook, the group-sync cronjob should be run so that the users of the different classes are added to their respective groups in openshift. + +In order to modify the deployment follow these steps: + +1. Modify the GROUPS env variable to contain the list of classes (openshift groups) of which you would like to assign class labels. This file is found here: webhooks/assign-class-label/deployment.yaml + +2. Generate a new OpenSSL certificate + + ``` + openssl req -x509 -sha256 -newkey rsa:2048 -keyout webhook.key -out webhook.crt -days 1024 -nodes -addext "subjectAltName = DNS.1:service_name.namespace.svc" + ``` + + When deployed to rhods-notebooks the command was specified as such: + + ``` + openssl req -x509 -sha256 -newkey rsa:2048 -keyout webhook.key -out webhook.crt -days 1024 -nodes -addext "subjectAltName = DNS.1:assign-class-label-webhook.rhods-notebooks.svc" + ``` + +3. Add the cert and key to the required resources: + + ``` + cat webhook.crt | base64 | tr -d '\n' + ``` + + ``` + cat webhook.key | base64 | tr -d '\n' + ``` + + This will encode the certificate and key in base64 format which is required. Copy the output of the webhook.crt to the caBundle in webhooks/assign-class-label/webhook-config.yaml. Then create a secret.yaml that looks like this + + ``` + apiVersion: v1 + kind: Secret + metadata: + name: webhook-cert + type: Opaque + data: + webhook.crt: + webhook.key: + ``` + + Copy and paste the output of the cat command to the respective fields for webhook.crt and webhook.key. Then execute + + ``` + oc apply -f secret.yaml --as system:admin + ``` + + within the same namespace that your webhook will be deployed to. + + +4. Change namespace variable in the kubernetes manifests to match namespace you want the webhook to be deployed to. + +5. From webhooks/assign-class-label/ directory run: +``` + oc apply -k . --as system:admin +``` + +***Steps 2, 3, and 4 are only required if you are deploying to a new namespace/environment.*** + +The python script and docker image used for the webserver should not need changes made to it. But in the case that changes must be made, the Dockerfile and python script can be found at docker/src/python/assign-class-label/. From 6b598efbf501d2aa0209c719b2d8d7ad6402c7d2 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Tue, 20 Aug 2024 11:16:52 -0400 Subject: [PATCH 03/11] Run ruff check & ruff format (python formatting) Signed-off-by: Isaiah Stapleton --- .gitignore | 2 +- .ruff.toml | 18 +++++ .../src/python/assign-class-label/mutate.py | 78 ++++++++++--------- docker/src/python/assign-class-label/wsgi.py | 4 +- docker/src/python/group-sync/group-sync.py | 24 +++--- scripts/get_url.py | 23 ++++-- 6 files changed, 91 insertions(+), 58 deletions(-) create mode 100644 .ruff.toml diff --git a/.gitignore b/.gitignore index 3817e2b..956529e 100644 --- a/.gitignore +++ b/.gitignore @@ -162,4 +162,4 @@ cython_debug/ webhook.crt webhook.key -webhooks/assign-class-label/secret.yaml \ No newline at end of file +secret.yaml \ No newline at end of file diff --git a/.ruff.toml b/.ruff.toml new file mode 100644 index 0000000..aa292ea --- /dev/null +++ b/.ruff.toml @@ -0,0 +1,18 @@ +[lint] +# 1. Enable flake8-bugbear (B) rules, in addition to the defaults. +select = ["E4", "E7", "E9", "F", "B"] + +# 2. Avoid enforcing line-length violations (E501) +ignore = ["E501"] + +# 3. Avoid trying to fix flake8-bugbear (B) violations. +unfixable = ["B"] + +# 4. Ignore E402 (import violations) in all __init__.py files, and in selected subdirectories. +[lint.per-file-ignores] +"__init__.py" = ["E402"] +"**/{tests,docs,tools}/*" = ["E402"] + +[format] +# 5. Use single quotes in ruff format. +quote-style = "single" \ No newline at end of file diff --git a/docker/src/python/assign-class-label/mutate.py b/docker/src/python/assign-class-label/mutate.py index ca699aa..d81ef14 100644 --- a/docker/src/python/assign-class-label/mutate.py +++ b/docker/src/python/assign-class-label/mutate.py @@ -14,7 +14,7 @@ try: config.load_incluster_config() except config.ConfigException as e: - LOG.error("Could not configure Kubernetes client: %s", str(e)) + LOG.error('Could not configure Kubernetes client: %s', str(e)) exit(1) k8s_client = client.ApiClient() @@ -25,43 +25,48 @@ def assign_class_label(pod, groups): # Extract pod metadata pod_metadata = pod.get('metadata', {}) pod_labels = pod_metadata.get('labels', {}) - pod_user = pod_labels.get("opendatahub.io/user", None) + pod_user = pod_labels.get('opendatahub.io/user', None) # Iterate through classes for group in groups: try: # Get users in group (class) - group_resource = dyn_client.resources.get(api_version='user.openshift.io/v1', kind='Group') + group_resource = dyn_client.resources.get( + api_version='user.openshift.io/v1', kind='Group' + ) group_obj = group_resource.get(name=group) group_obj_dict = group_obj.to_dict() group_users = group_obj_dict.get('users', []) # Check if group has no users if not group_users: - LOG.warning(f"Group {group} has no users or users attribute is not a list.") + LOG.warning( + f'Group {group} has no users or users attribute is not a list.' + ) continue # Compare users in the groups (classes) with the pod user if pod_user in group_users: - LOG.info(f"Assigning class label: {group} to user {pod_user}") + LOG.info(f'Assigning class label: {group} to user {pod_user}') return group except Exception as e: - LOG.error(f"Error fetching group {group}: {str(e)}") + LOG.error(f'Error fetching group {group}: {str(e)}') continue return None + @webhook.route('/mutate', methods=['POST']) def mutate_pod(): - # Grab pod for mutation + # Grab pod for mutation request_info = request.get_json() - uid = request_info["request"]["uid"] - pod = request_info["request"]["object"] + uid = request_info['request']['uid'] + pod = request_info['request']['object'] - groups = os.environ.get("GROUPS").split(",") + groups = os.environ.get('GROUPS').split(',') if not groups: - LOG.error("GROUPS environment variables are required.") + LOG.error('GROUPS environment variables are required.') exit(1) # Grab class that the pod user belongs to @@ -69,38 +74,39 @@ def mutate_pod(): # If user not in any class, return without modifications if not class_label: - return jsonify({ - "apiVersion": "admission.k8s.io/v1", - "kind": "AdmissionReview", - "response": { - "uid": uid, - "allowed": True, - "status": {"message": "No class label assigned."} + return jsonify( + { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'uid': uid, + 'allowed': True, + 'status': {'message': 'No class label assigned.'}, + }, } - }) - + ) + # Generate JSON Patch to add class label - patch = [{ - "op": "add", - "path": "/metadata/labels/class", - "value": class_label - }] + patch = [{'op': 'add', 'path': '/metadata/labels/class', 'value': class_label}] # Encode patch as base64 for response patch_base64 = base64.b64encode(json.dumps(patch).encode('utf-8')).decode('utf-8') # Return webhook response that includes the patch to add class label - return jsonify({ - "apiVersion": "admission.k8s.io/v1", - "kind": "AdmissionReview", - "response": { - "uid": uid, - "allowed": True, - "patchType": "JSONPatch", - "patch": patch_base64 + return jsonify( + { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'uid': uid, + 'allowed': True, + 'patchType': 'JSONPatch', + 'patch': patch_base64, + }, } - }) + ) + -if __name__ == "__main__": - logging.basicConfig(level="INFO") +if __name__ == '__main__': + logging.basicConfig(level='INFO') webhook.run(host='0.0.0.0', port=8443) diff --git a/docker/src/python/assign-class-label/wsgi.py b/docker/src/python/assign-class-label/wsgi.py index 4d9cb82..b9486f8 100644 --- a/docker/src/python/assign-class-label/wsgi.py +++ b/docker/src/python/assign-class-label/wsgi.py @@ -1,4 +1,4 @@ from mutate import webhook -if __name__ == "__main__": - webhook.run() \ No newline at end of file +if __name__ == '__main__': + webhook.run() diff --git a/docker/src/python/group-sync/group-sync.py b/docker/src/python/group-sync/group-sync.py index d1b1c93..d8ab1a9 100644 --- a/docker/src/python/group-sync/group-sync.py +++ b/docker/src/python/group-sync/group-sync.py @@ -8,10 +8,10 @@ def add_users_to_group(group): # Run the `oc get` command, capture the JSON output, and load the data - rolebinding = oc.selector("rolebinding/edit").object() + rolebinding = oc.selector('rolebinding/edit').object() users_in_rolebinding = set( - subject["name"] for subject in rolebinding.model.subjects + subject['name'] for subject in rolebinding.model.subjects ) users_in_group = set(group.model.users) @@ -19,28 +19,28 @@ def add_users_to_group(group): users_to_remove = users_in_group.difference(users_in_rolebinding) group_name = group.model.metadata.name - LOG.info("adding to group %s: %s", group_name, users_to_add) - LOG.info("removing from group %s: %s", group_name, users_to_remove) - group.patch({"users": list(users_in_rolebinding)}) + LOG.info('adding to group %s: %s', group_name, users_to_add) + LOG.info('removing from group %s: %s', group_name, users_to_remove) + group.patch({'users': list(users_in_rolebinding)}) -if __name__ == "__main__": +if __name__ == '__main__': # Use environment variables for group name and namespace - group_name = os.environ.get("GROUP_NAME") - namespace_name = os.environ.get("NAMESPACE") + group_name = os.environ.get('GROUP_NAME') + namespace_name = os.environ.get('NAMESPACE') - logging.basicConfig(level="INFO") + logging.basicConfig(level='INFO') # Check if the required environment variables are present if not group_name or not namespace_name: - LOG.error("GROUP_NAME and NAMESPACE environment variables are required.") + LOG.error('GROUP_NAME and NAMESPACE environment variables are required.') sys.exit(1) # Check that group exists try: - group = oc.selector(f"group/{group_name}").object() + group = oc.selector(f'group/{group_name}').object() except oc.model.OpenShiftPythonException: - LOG.error("Unable to find group %s", group_name) + LOG.error('Unable to find group %s', group_name) sys.exit(1) # Run add_users_to_group in the given namespace diff --git a/scripts/get_url.py b/scripts/get_url.py index 95895a3..87745a4 100644 --- a/scripts/get_url.py +++ b/scripts/get_url.py @@ -1,22 +1,31 @@ import subprocess -import os import yaml + def extract_url(notebook_name): - cmd = ["oc", "get", "notebook", notebook_name, "-o", "yaml", "-n", "rhods-notebooks"] + cmd = [ + 'oc', + 'get', + 'notebook', + notebook_name, + '-o', + 'yaml', + '-n', + 'rhods-notebooks', + ] result = subprocess.run(cmd, capture_output=True, text=True) if result.returncode != 0: - print(f"Error getting YAML for notebook {notebook_name}:", result.stderr) + print(f'Error getting YAML for notebook {notebook_name}:', result.stderr) return None data = yaml.safe_load(result.stdout) url = data.get('metadata', {}).get('annotations', {}).get('opendatahub.io/link') return url -notebook_name = input("Enter the notebook name: ") + +notebook_name = input('Enter the notebook name: ') url = extract_url(notebook_name) if url: - print(f"URL for notebook {notebook_name}: {url}") + print(f'URL for notebook {notebook_name}: {url}') else: - print(f"No URL found for notebook {notebook_name}") - + print(f'No URL found for notebook {notebook_name}') From 75558d9665028a1bdb529e41ee403f4e69604778 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Wed, 21 Aug 2024 11:42:15 -0400 Subject: [PATCH 04/11] Initial changes based off of PR comments Refactored some code in mutate.py, changed directory organization, moved app labels out of individual files and into the kustomization.yaml. Changed port service is running on. Signed-off-by: Isaiah Stapleton --- .../assign-class-label/Dockerfile | 10 ++-- .../assign-class-label/mutate.py | 54 +++++++++++-------- .../assign-class-label/requirements.txt | 0 .../assign-class-label/wsgi.py | 0 .../group-sync}/Dockerfile | 0 .../group-sync/group-sync.py | 0 .../group-sync/requirements.txt | 0 webhooks/assign-class-label/deployment.yaml | 10 +--- .../assign-class-label/kustomization.yaml | 2 +- webhooks/assign-class-label/service.yaml | 4 +- 10 files changed, 40 insertions(+), 40 deletions(-) rename {docker/src/python => container_images}/assign-class-label/Dockerfile (55%) rename {docker/src/python => container_images}/assign-class-label/mutate.py (76%) rename {docker/src/python => container_images}/assign-class-label/requirements.txt (100%) rename {docker/src/python => container_images}/assign-class-label/wsgi.py (100%) rename {docker => container_images/group-sync}/Dockerfile (100%) rename {docker/src/python => container_images}/group-sync/group-sync.py (100%) rename {docker/src/python => container_images}/group-sync/requirements.txt (100%) diff --git a/docker/src/python/assign-class-label/Dockerfile b/container_images/assign-class-label/Dockerfile similarity index 55% rename from docker/src/python/assign-class-label/Dockerfile rename to container_images/assign-class-label/Dockerfile index b4a8a80..5a7a580 100644 --- a/docker/src/python/assign-class-label/Dockerfile +++ b/container_images/assign-class-label/Dockerfile @@ -1,11 +1,13 @@ FROM python:3.9-slim -WORKDIR /app +WORKDIR /app/ -COPY . /app +COPY requirements.txt /app/ RUN pip install -r requirements.txt -EXPOSE 8443 +COPY . /app/ -CMD ["gunicorn", "wsgi:webhook", "--log-level=info", "--workers", "3", "--bind", "0.0.0.0:8443", "--keyfile", "/certs/webhook.key", "--certfile", "/certs/webhook.crt"] +EXPOSE 5000 + +CMD ["gunicorn", "wsgi:webhook", "--log-level=info", "--workers", "3", "--bind", "0.0.0.0:5000", "--keyfile", "/certs/webhook.key", "--certfile", "/certs/webhook.crt"] diff --git a/docker/src/python/assign-class-label/mutate.py b/container_images/assign-class-label/mutate.py similarity index 76% rename from docker/src/python/assign-class-label/mutate.py rename to container_images/assign-class-label/mutate.py index d81ef14..2c7e114 100644 --- a/docker/src/python/assign-class-label/mutate.py +++ b/container_images/assign-class-label/mutate.py @@ -13,30 +13,47 @@ try: config.load_incluster_config() -except config.ConfigException as e: - LOG.error('Could not configure Kubernetes client: %s', str(e)) - exit(1) +except config.ConfigException: + try: + config.load_kube_config() + except config.ConfigException: + LOG.error('Could not configure Kubernetes client') + exit(1) k8s_client = client.ApiClient() dyn_client = DynamicClient(k8s_client) -def assign_class_label(pod, groups): +# Get list of classes (groups) +groups_env = os.environ.get('CLASS_GROUPS') + +if not groups_env: + LOG.error('Must set CLASS_GROUPS environment variable in deployment.yaml') + exit(1) + +groups = groups_env.split(',') + + +def assign_class_label(pod): # Extract pod metadata pod_metadata = pod.get('metadata', {}) pod_labels = pod_metadata.get('labels', {}) pod_user = pod_labels.get('opendatahub.io/user', None) + if not pod_user: + return None + + # Create group api client + group_resource = dyn_client.resources.get( + api_version='user.openshift.io/v1', kind='Group' + ) + # Iterate through classes for group in groups: try: - # Get users in group (class) - group_resource = dyn_client.resources.get( - api_version='user.openshift.io/v1', kind='Group' - ) + # Get users in class (group) group_obj = group_resource.get(name=group) - group_obj_dict = group_obj.to_dict() - group_users = group_obj_dict.get('users', []) + group_users = group_obj.users # Check if group has no users if not group_users: @@ -63,14 +80,8 @@ def mutate_pod(): uid = request_info['request']['uid'] pod = request_info['request']['object'] - groups = os.environ.get('GROUPS').split(',') - - if not groups: - LOG.error('GROUPS environment variables are required.') - exit(1) - # Grab class that the pod user belongs to - class_label = assign_class_label(pod, groups) + class_label = assign_class_label(pod) # If user not in any class, return without modifications if not class_label: @@ -87,7 +98,9 @@ def mutate_pod(): ) # Generate JSON Patch to add class label - patch = [{'op': 'add', 'path': '/metadata/labels/class', 'value': class_label}] + patch = [ + {'op': 'add', 'path': "/metadata/labels/ope_class", 'value': class_label} + ] # Encode patch as base64 for response patch_base64 = base64.b64encode(json.dumps(patch).encode('utf-8')).decode('utf-8') @@ -105,8 +118,3 @@ def mutate_pod(): }, } ) - - -if __name__ == '__main__': - logging.basicConfig(level='INFO') - webhook.run(host='0.0.0.0', port=8443) diff --git a/docker/src/python/assign-class-label/requirements.txt b/container_images/assign-class-label/requirements.txt similarity index 100% rename from docker/src/python/assign-class-label/requirements.txt rename to container_images/assign-class-label/requirements.txt diff --git a/docker/src/python/assign-class-label/wsgi.py b/container_images/assign-class-label/wsgi.py similarity index 100% rename from docker/src/python/assign-class-label/wsgi.py rename to container_images/assign-class-label/wsgi.py diff --git a/docker/Dockerfile b/container_images/group-sync/Dockerfile similarity index 100% rename from docker/Dockerfile rename to container_images/group-sync/Dockerfile diff --git a/docker/src/python/group-sync/group-sync.py b/container_images/group-sync/group-sync.py similarity index 100% rename from docker/src/python/group-sync/group-sync.py rename to container_images/group-sync/group-sync.py diff --git a/docker/src/python/group-sync/requirements.txt b/container_images/group-sync/requirements.txt similarity index 100% rename from docker/src/python/group-sync/requirements.txt rename to container_images/group-sync/requirements.txt diff --git a/webhooks/assign-class-label/deployment.yaml b/webhooks/assign-class-label/deployment.yaml index 2ac394e..034a814 100644 --- a/webhooks/assign-class-label/deployment.yaml +++ b/webhooks/assign-class-label/deployment.yaml @@ -2,17 +2,9 @@ kind: Deployment apiVersion: apps/v1 metadata: name: assign-class-label-webhook - labels: - app: assign-class-label spec: replicas: 1 - selector: - matchLabels: - app: assign-class-label template: - metadata: - labels: - app: assign-class-label spec: containers: - name: assign-class-label @@ -30,7 +22,7 @@ spec: memory: 512Mi env: # EDIT VALUE HERE BEFORE RUNNING, must be comma separated - - name: GROUPS + - name: CLASS_GROUPS value: "cs210,cs506,ee440" serviceAccountName: webhook-sa volumes: diff --git a/webhooks/assign-class-label/kustomization.yaml b/webhooks/assign-class-label/kustomization.yaml index dd142d6..5bcb16d 100644 --- a/webhooks/assign-class-label/kustomization.yaml +++ b/webhooks/assign-class-label/kustomization.yaml @@ -2,7 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization namespace: rhods-notebooks commonLabels: - app: ope-class-mutating-webhook + app: assign-class-label-webhook resources: - deployment.yaml diff --git a/webhooks/assign-class-label/service.yaml b/webhooks/assign-class-label/service.yaml index fa6f57c..06bd0d1 100644 --- a/webhooks/assign-class-label/service.yaml +++ b/webhooks/assign-class-label/service.yaml @@ -3,10 +3,8 @@ kind: Service metadata: name: assign-class-label-webhook spec: - selector: - app: assign-class-label-webhook ports: - name: https protocol: TCP port: 443 - targetPort: 8443 + targetPort: 5000 From a9af7e46bbb97b05ba5a841ed0b8cf6cb0d1cf30 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Wed, 21 Aug 2024 17:00:46 -0400 Subject: [PATCH 05/11] Make mutate.py more modular, in preparation of adding tests Signed-off-by: Isaiah Stapleton --- container_images/assign-class-label/mutate.py | 150 +++++++++--------- container_images/assign-class-label/wsgi.py | 4 +- webhooks/assign-class-label/deployment.yaml | 2 +- 3 files changed, 80 insertions(+), 76 deletions(-) diff --git a/container_images/assign-class-label/mutate.py b/container_images/assign-class-label/mutate.py index 2c7e114..0a00380 100644 --- a/container_images/assign-class-label/mutate.py +++ b/container_images/assign-class-label/mutate.py @@ -1,90 +1,111 @@ import logging import json import base64 -import os from flask import Flask, request, jsonify from kubernetes import config, client from openshift.dynamic import DynamicClient -webhook = Flask(__name__) - LOG = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) -try: - config.load_incluster_config() -except config.ConfigException: + +def get_client(): try: - config.load_kube_config() - except config.ConfigException: - LOG.error('Could not configure Kubernetes client') + config.load_config() + except config.ConfigException as e: + LOG.error('Could not configure Kubernetes client: %s', str(e)) exit(1) -k8s_client = client.ApiClient() -dyn_client = DynamicClient(k8s_client) + k8s_client = client.ApiClient() + dyn_client = DynamicClient(k8s_client) + return dyn_client -# Get list of classes (groups) -groups_env = os.environ.get('CLASS_GROUPS') -if not groups_env: - LOG.error('Must set CLASS_GROUPS environment variable in deployment.yaml') - exit(1) +# Get users of a given group +def get_group_members(group_resource, group_name): + group_obj = group_resource.get(name=group_name) + return group_obj.users -groups = groups_env.split(',') - -def assign_class_label(pod): +def assign_class_label(pod, groups, dyn_client): # Extract pod metadata pod_metadata = pod.get('metadata', {}) pod_labels = pod_metadata.get('labels', {}) pod_user = pod_labels.get('opendatahub.io/user', None) - if not pod_user: - return None - - # Create group api client - group_resource = dyn_client.resources.get( + group_resource = dyn_client.resources.get( # Need to move this group_resource, this is running every iteration of for loop api_version='user.openshift.io/v1', kind='Group' ) # Iterate through classes for group in groups: - try: - # Get users in class (group) - group_obj = group_resource.get(name=group) - group_users = group_obj.users - - # Check if group has no users - if not group_users: - LOG.warning( - f'Group {group} has no users or users attribute is not a list.' - ) - continue - - # Compare users in the groups (classes) with the pod user - if pod_user in group_users: - LOG.info(f'Assigning class label: {group} to user {pod_user}') - return group - except Exception as e: - LOG.error(f'Error fetching group {group}: {str(e)}') + users = get_group_members(group_resource, group) + + # Check if group has no users + if not users: + LOG.warning(f'Group {group} has no users or users attribute is not a list.') continue + # Compare users in the groups (classes) with the pod user + if pod_user in users: + LOG.info(f'Assigning class label: {group} to user {pod_user}') + return group + return None -@webhook.route('/mutate', methods=['POST']) -def mutate_pod(): - # Grab pod for mutation - request_info = request.get_json() - uid = request_info['request']['uid'] - pod = request_info['request']['object'] +def create_app(**config): + app = Flask(__name__) + app.config.from_prefixed_env('RHOAI_CLASS') + app.config.update(config) - # Grab class that the pod user belongs to - class_label = assign_class_label(pod) + if not app.config['GROUPS']: + LOG.error('RHOAI_CLASS_GROUPS environment variables are required.') + exit(1) + + groups = app.config['GROUPS'].split(',') + + dyn_client = ( + get_client() + ) # Moved here so not being called every for loop in assign_class_label + + @app.route('/mutate', methods=['POST']) + def mutate_pod(): + # Grab pod for mutation + request_info = request.get_json() + uid = request_info['request']['uid'] + pod = request_info['request']['object'] - # If user not in any class, return without modifications - if not class_label: + # Grab class that the pod user belongs to + try: + class_label = assign_class_label(pod, groups, dyn_client) + except Exception as err: + LOG.error('failed to assign class label: %s', err) + return 'unexpected error encountered', 500, {'content-type': 'text/plain'} + + # If user not in any class, return without modifications + if not class_label: + return jsonify( + { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'uid': uid, + 'allowed': True, + 'status': {'message': 'No class label assigned.'}, + }, + } + ) + + # Generate JSON Patch to add class label + patch = [{'op': 'add', 'path': '/metadata/labels/class', 'value': class_label}] + + # Encode patch as base64 for response + patch_base64 = base64.b64encode(json.dumps(patch).encode('utf-8')).decode( + 'utf-8' + ) + # Return webhook response that includes the patch to add class label return jsonify( { 'apiVersion': 'admission.k8s.io/v1', @@ -92,29 +113,10 @@ def mutate_pod(): 'response': { 'uid': uid, 'allowed': True, - 'status': {'message': 'No class label assigned.'}, + 'patchType': 'JSONPatch', + 'patch': patch_base64, }, } ) - # Generate JSON Patch to add class label - patch = [ - {'op': 'add', 'path': "/metadata/labels/ope_class", 'value': class_label} - ] - - # Encode patch as base64 for response - patch_base64 = base64.b64encode(json.dumps(patch).encode('utf-8')).decode('utf-8') - - # Return webhook response that includes the patch to add class label - return jsonify( - { - 'apiVersion': 'admission.k8s.io/v1', - 'kind': 'AdmissionReview', - 'response': { - 'uid': uid, - 'allowed': True, - 'patchType': 'JSONPatch', - 'patch': patch_base64, - }, - } - ) + return app diff --git a/container_images/assign-class-label/wsgi.py b/container_images/assign-class-label/wsgi.py index b9486f8..03851b3 100644 --- a/container_images/assign-class-label/wsgi.py +++ b/container_images/assign-class-label/wsgi.py @@ -1,4 +1,6 @@ -from mutate import webhook +from mutate import create_app + +webhook = create_app() if __name__ == '__main__': webhook.run() diff --git a/webhooks/assign-class-label/deployment.yaml b/webhooks/assign-class-label/deployment.yaml index 034a814..6cb1c3d 100644 --- a/webhooks/assign-class-label/deployment.yaml +++ b/webhooks/assign-class-label/deployment.yaml @@ -22,7 +22,7 @@ spec: memory: 512Mi env: # EDIT VALUE HERE BEFORE RUNNING, must be comma separated - - name: CLASS_GROUPS + - name: RHOAI_CLASS_GROUPS value: "cs210,cs506,ee440" serviceAccountName: webhook-sa volumes: From 9b09f7a19557ad61f0383e06406d7e9f1f43e1bd Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Tue, 27 Aug 2024 15:29:55 -0400 Subject: [PATCH 06/11] Reformat code, add validation, add testing Signed-off-by: Isaiah Stapleton --- .github/workflows/assign-class-label.yaml | 41 ++++ .../assign-class-label/Dockerfile | 0 .../assign-class-label/mutate.py | 104 ++++++-- .../assign-class-label/requirements.txt | 1 + .../assign-class-label/tests/requirements.txt | 2 + .../assign-class-label/tests/test_mutate.py | 232 ++++++++++++++++++ .../assign-class-label/wsgi.py | 0 .../group-sync/Dockerfile | 0 .../group-sync/group-sync.py | 0 .../group-sync/requirements.txt | 0 .../assign-class-label/kustomization.yaml | 2 +- webhooks/assign-class-label/rolebinding.yaml | 2 +- .../assign-class-label/serviceaccount.yaml | 2 +- .../assign-class-label/webhook-config.yaml | 4 +- 14 files changed, 359 insertions(+), 31 deletions(-) create mode 100644 .github/workflows/assign-class-label.yaml rename {container_images => container-images}/assign-class-label/Dockerfile (100%) rename {container_images => container-images}/assign-class-label/mutate.py (53%) rename {container_images => container-images}/assign-class-label/requirements.txt (80%) create mode 100644 container-images/assign-class-label/tests/requirements.txt create mode 100644 container-images/assign-class-label/tests/test_mutate.py rename {container_images => container-images}/assign-class-label/wsgi.py (100%) rename {container_images => container-images}/group-sync/Dockerfile (100%) rename {container_images => container-images}/group-sync/group-sync.py (100%) rename {container_images => container-images}/group-sync/requirements.txt (100%) diff --git a/.github/workflows/assign-class-label.yaml b/.github/workflows/assign-class-label.yaml new file mode 100644 index 0000000..9c2749a --- /dev/null +++ b/.github/workflows/assign-class-label.yaml @@ -0,0 +1,41 @@ +name: assign-class-label tests and linting/formatting + +on: + push: + paths: + - container-images/assign-class-label/** + pull_request: + paths: + - container-images/assign-class-label/** + +jobs: + lint-and-test: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.12' + + - name: Install dependencies + working-directory: ./container-images/assign-class-label/ + run: | + pip install -r requirements.txt + pip install -r tests/requirements.txt + + - name: Run ruff format (formatting) + run: | + ruff format + + - name: Run ruff check (linting) + run: | + ruff check --fix + + - name: Run tests + working-directory: ./container-images/assign-class-label/ + run: | + PYTHONPATH=. pytest tests/ \ No newline at end of file diff --git a/container_images/assign-class-label/Dockerfile b/container-images/assign-class-label/Dockerfile similarity index 100% rename from container_images/assign-class-label/Dockerfile rename to container-images/assign-class-label/Dockerfile diff --git a/container_images/assign-class-label/mutate.py b/container-images/assign-class-label/mutate.py similarity index 53% rename from container_images/assign-class-label/mutate.py rename to container-images/assign-class-label/mutate.py index 0a00380..6303738 100644 --- a/container_images/assign-class-label/mutate.py +++ b/container-images/assign-class-label/mutate.py @@ -1,42 +1,70 @@ import logging import json import base64 -from flask import Flask, request, jsonify +from flask import Flask, request, jsonify, Response from kubernetes import config, client from openshift.dynamic import DynamicClient +from pydantic import BaseModel, ValidationError, constr +from typing import Dict, Any, Optional, List + LOG = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) -def get_client(): +class PodMetadata(BaseModel): + labels: Optional[Dict[str, str]] = None + + +class PodObject(BaseModel): + metadata: PodMetadata + + +class AdmissionRequest(BaseModel): + uid: constr(min_length=1) + object: PodObject + + +class AdmissionReview(BaseModel): + request: AdmissionRequest + + +def get_client() -> DynamicClient: try: - config.load_config() + config.load_config() + k8s_client = client.ApiClient() + dyn_client = DynamicClient( + k8s_client + ) + return dyn_client except config.ConfigException as e: LOG.error('Could not configure Kubernetes client: %s', str(e)) exit(1) - k8s_client = client.ApiClient() - dyn_client = DynamicClient(k8s_client) - - return dyn_client - +def get_group_resource(dyn_client): + return dyn_client.resources.get( + api_version='user.openshift.io/v1', kind='Group' + ) # Get users of a given group -def get_group_members(group_resource, group_name): +def get_group_members(group_resource: Any, group_name: str) -> List[str]: group_obj = group_resource.get(name=group_name) return group_obj.users -def assign_class_label(pod, groups, dyn_client): +def assign_class_label( + pod: Dict[str, Any], groups: List[str], dyn_client: DynamicClient +) -> Optional[str]: # Extract pod metadata - pod_metadata = pod.get('metadata', {}) - pod_labels = pod_metadata.get('labels', {}) - pod_user = pod_labels.get('opendatahub.io/user', None) + try: + pod_metadata = pod.get('metadata', {}) + pod_labels = pod_metadata.get('labels', {}) + pod_user = pod_labels.get('opendatahub.io/user', None) + except AttributeError as e: + LOG.error(f'Error extracting pod information: {e}') + return None - group_resource = dyn_client.resources.get( # Need to move this group_resource, this is running every iteration of for loop - api_version='user.openshift.io/v1', kind='Group' - ) + group_resource = get_group_resource() # Iterate through classes for group in groups: @@ -55,7 +83,7 @@ def assign_class_label(pod, groups, dyn_client): return None -def create_app(**config): +def create_app(**config: Any) -> Flask: app = Flask(__name__) app.config.from_prefixed_env('RHOAI_CLASS') app.config.update(config) @@ -66,16 +94,33 @@ def create_app(**config): groups = app.config['GROUPS'].split(',') - dyn_client = ( - get_client() - ) # Moved here so not being called every for loop in assign_class_label + dyn_client = get_client() @app.route('/mutate', methods=['POST']) - def mutate_pod(): - # Grab pod for mutation - request_info = request.get_json() - uid = request_info['request']['uid'] - pod = request_info['request']['object'] + def mutate_pod() -> Response: + # Grab pod for mutation and validate request + try: + admission_review = AdmissionReview(**request.get_json()) + except ValidationError as e: + LOG.error('Validation error: %s', e) + return ( + jsonify( + { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'uid': request.json.get('request', {}).get('uid', ''), + 'allowed': False, + 'status': {'message': f'Invalid request: {e}'}, + }, + } + ), + 400, + {'content-type': 'application/json'}, + ) + + uid = admission_review.request.uid + pod = admission_review.request.object.model_dump() # Grab class that the pod user belongs to try: @@ -99,12 +144,19 @@ def mutate_pod(): ) # Generate JSON Patch to add class label - patch = [{'op': 'add', 'path': '/metadata/labels/class', 'value': class_label}] + patch = [ + { + 'op': 'add', + 'path': '/metadata/labels/nerc.mghpcc.org~1class', + 'value': class_label, + } + ] # Encode patch as base64 for response patch_base64 = base64.b64encode(json.dumps(patch).encode('utf-8')).decode( 'utf-8' ) + # Return webhook response that includes the patch to add class label return jsonify( { diff --git a/container_images/assign-class-label/requirements.txt b/container-images/assign-class-label/requirements.txt similarity index 80% rename from container_images/assign-class-label/requirements.txt rename to container-images/assign-class-label/requirements.txt index 35f5f44..fde8659 100644 --- a/container_images/assign-class-label/requirements.txt +++ b/container-images/assign-class-label/requirements.txt @@ -2,3 +2,4 @@ flask kubernetes openshift gunicorn +pydantic diff --git a/container-images/assign-class-label/tests/requirements.txt b/container-images/assign-class-label/tests/requirements.txt new file mode 100644 index 0000000..71cc539 --- /dev/null +++ b/container-images/assign-class-label/tests/requirements.txt @@ -0,0 +1,2 @@ +pytest +ruff diff --git a/container-images/assign-class-label/tests/test_mutate.py b/container-images/assign-class-label/tests/test_mutate.py new file mode 100644 index 0000000..9172f5e --- /dev/null +++ b/container-images/assign-class-label/tests/test_mutate.py @@ -0,0 +1,232 @@ +import pytest +import mutate + +from unittest import mock + + +@pytest.fixture() +def app(): + with mock.patch('mutate.get_client') as mock_get_client, \ + mock.patch('mutate.get_group_resource') as mock_get_group_resource, \ + mock.patch('mutate.get_group_members') as mock_group_members: + + + mock_object = mock.Mock() + mock_get_client.return_value = mock_object + mock_get_group_resource.return_value = mock_object + + def mock_get_group_members(group_resource, group_name): + if group_name == 'class1': + return ['user1'] + elif group_name == 'class2': + return ['user2'] + return [] + + mock_group_members.side_effect = mock_get_group_members + + app = mutate.create_app(GROUPS='class1,class2', LABEL='testlabel') + + app.config.update( + { + 'TESTING': True, + } + ) + + yield app + + +@pytest.fixture() +def client(app): + return app.test_client() + + +# Sending empty json request +def test_mutate_bad_data(client): + res = client.post('/mutate', json={}) + + assert res.status_code == 400 + + res_json = res.get_json() + + assert 'Invalid request' in res_json['response']['status']['message'] + + +# Calling a bad path +def test_bad_path(client): + res = client.get('/fake_path') + assert res.status_code == 404 + + assert 'The requested URL was not found' in res.data.decode('utf-8') + + +# Proper request that contains no metadata +def test_request_no_metadata(client): + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '1234', + 'object': {'metadata': {}}, + } + }, + ) + + assert res.status_code == 200 + assert res.json == { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'allowed': True, + 'status': {'message': 'No class label assigned.'}, + 'uid': '1234', + }, + } + + +# Exeption response +def test_api_exception(client): + mutate.get_group_members.side_effect = ValueError('this is a test') + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '1234', + 'object': { + 'metadata': { + 'labels': { + 'opendatahub.io/user': 'testuser1', + } + } + }, + } + }, + ) + assert res.status_code == 500 + assert res.text == 'unexpected error encountered' + + +# If user1 in group1, returns successful json patch +def test_valid_request(client): + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '1234', + 'object': { + 'metadata': { + 'labels': { + 'opendatahub.io/user': 'user1', + } + } + }, + } + }, + ) + assert res.status_code == 200 + assert res.get_json()['response']['patchType'] == 'JSONPatch' + + +# Pod has invalid (empty) UID +def test_invalid_uid(client): + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '', + 'object': { + 'metadata': { + 'labels': { + 'opendatahub.io/user': 'user1', + } + } + }, + } + }, + ) + + assert res.status_code == 400 + + assert 'Invalid request' in res.get_json()['response']['status']['message'] + + +# Request formatted correctly but contains empty or no user +def test_no_user(client): + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '1234', + 'object': { + 'metadata': { + 'labels': { + 'opendatahub.io/user': '', + } + } + }, + } + }, + ) + + assert res.status_code == 200 + assert res.json == { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'allowed': True, + 'status': {'message': 'No class label assigned.'}, + 'uid': '1234', + }, + } + + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '1234', + 'object': {'metadata': {'labels': {}}}, + } + }, + ) + + assert res.status_code == 200 + assert res.json == { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'allowed': True, + 'status': {'message': 'No class label assigned.'}, + 'uid': '1234', + }, + } + + +# Valid request but all groups empty +def test_empty_group_members(client): + with mock.patch('mutate.get_group_members') as mock_group_members: + mock_group_members.return_value = [] + res = client.post( + '/mutate', + json={ + 'request': { + 'uid': '1234', + 'object': { + 'metadata': { + 'labels': { + 'opendatahub.io/user': 'user1', + } + } + }, + } + }, + ) + + assert res.status_code == 200 + assert res.json == { + 'apiVersion': 'admission.k8s.io/v1', + 'kind': 'AdmissionReview', + 'response': { + 'allowed': True, + 'status': {'message': 'No class label assigned.'}, + 'uid': '1234', + }, + } diff --git a/container_images/assign-class-label/wsgi.py b/container-images/assign-class-label/wsgi.py similarity index 100% rename from container_images/assign-class-label/wsgi.py rename to container-images/assign-class-label/wsgi.py diff --git a/container_images/group-sync/Dockerfile b/container-images/group-sync/Dockerfile similarity index 100% rename from container_images/group-sync/Dockerfile rename to container-images/group-sync/Dockerfile diff --git a/container_images/group-sync/group-sync.py b/container-images/group-sync/group-sync.py similarity index 100% rename from container_images/group-sync/group-sync.py rename to container-images/group-sync/group-sync.py diff --git a/container_images/group-sync/requirements.txt b/container-images/group-sync/requirements.txt similarity index 100% rename from container_images/group-sync/requirements.txt rename to container-images/group-sync/requirements.txt diff --git a/webhooks/assign-class-label/kustomization.yaml b/webhooks/assign-class-label/kustomization.yaml index 5bcb16d..c7186f4 100644 --- a/webhooks/assign-class-label/kustomization.yaml +++ b/webhooks/assign-class-label/kustomization.yaml @@ -1,6 +1,6 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization -namespace: rhods-notebooks +namespace: ope-rhods-testing-1fef2f commonLabels: app: assign-class-label-webhook diff --git a/webhooks/assign-class-label/rolebinding.yaml b/webhooks/assign-class-label/rolebinding.yaml index 48cebf2..72f70ff 100644 --- a/webhooks/assign-class-label/rolebinding.yaml +++ b/webhooks/assign-class-label/rolebinding.yaml @@ -9,4 +9,4 @@ roleRef: subjects: - kind: ServiceAccount name: webhook-sa - namespace: rhods-notebooks + namespace: ope-rhods-testing-1fef2f diff --git a/webhooks/assign-class-label/serviceaccount.yaml b/webhooks/assign-class-label/serviceaccount.yaml index 86983aa..f425f56 100644 --- a/webhooks/assign-class-label/serviceaccount.yaml +++ b/webhooks/assign-class-label/serviceaccount.yaml @@ -2,4 +2,4 @@ apiVersion: v1 kind: ServiceAccount metadata: name: webhook-sa - namespace: rhods-notebooks + namespace: ope-rhods-testing-1fef2f diff --git a/webhooks/assign-class-label/webhook-config.yaml b/webhooks/assign-class-label/webhook-config.yaml index 5b17a87..db3d86e 100644 --- a/webhooks/assign-class-label/webhook-config.yaml +++ b/webhooks/assign-class-label/webhook-config.yaml @@ -6,10 +6,10 @@ webhooks: - name: assign-class-label-webhook.nerc.com clientConfig: service: - namespace: rhods-notebooks + namespace: ope-rhods-testing-1fef2f name: assign-class-label-webhook path: /mutate - caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVRRENDQXlpZ0F3SUJBZ0lVU3JnWEl4OUpON21wY3FnLzRsMWg5bFZNMFBNd0RRWUpLb1pJaHZjTkFRRUwKQlFBd2daQXhDekFKQmdOVkJBWVRBbFZUTVJZd0ZBWURWUVFJREExTllYTnpZV05vZFhOelpYUnpNUTh3RFFZRApWUVFIREFaQ2IzTjBiMjR4RURBT0JnTlZCQW9NQjFKbFpDQklZWFF4RVRBUEJnTlZCQXNNQ0ZKbGMyVmhjbU5vCk1ROHdEUVlEVlFRRERBWkpjMkZwWVdneElqQWdCZ2txaGtpRzl3MEJDUUVXRTJsemRHRndiR1YwUUhKbFpHaGgKZEM1amIyMHdIaGNOTWpRd09ERTVNVGN4TnpReFdoY05NamN3TmpBNU1UY3hOelF4V2pDQmtERUxNQWtHQTFVRQpCaE1DVlZNeEZqQVVCZ05WQkFnTURVMWhjM05oWTJoMWMzTmxkSE14RHpBTkJnTlZCQWNNQmtKdmMzUnZiakVRCk1BNEdBMVVFQ2d3SFVtVmtJRWhoZERFUk1BOEdBMVVFQ3d3SVVtVnpaV0Z5WTJneER6QU5CZ05WQkFNTUJrbHoKWVdsaGFERWlNQ0FHQ1NxR1NJYjNEUUVKQVJZVGFYTjBZWEJzWlhSQWNtVmthR0YwTG1OdmJUQ0NBU0l3RFFZSgpLb1pJaHZjTkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFJbEJnaXZMVVZWWENmOTIyV2JHN3Fwcm9MZUgvODlQCkNVbHJLSTFISUNYRW1tZTRKRHJvQzRIV1c0QW4vS1VoVW5pNkpWbHhjaE02dWJMeU5sNjBmRVRERWJBeHgzWVUKa1RHa01JNGxWS0VlVGlpcFJVQUdwQjg0VXBEbi82cnpwQUJqT2xoakFmMUNZcnhzSVJvc1o2L1ZVcG9veTdOZApiYllIdlFTMmd1N2IwODAyS2YrWko2bUpPbGlIcUdtVjRqVlRJK0RPem15RXlhMmhwSXpHMnZOQ0JIcDJVZXN2CkdaMXJ0dE8xOWs3K1AyQXdoZFVUMk14Z2NEeUdaSjF5N2ZuR25JbXczRGtEL2FKWUp3bklHaWhaMnNSZTlsbjgKbGVsRVN3YWx3S2NPTlZRcnB3QlJzdC8ySXdhdlpyd1dNQVg0cUpEeks3dWJyc3FJdkZZcUh6MENBd0VBQWFPQgpqekNCakRBZEJnTlZIUTRFRmdRVXpDWUswU2tkbm02S0lNbnFiSGF1QnlSOWxrb3dId1lEVlIwakJCZ3dGb0FVCnpDWUswU2tkbm02S0lNbnFiSGF1QnlSOWxrb3dEd1lEVlIwVEFRSC9CQVV3QXdFQi96QTVCZ05WSFJFRU1qQXcKZ2k1aGMzTnBaMjR0WTJ4aGMzTXRiR0ZpWld3dGQyVmlhRzl2YXk1eWFHOWtjeTF1YjNSbFltOXZhM011YzNaagpNQTBHQ1NxR1NJYjNEUUVCQ3dVQUE0SUJBUUI3eVpGM1k5ZWFkTDR5Y1R3cHJsem5JT2IreUJncGxkOFdlNndqCisvWE9XVnlXb09rM1JYT0VadWoxUE8xcWpHY0lJOFpQc05xamJmNVhOd1hkaUlsSm1KdDJaQVFWWkpqY0dkMmQKS2RRNXF1Wk44Z2ZFS0pwWHJUVWlIZ3VQQjdCNmd5dGVBYWlyaXl4RDllZmgzcG1HTGJaU0xscXlRcVBrUFZ6UwpUeXBIbDNPVHllaE8wKy82MWh0MnE1TVpYamd4Rk1CQTlqNXZKd2JUMlVxWFllSGFOOTlqcU0rTW10Zks2NUUxCjkxQUY2bndmQys0Z1FNS0tUQ0hpVkFWb09KakJBVjZyOXkydSs5a0xyNE9LUkFaWEF4alI3dFl1YitsQmJYZUsKSHpOdnBYajBLZmpBaG9BNkdPUW1MVG01UGRNZjN2dWhlS1FFdmp1L3lldzZTMlk5Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K + caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURxekNDQXBPZ0F3SUJBZ0lVT1ZseDVzTnpXSzA0TVVoVVJVWVdlUXBiTWlvd0RRWUpLb1pJaHZjTkFRRUwKQlFBd1FqRUxNQWtHQTFVRUJoTUNXRmd4RlRBVEJnTlZCQWNNREVSbFptRjFiSFFnUTJsMGVURWNNQm9HQTFVRQpDZ3dUUkdWbVlYVnNkQ0JEYjIxd1lXNTVJRXgwWkRBZUZ3MHlOREE0TWpreE9EQTJNRFZhRncweU56QTJNVGt4Ck9EQTJNRFZhTUVJeEN6QUpCZ05WQkFZVEFsaFlNUlV3RXdZRFZRUUhEQXhFWldaaGRXeDBJRU5wZEhreEhEQWEKQmdOVkJBb01FMFJsWm1GMWJIUWdRMjl0Y0dGdWVTQk1kR1F3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQgpEd0F3Z2dFS0FvSUJBUUN5WmxLeWdzVW8zS2g2VThGVEFQNGViQ25wRHNuU1hmUjZHUCtlSGJBU2xGZVBTU2hRCkJJQzJKcytMb3Vpd0F0VkRJblRnRWVzZXZSS3FWdHhveDQwSEdhakVoUkRDNTZKWjNtKzl2L1ZJcDhMeWtic00KSmFQakRwWUJKczBJWm1zaFlMaFBtT3JOSmFZbWJlL25IeFNHcGhXWjE5TlVKcjg0Qm4zSmdTZTkwekZwbGszTQpGbEF5UmY1eDVIbTk4UzhhS3RVeHh5RjAvaXNBK3RFbmgvL0c3aWs4QndCT2lFS1A4cTluaG1EVnZDOURXTkJMCkRZNjFEOWtKQmFhQ3RYaGNQNWNGLzBlTUFrc2NKSS9iZkxjc3pIYUhnWW1kUlB6RSsyN3VlckxFZGsrUWdUSGwKcmtuWE85NEdwNG1UZ1lKV2ZwdjFYVmxncVhoRGVyTnhsUjROQWdNQkFBR2pnWmd3Z1pVd0hRWURWUjBPQkJZRQpGTklsbVAwTlhkdW5Fak1IZ3pGcUYxZ1RIYU5ZTUI4R0ExVWRJd1FZTUJhQUZOSWxtUDBOWGR1bkVqTUhnekZxCkYxZ1RIYU5ZTUE4R0ExVWRFd0VCL3dRRk1BTUJBZjh3UWdZRFZSMFJCRHN3T1lJM1lYTnphV2R1TFdOc1lYTnoKTFd4aFltVnNMWGRsWW1odmIyc3ViM0JsTFhKb2IyUnpMWFJsYzNScGJtY3RNV1psWmpKbUxuTjJZekFOQmdrcQpoa2lHOXcwQkFRc0ZBQU9DQVFFQVF5Y2xJZ3VCRGhOMWZBQWRKV0R0aDNJb3JydytvQVlOYkxsR0pxY0xnRVVhCjRISnZwb0grTWpqYkRGOTF1QUFyb3g1bXU5c3grVGlySjE1VnRSTnlCeHozbkFQd2F2NmplZHhPbmVTSjVjZlYKNnBZMkNTOEJIa2h1ZWJKWTVRVUM5NzMvNEpsZnY1WlN4OUpZS2d5Yk9McTNSRnlBZ21pZjA1L2w1NzN0MFJraQpCRXUwbHc3OVlXaktBRDluYkRFYUhrajVkTStLcTZrdVZ0T3dlaTBBMlNoc0p1MVVZVEhDdGFxQjNQTDU2dWVBCnhIejJBTlQwdlFNK2lLMFJCUlBLODE2K2ZOQm10TGlpZG1mSi9QWVFxeVdDQnhudWlxQ1B2bzUvcXRmdS9DbzcKNENjUTJaSTV1QWhleGI5b0xWZW5pM2lmTGg1MHV3M1dmZWlyMk8wckJRPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= rules: - operations: ["CREATE"] apiGroups: [""] From f9803844264dc499e470ad9453285e6afc4672b7 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Thu, 29 Aug 2024 17:22:38 -0400 Subject: [PATCH 07/11] Add decode pod user to properly represent pod usernames Signed-off-by: Isaiah Stapleton --- container-images/assign-class-label/mutate.py | 22 ++++++++++++------- .../assign-class-label/tests/test_mutate.py | 10 ++++----- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/container-images/assign-class-label/mutate.py b/container-images/assign-class-label/mutate.py index 6303738..248a25d 100644 --- a/container-images/assign-class-label/mutate.py +++ b/container-images/assign-class-label/mutate.py @@ -29,22 +29,25 @@ class AdmissionReview(BaseModel): request: AdmissionRequest +def decode_pod_user(pod_user: str) -> str: + user = pod_user.replace('-40', '@').replace('-2e', '.') + return user + + def get_client() -> DynamicClient: try: - config.load_config() + config.load_config() k8s_client = client.ApiClient() - dyn_client = DynamicClient( - k8s_client - ) + dyn_client = DynamicClient(k8s_client) return dyn_client except config.ConfigException as e: LOG.error('Could not configure Kubernetes client: %s', str(e)) exit(1) + def get_group_resource(dyn_client): - return dyn_client.resources.get( - api_version='user.openshift.io/v1', kind='Group' - ) + return dyn_client.resources.get(api_version='user.openshift.io/v1', kind='Group') + # Get users of a given group def get_group_members(group_resource: Any, group_name: str) -> List[str]: @@ -63,8 +66,11 @@ def assign_class_label( except AttributeError as e: LOG.error(f'Error extracting pod information: {e}') return None + + if pod_user is not None: + pod_user = decode_pod_user(pod_user) - group_resource = get_group_resource() + group_resource = get_group_resource(dyn_client) # Iterate through classes for group in groups: diff --git a/container-images/assign-class-label/tests/test_mutate.py b/container-images/assign-class-label/tests/test_mutate.py index 9172f5e..5156938 100644 --- a/container-images/assign-class-label/tests/test_mutate.py +++ b/container-images/assign-class-label/tests/test_mutate.py @@ -6,11 +6,11 @@ @pytest.fixture() def app(): - with mock.patch('mutate.get_client') as mock_get_client, \ - mock.patch('mutate.get_group_resource') as mock_get_group_resource, \ - mock.patch('mutate.get_group_members') as mock_group_members: - - + with mock.patch('mutate.get_client') as mock_get_client, mock.patch( + 'mutate.get_group_resource' + ) as mock_get_group_resource, mock.patch( + 'mutate.get_group_members' + ) as mock_group_members: mock_object = mock.Mock() mock_get_client.return_value = mock_object mock_get_group_resource.return_value = mock_object From e46e9149467fe0841b1e1a74f87f47bff949c001 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Fri, 30 Aug 2024 10:48:33 -0400 Subject: [PATCH 08/11] Changes based off of second PR revision comments Signed-off-by: Isaiah Stapleton --- .github/workflows/assign-class-label.yaml | 6 +- .../assign-class-label/Dockerfile | 6 +- .../assign-class-label/conftest.py | 0 container-images/assign-class-label/mutate.py | 92 +++++++++++-------- ...requirements.txt => test-requirements.txt} | 0 .../assign-class-label/tests/test_mutate.py | 19 +++- 6 files changed, 77 insertions(+), 46 deletions(-) create mode 100644 container-images/assign-class-label/conftest.py rename container-images/assign-class-label/{tests/requirements.txt => test-requirements.txt} (100%) diff --git a/.github/workflows/assign-class-label.yaml b/.github/workflows/assign-class-label.yaml index 9c2749a..f6553aa 100644 --- a/.github/workflows/assign-class-label.yaml +++ b/.github/workflows/assign-class-label.yaml @@ -25,17 +25,19 @@ jobs: working-directory: ./container-images/assign-class-label/ run: | pip install -r requirements.txt - pip install -r tests/requirements.txt + pip install -r test-requirements.txt - name: Run ruff format (formatting) + working-directory: ./container-images/assign-class-label/ run: | ruff format - name: Run ruff check (linting) + working-directory: ./container-images/assign-class-label/ run: | ruff check --fix - name: Run tests working-directory: ./container-images/assign-class-label/ run: | - PYTHONPATH=. pytest tests/ \ No newline at end of file + pytest tests/ \ No newline at end of file diff --git a/container-images/assign-class-label/Dockerfile b/container-images/assign-class-label/Dockerfile index 5a7a580..b362378 100644 --- a/container-images/assign-class-label/Dockerfile +++ b/container-images/assign-class-label/Dockerfile @@ -1,12 +1,12 @@ -FROM python:3.9-slim +FROM python:3.12-slim WORKDIR /app/ -COPY requirements.txt /app/ +COPY requirements.txt ./ RUN pip install -r requirements.txt -COPY . /app/ +COPY . ./ EXPOSE 5000 diff --git a/container-images/assign-class-label/conftest.py b/container-images/assign-class-label/conftest.py new file mode 100644 index 0000000..e69de29 diff --git a/container-images/assign-class-label/mutate.py b/container-images/assign-class-label/mutate.py index 248a25d..7926127 100644 --- a/container-images/assign-class-label/mutate.py +++ b/container-images/assign-class-label/mutate.py @@ -29,9 +29,26 @@ class AdmissionReview(BaseModel): request: AdmissionRequest +class Status(BaseModel): + message: Optional[str] = None + + +class AdmissionResponse(BaseModel): + uid: str + allowed: bool + status: Optional[Status] = None + patchType: Optional[str] = None + patch: Optional[str] = None + + +class AdmissionReviewResponse(BaseModel): + apiVersion: str = 'admission.k8s.io/v1' + kind: str = 'AdmissionReview' + response: AdmissionResponse + + def decode_pod_user(pod_user: str) -> str: - user = pod_user.replace('-40', '@').replace('-2e', '.') - return user + return pod_user.replace('-40', '@').replace('-2e', '.') def get_client() -> DynamicClient: @@ -56,8 +73,8 @@ def get_group_members(group_resource: Any, group_name: str) -> List[str]: def assign_class_label( - pod: Dict[str, Any], groups: List[str], dyn_client: DynamicClient -) -> Optional[str]: + pod: dict[str, Any], groups: list[str], dyn_client: DynamicClient +) -> str | None: # Extract pod metadata try: pod_metadata = pod.get('metadata', {}) @@ -66,9 +83,11 @@ def assign_class_label( except AttributeError as e: LOG.error(f'Error extracting pod information: {e}') return None - - if pod_user is not None: - pod_user = decode_pod_user(pod_user) + + if pod_user is None: + return None + + pod_user = decode_pod_user(pod_user) group_resource = get_group_resource(dyn_client) @@ -111,15 +130,13 @@ def mutate_pod() -> Response: LOG.error('Validation error: %s', e) return ( jsonify( - { - 'apiVersion': 'admission.k8s.io/v1', - 'kind': 'AdmissionReview', - 'response': { - 'uid': request.json.get('request', {}).get('uid', ''), - 'allowed': False, - 'status': {'message': f'Invalid request: {e}'}, - }, - } + AdmissionReviewResponse( + response=AdmissionResponse( + uid=request.json.get('request', {}).get('uid', ''), + allowed=False, + status=Status(message=f'Invalid request: {e}'), + ) + ).model_dump() ), 400, {'content-type': 'application/json'}, @@ -137,16 +154,18 @@ def mutate_pod() -> Response: # If user not in any class, return without modifications if not class_label: - return jsonify( - { - 'apiVersion': 'admission.k8s.io/v1', - 'kind': 'AdmissionReview', - 'response': { - 'uid': uid, - 'allowed': True, - 'status': {'message': 'No class label assigned.'}, - }, - } + return ( + jsonify( + AdmissionReviewResponse( + response=AdmissionResponse( + uid=uid, + allowed=True, + status=Status(message='No class label assigned.'), + ) + ).model_dump() + ), + 400, + {'content-type': 'application/json'}, ) # Generate JSON Patch to add class label @@ -164,17 +183,16 @@ def mutate_pod() -> Response: ) # Return webhook response that includes the patch to add class label - return jsonify( - { - 'apiVersion': 'admission.k8s.io/v1', - 'kind': 'AdmissionReview', - 'response': { - 'uid': uid, - 'allowed': True, - 'patchType': 'JSONPatch', - 'patch': patch_base64, - }, - } + return ( + jsonify( + AdmissionReviewResponse( + response=AdmissionResponse( + uid=uid, allowed=True, patchType='JSONPatch', patch=patch_base64 + ) + ).model_dump() + ), + 200, + {'content-type': 'application/json'}, ) return app diff --git a/container-images/assign-class-label/tests/requirements.txt b/container-images/assign-class-label/test-requirements.txt similarity index 100% rename from container-images/assign-class-label/tests/requirements.txt rename to container-images/assign-class-label/test-requirements.txt diff --git a/container-images/assign-class-label/tests/test_mutate.py b/container-images/assign-class-label/tests/test_mutate.py index 5156938..82c5254 100644 --- a/container-images/assign-class-label/tests/test_mutate.py +++ b/container-images/assign-class-label/tests/test_mutate.py @@ -71,7 +71,7 @@ def test_request_no_metadata(client): }, ) - assert res.status_code == 200 + assert res.status_code == 400 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -79,6 +79,8 @@ def test_request_no_metadata(client): 'allowed': True, 'status': {'message': 'No class label assigned.'}, 'uid': '1234', + 'patchType': None, + 'patch': None, }, } @@ -122,6 +124,9 @@ def test_valid_request(client): } }, ) + + print(f'Response code: {res.status_code}') + print(f'Response JSON: {res.get_json()}') assert res.status_code == 200 assert res.get_json()['response']['patchType'] == 'JSONPatch' @@ -167,7 +172,7 @@ def test_no_user(client): }, ) - assert res.status_code == 200 + assert res.status_code == 400 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -175,6 +180,8 @@ def test_no_user(client): 'allowed': True, 'status': {'message': 'No class label assigned.'}, 'uid': '1234', + 'patchType': None, + 'patch': None, }, } @@ -188,7 +195,7 @@ def test_no_user(client): }, ) - assert res.status_code == 200 + assert res.status_code == 400 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -196,6 +203,8 @@ def test_no_user(client): 'allowed': True, 'status': {'message': 'No class label assigned.'}, 'uid': '1234', + 'patchType': None, + 'patch': None, }, } @@ -220,7 +229,7 @@ def test_empty_group_members(client): }, ) - assert res.status_code == 200 + assert res.status_code == 400 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -228,5 +237,7 @@ def test_empty_group_members(client): 'allowed': True, 'status': {'message': 'No class label assigned.'}, 'uid': '1234', + 'patchType': None, + 'patch': None, }, } From 02b05c5b20a1cdd6f190975661a9f98aa9b625a8 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Wed, 18 Sep 2024 13:51:37 -0400 Subject: [PATCH 09/11] Move pydantic validation models into separate file Signed-off-by: Isaiah Stapleton --- container-images/assign-class-label/models.py | 37 +++++++++++++++++ container-images/assign-class-label/mutate.py | 41 ++----------------- 2 files changed, 41 insertions(+), 37 deletions(-) create mode 100644 container-images/assign-class-label/models.py diff --git a/container-images/assign-class-label/models.py b/container-images/assign-class-label/models.py new file mode 100644 index 0000000..d318b9f --- /dev/null +++ b/container-images/assign-class-label/models.py @@ -0,0 +1,37 @@ +from pydantic import BaseModel, constr +from typing import Dict, Optional + + +class PodMetadata(BaseModel): + labels: Optional[Dict[str, str]] = None + + +class PodObject(BaseModel): + metadata: PodMetadata + + +class AdmissionRequest(BaseModel): + uid: constr(min_length=1) + object: PodObject + + +class AdmissionReview(BaseModel): + request: AdmissionRequest + + +class Status(BaseModel): + message: Optional[str] = None + + +class AdmissionResponse(BaseModel): + uid: str + allowed: bool + status: Optional[Status] = None + patchType: Optional[str] = None + patch: Optional[str] = None + + +class AdmissionReviewResponse(BaseModel): + apiVersion: str = 'admission.k8s.io/v1' + kind: str = 'AdmissionReview' + response: AdmissionResponse diff --git a/container-images/assign-class-label/mutate.py b/container-images/assign-class-label/mutate.py index 7926127..98054ab 100644 --- a/container-images/assign-class-label/mutate.py +++ b/container-images/assign-class-label/mutate.py @@ -5,48 +5,15 @@ from kubernetes import config, client from openshift.dynamic import DynamicClient -from pydantic import BaseModel, ValidationError, constr -from typing import Dict, Any, Optional, List +from pydantic import ValidationError +from typing import Any, List + +from models import AdmissionReviewResponse, AdmissionReview, AdmissionResponse, Status LOG = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) -class PodMetadata(BaseModel): - labels: Optional[Dict[str, str]] = None - - -class PodObject(BaseModel): - metadata: PodMetadata - - -class AdmissionRequest(BaseModel): - uid: constr(min_length=1) - object: PodObject - - -class AdmissionReview(BaseModel): - request: AdmissionRequest - - -class Status(BaseModel): - message: Optional[str] = None - - -class AdmissionResponse(BaseModel): - uid: str - allowed: bool - status: Optional[Status] = None - patchType: Optional[str] = None - patch: Optional[str] = None - - -class AdmissionReviewResponse(BaseModel): - apiVersion: str = 'admission.k8s.io/v1' - kind: str = 'AdmissionReview' - response: AdmissionResponse - - def decode_pod_user(pod_user: str) -> str: return pod_user.replace('-40', '@').replace('-2e', '.') From 3630af2b47627bafa59938939a0c621c1afe940b Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Wed, 18 Sep 2024 14:03:48 -0400 Subject: [PATCH 10/11] Change response for pod with user in no class to 200 This fixes the issue where the webhook returns a 400 response in the case where the pods user doesn't belong to any class. This caused issues for deployments trying to start up pods but being unable to because of the error response from the webhook. Signed-off-by: Isaiah Stapleton --- container-images/assign-class-label/mutate.py | 2 +- container-images/assign-class-label/tests/test_mutate.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/container-images/assign-class-label/mutate.py b/container-images/assign-class-label/mutate.py index 98054ab..9b67759 100644 --- a/container-images/assign-class-label/mutate.py +++ b/container-images/assign-class-label/mutate.py @@ -131,7 +131,7 @@ def mutate_pod() -> Response: ) ).model_dump() ), - 400, + 200, {'content-type': 'application/json'}, ) diff --git a/container-images/assign-class-label/tests/test_mutate.py b/container-images/assign-class-label/tests/test_mutate.py index 82c5254..6386333 100644 --- a/container-images/assign-class-label/tests/test_mutate.py +++ b/container-images/assign-class-label/tests/test_mutate.py @@ -71,7 +71,7 @@ def test_request_no_metadata(client): }, ) - assert res.status_code == 400 + assert res.status_code == 200 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -172,7 +172,7 @@ def test_no_user(client): }, ) - assert res.status_code == 400 + assert res.status_code == 200 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -195,7 +195,7 @@ def test_no_user(client): }, ) - assert res.status_code == 400 + assert res.status_code == 200 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', @@ -229,7 +229,7 @@ def test_empty_group_members(client): }, ) - assert res.status_code == 400 + assert res.status_code == 200 assert res.json == { 'apiVersion': 'admission.k8s.io/v1', 'kind': 'AdmissionReview', From 90d59fbaae28f0a84b7728612ed6ac0728efba31 Mon Sep 17 00:00:00 2001 From: Isaiah Stapleton Date: Wed, 18 Sep 2024 14:38:35 -0400 Subject: [PATCH 11/11] Change namespace to rhods-notebooks Signed-off-by: Isaiah Stapleton --- webhooks/assign-class-label/kustomization.yaml | 2 +- webhooks/assign-class-label/rolebinding.yaml | 2 +- webhooks/assign-class-label/serviceaccount.yaml | 2 +- webhooks/assign-class-label/webhook-config.yaml | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/webhooks/assign-class-label/kustomization.yaml b/webhooks/assign-class-label/kustomization.yaml index c7186f4..5bcb16d 100644 --- a/webhooks/assign-class-label/kustomization.yaml +++ b/webhooks/assign-class-label/kustomization.yaml @@ -1,6 +1,6 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization -namespace: ope-rhods-testing-1fef2f +namespace: rhods-notebooks commonLabels: app: assign-class-label-webhook diff --git a/webhooks/assign-class-label/rolebinding.yaml b/webhooks/assign-class-label/rolebinding.yaml index 72f70ff..48cebf2 100644 --- a/webhooks/assign-class-label/rolebinding.yaml +++ b/webhooks/assign-class-label/rolebinding.yaml @@ -9,4 +9,4 @@ roleRef: subjects: - kind: ServiceAccount name: webhook-sa - namespace: ope-rhods-testing-1fef2f + namespace: rhods-notebooks diff --git a/webhooks/assign-class-label/serviceaccount.yaml b/webhooks/assign-class-label/serviceaccount.yaml index f425f56..86983aa 100644 --- a/webhooks/assign-class-label/serviceaccount.yaml +++ b/webhooks/assign-class-label/serviceaccount.yaml @@ -2,4 +2,4 @@ apiVersion: v1 kind: ServiceAccount metadata: name: webhook-sa - namespace: ope-rhods-testing-1fef2f + namespace: rhods-notebooks diff --git a/webhooks/assign-class-label/webhook-config.yaml b/webhooks/assign-class-label/webhook-config.yaml index db3d86e..e7fef76 100644 --- a/webhooks/assign-class-label/webhook-config.yaml +++ b/webhooks/assign-class-label/webhook-config.yaml @@ -6,10 +6,10 @@ webhooks: - name: assign-class-label-webhook.nerc.com clientConfig: service: - namespace: ope-rhods-testing-1fef2f + namespace: rhods-notebooks name: assign-class-label-webhook path: /mutate - caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURxekNDQXBPZ0F3SUJBZ0lVT1ZseDVzTnpXSzA0TVVoVVJVWVdlUXBiTWlvd0RRWUpLb1pJaHZjTkFRRUwKQlFBd1FqRUxNQWtHQTFVRUJoTUNXRmd4RlRBVEJnTlZCQWNNREVSbFptRjFiSFFnUTJsMGVURWNNQm9HQTFVRQpDZ3dUUkdWbVlYVnNkQ0JEYjIxd1lXNTVJRXgwWkRBZUZ3MHlOREE0TWpreE9EQTJNRFZhRncweU56QTJNVGt4Ck9EQTJNRFZhTUVJeEN6QUpCZ05WQkFZVEFsaFlNUlV3RXdZRFZRUUhEQXhFWldaaGRXeDBJRU5wZEhreEhEQWEKQmdOVkJBb01FMFJsWm1GMWJIUWdRMjl0Y0dGdWVTQk1kR1F3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQgpEd0F3Z2dFS0FvSUJBUUN5WmxLeWdzVW8zS2g2VThGVEFQNGViQ25wRHNuU1hmUjZHUCtlSGJBU2xGZVBTU2hRCkJJQzJKcytMb3Vpd0F0VkRJblRnRWVzZXZSS3FWdHhveDQwSEdhakVoUkRDNTZKWjNtKzl2L1ZJcDhMeWtic00KSmFQakRwWUJKczBJWm1zaFlMaFBtT3JOSmFZbWJlL25IeFNHcGhXWjE5TlVKcjg0Qm4zSmdTZTkwekZwbGszTQpGbEF5UmY1eDVIbTk4UzhhS3RVeHh5RjAvaXNBK3RFbmgvL0c3aWs4QndCT2lFS1A4cTluaG1EVnZDOURXTkJMCkRZNjFEOWtKQmFhQ3RYaGNQNWNGLzBlTUFrc2NKSS9iZkxjc3pIYUhnWW1kUlB6RSsyN3VlckxFZGsrUWdUSGwKcmtuWE85NEdwNG1UZ1lKV2ZwdjFYVmxncVhoRGVyTnhsUjROQWdNQkFBR2pnWmd3Z1pVd0hRWURWUjBPQkJZRQpGTklsbVAwTlhkdW5Fak1IZ3pGcUYxZ1RIYU5ZTUI4R0ExVWRJd1FZTUJhQUZOSWxtUDBOWGR1bkVqTUhnekZxCkYxZ1RIYU5ZTUE4R0ExVWRFd0VCL3dRRk1BTUJBZjh3UWdZRFZSMFJCRHN3T1lJM1lYTnphV2R1TFdOc1lYTnoKTFd4aFltVnNMWGRsWW1odmIyc3ViM0JsTFhKb2IyUnpMWFJsYzNScGJtY3RNV1psWmpKbUxuTjJZekFOQmdrcQpoa2lHOXcwQkFRc0ZBQU9DQVFFQVF5Y2xJZ3VCRGhOMWZBQWRKV0R0aDNJb3JydytvQVlOYkxsR0pxY0xnRVVhCjRISnZwb0grTWpqYkRGOTF1QUFyb3g1bXU5c3grVGlySjE1VnRSTnlCeHozbkFQd2F2NmplZHhPbmVTSjVjZlYKNnBZMkNTOEJIa2h1ZWJKWTVRVUM5NzMvNEpsZnY1WlN4OUpZS2d5Yk9McTNSRnlBZ21pZjA1L2w1NzN0MFJraQpCRXUwbHc3OVlXaktBRDluYkRFYUhrajVkTStLcTZrdVZ0T3dlaTBBMlNoc0p1MVVZVEhDdGFxQjNQTDU2dWVBCnhIejJBTlQwdlFNK2lLMFJCUlBLODE2K2ZOQm10TGlpZG1mSi9QWVFxeVdDQnhudWlxQ1B2bzUvcXRmdS9DbzcKNENjUTJaSTV1QWhleGI5b0xWZW5pM2lmTGg1MHV3M1dmZWlyMk8wckJRPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= + caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURvakNDQW9xZ0F3SUJBZ0lVTlpUUGJOMVNBdnNzVUdUTHRUQW1FamE5TmJzd0RRWUpLb1pJaHZjTkFRRUwKQlFBd1FqRUxNQWtHQTFVRUJoTUNXRmd4RlRBVEJnTlZCQWNNREVSbFptRjFiSFFnUTJsMGVURWNNQm9HQTFVRQpDZ3dUUkdWbVlYVnNkQ0JEYjIxd1lXNTVJRXgwWkRBZUZ3MHlOREE1TVRjeE5ESXlNekZhRncweU56QTNNRGd4Ck5ESXlNekZhTUVJeEN6QUpCZ05WQkFZVEFsaFlNUlV3RXdZRFZRUUhEQXhFWldaaGRXeDBJRU5wZEhreEhEQWEKQmdOVkJBb01FMFJsWm1GMWJIUWdRMjl0Y0dGdWVTQk1kR1F3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQgpEd0F3Z2dFS0FvSUJBUUN3Z2RyWGVSek1BYW1TT1hwYnB1a0dsdTdBL1lpVTl1bytZbTI1Zm9aRmhrQXBJeWY1CkNCNzIzRW0wbUtrdmdtR0VjYy9oZ1kwYUxnV3dHZTBGRW40ZjZGQ21RRjJzMlRHMEJPUDZ4Z3NjUHlGMU41NmgKKzRwVnE4UXFrbTA3YktxcEhZSzF2czVTd2tNTXExYjBZWjdBd0Rjd3p6VWtWM20zb0VtSnptR2U1L1JUd1puTApGdlg0LzRsUEdkMGF6MnBlUnBudllOOWZLQWd3bERxMklFdjJJZUljTWpOMmxVQTAxYkhGc09qZXAyakU1dWQrCi9DWmwxZHRuWkJYWWY1N01DNFc2MitmZnlRWExyOGt1clNUN0Z5Zi9OTjVRblhxNDBQSHd4N3dWOHlzY3l6M2sKWmRsalBiTHBMQTJYeE5ZcXJRMDBid1FDVS9kVTVPZ0VaZVU3QWdNQkFBR2pnWTh3Z1l3d0hRWURWUjBPQkJZRQpGUDBobG4xTGs0aXcwUTNwTnVUNVNsWHNKdDYvTUI4R0ExVWRJd1FZTUJhQUZQMGhsbjFMazRpdzBRM3BOdVQ1ClNsWHNKdDYvTUE4R0ExVWRFd0VCL3dRRk1BTUJBZjh3T1FZRFZSMFJCREl3TUlJdVlYTnphV2R1TFdOc1lYTnoKTFd4aFltVnNMWGRsWW1odmIyc3VjbWh2WkhNdGJtOTBaV0p2YjJ0ekxuTjJZekFOQmdrcWhraUc5dzBCQVFzRgpBQU9DQVFFQVNSdWZpcDI3TmFQVC8wZGZ5VU85MDMxV2F6Z3U0YzY4dG1FVXlBYXpvNlY2Z1lzK005YnVERDNJCkl3ZkkvUXB5bGhIb01EZ3FjcjlyZ1NHY2lvbTFocXM4RjAyNGFaa3BobzV3LzRiSUJ2VWVUVHVKNlhyYzhJOCsKSW1ESHh1SFREVXZ4bG15U0NWc0lOSlBUZWprQ0RQRVR0bVlzaHlNZkxyM1NOYUZSYXU4VlNqUFNsd2VDNVkzUQo4Q3I0Q2V0eHlCdlp0enhpTXZvemR0dGx3d3RGejRaMmQ5ZmRIWmZWbms5UFlqZFpRb3o1cVlBbklkcGlJemo3Ci92UUxQZ1RpbmRDWkIvTW1PbHRBZ0UrNmQrQ2lsOW1yOHlRVVlTdlYzL015SUU1ZDd5M0h0VnJrRnhKYkZqNjEKVjVpOUR1YWNpTXpMUVM2UzJOUUJkcFpINzc0NHZ3PT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= rules: - operations: ["CREATE"] apiGroups: [""]