From 4674563569e2c68c990cb359f7fe2d4f30d16c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 12 Sep 2024 10:48:38 +0200 Subject: [PATCH 01/24] Add signify-certificate-rotator image --- .../rotate-signify-certificate/Dockerfile | 14 + .../requirements.txt | 5 + .../rotate_signify_certificate.py | 279 ++++++++++++++++++ .../test_fixtures.py | 21 ++ .../test_rotate_signify_certificate.py | 100 +++++++ 5 files changed, 419 insertions(+) create mode 100644 cmd/cloud-run/rotate-signify-certificate/Dockerfile create mode 100644 cmd/cloud-run/rotate-signify-certificate/requirements.txt create mode 100644 cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py create mode 100644 cmd/cloud-run/rotate-signify-certificate/test_fixtures.py create mode 100644 cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py diff --git a/cmd/cloud-run/rotate-signify-certificate/Dockerfile b/cmd/cloud-run/rotate-signify-certificate/Dockerfile new file mode 100644 index 000000000000..0a15710d2f51 --- /dev/null +++ b/cmd/cloud-run/rotate-signify-certificate/Dockerfile @@ -0,0 +1,14 @@ +FROM python:3.12.5-alpine3.20 + +# Allow statements and log messages to immediately appear in the Knative logs +ENV PYTHONUNBUFFERED True + +WORKDIR /app + +COPY ./cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py . +COPY ./cmd/cloud-run/rotate-signify-certificate/requirements.txt . + +RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ + apk add --no-cache ca-certificates + +CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "rotate-signify-certificate:app"] diff --git a/cmd/cloud-run/rotate-signify-certificate/requirements.txt b/cmd/cloud-run/rotate-signify-certificate/requirements.txt new file mode 100644 index 000000000000..0aff4ef06eb4 --- /dev/null +++ b/cmd/cloud-run/rotate-signify-certificate/requirements.txt @@ -0,0 +1,5 @@ + +flask==2.3.2 +google-cloud-secret-manager==2.20.2 +cloudevents==1.9.0 +gunicorn==22.0.0 \ No newline at end of file diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py new file mode 100644 index 000000000000..653ddae2b38e --- /dev/null +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -0,0 +1,279 @@ +"""Simple PubSub handler to rotate signify certificates""" + +import datetime +import os +import base64 +import json +import sys +import tempfile +import traceback +from typing import Any, Dict, List +import requests +from google.cloud import secretmanager +from flask import Flask, Response, request, make_response +from cryptography import x509 +from cryptography.hazmat.primitives import serialization, hashes +from cryptography.hazmat.primitives.serialization import pkcs7, Encoding, PrivateFormat +from cryptography.hazmat.primitives.asymmetric import rsa + +app = Flask(__name__) +project_id: str = os.getenv("PROJECT_ID", "") +component_name: str = os.getenv("COMPONENT_NAME", "") +application_name: str = os.getenv("APPLICATION_NAME", "") + + +# TODO(kacpermalachowski): Move it to common package +class LogEntry(dict): + """LogEntry simplifies logging by returning JSON string""" + + def __str__(self): + return json.dumps(self) + + +# pylint: disable-msg=too-many-locals +@app.route("/", methods=["POST"]) +def rotate_signify_secret() -> Response: + """HTTP webhook handler for rotating signify secrets""" + log_fields: Dict[str, Any] = prepare_log_fields() + log_fields["labels"]["io.kyma.app"] = "signify-certificate-rotate" + + try: + pubsub_message = get_pubsub_message() + + secret_rotate_msg = extract_message_data(pubsub_message) + + if secret_rotate_msg["labels"]["type"] != "signify": + return prepare_error_response("Unsupported resource type", log_fields) + + secret_data = get_secret(secret_rotate_msg["name"]) + + token_url = secret_data["tokenURL"] + client_id = secret_data["clientID"] + cert_service_url = secret_data["certServiceURL"] + old_cert_data = base64.b64decode(secret_data["certData"]) + old_pk_data = base64.b64decode(secret_data["privateKeyData"]) + + old_cert = x509.load_pem_x509_certificate(old_cert_data) + + if "password" in secret_data and secret_data["password"] != "": + old_pk_data = decrypt_private_key( + old_pk_data, secret_data["password"].encode() + ) + + new_private_key = rsa.generate_private_key(public_exponent=65537, key_size=4096) + + csr = ( + x509.CertificateSigningRequestBuilder() + .subject_name(old_cert.subject) + .sign(new_private_key, hashes.SHA256()) + ) + + access_token = fetch_access_token( + old_cert_data, old_pk_data, token_url, client_id + ) + + created_at = datetime.datetime.now().strftime("%d-%m-%Y %H:%M:%S") + + new_certs: List[x509.Certificate] = fetch_new_certificate( + csr, access_token, cert_service_url + ) + + new_secret_data = prepare_new_secret( + new_certs, new_private_key, secret_data, created_at + ) + + set_secret( + secret_id=secret_rotate_msg["name"], data=json.dumps(new_secret_data) + ) + + print( + LogEntry( + severity="INFO", + message="Certificate rotated successfully", + **log_fields, + ) + ) + + return "Certificate rotated successfully" + # pylint: disable=broad-exception-caught + except Exception as exc: + return prepare_error_response(exc, log_fields) + + +def fetch_new_certificate( + csr: x509.CertificateSigningRequest, access_token: str, certificate_service_url: str +): + """Fetch new certificates from given certificate service""" + crt_create_payload = json.dumps( + { + "csr": { + "value": csr.public_bytes(serialization.Encoding.PEM).decode("utf-8") + }, + "validity": {"value": 7, "type": "DAYS"}, + "policy": "sap-cloud-platform-clients", + } + ) + + cert_create_response = requests.post( + certificate_service_url, + headers={ + "Authorization": f"Bearer {access_token}", + "Content-Type": "application/json", + "Accept": "applicaiton/json", + }, + data=crt_create_payload, + timeout=10, + ).json() + + pkcs7_certs = cert_create_response["certificateChain"]["value"].encode() + + return pkcs7.load_pem_pkcs7_certificates(pkcs7_certs) + + +def prepare_new_secret( + certificates: List[x509.Certificate], + private_key: rsa.RSAPrivateKey, + secret_data: Dict[str, Any], + created_at: str, +) -> Dict[str, Any]: + """Prepare new secret data""" + + # format certificates + certs_string = "" + + for cert in certificates: + certs_string += f"subject={cert.subject.rfc4514_string()}\n" + certs_string += f"issuer={cert.issuer.rfc4514_string()}\n" + certs_string += f"{cert.public_bytes(Encoding.PEM).decode()}\n" + + private_key_bytes = private_key.private_bytes( + Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() + ) + + return { + "certData": base64.b64encode(certs_string.encode()).decode(), + "privateKeyData": base64.b64encode(private_key_bytes).decode(), + "createdAt": created_at, + "certServiceURL": secret_data["certServiceURL"], + "tokenURL": secret_data["tokenURL"], + "clientID": secret_data["clientID"], + "password": "", # keep it empty to maintain structure, but indicate it's not needed + } + + +def extract_message_data(pubsub_message: Any) -> Any: + """Extract secret rotate emssage from pubsub message""" + if pubsub_message["attributes"]["eventType"] != "SECRET_ROTATE": + # pylint: disable=broad-exception-raised + raise Exception("Unsupported event type") + + data = base64.b64decode(pubsub_message["data"]) + return json.loads(data) + + +def decrypt_private_key(private_key_data: bytes, password: bytes) -> bytes: + """Decrypr encrypted private key""" + # pylint: disable=line-too-long + private_key = serialization.load_pem_private_key(private_key_data, password) + + # pylint: disable=line-too-long + return private_key.private_bytes( + Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() + ) + + +def fetch_access_token( + certificate: bytes, private_key: bytes, token_url: str, client_id: str +) -> str: + """fetches access token from given token_url using certificate and private key""" + # Use temporary file for old cert and key because requests library needs file paths, + # it's not a security concern because the code is running in known environment controlled by us + # pylint: disable=line-too-long + with tempfile.NamedTemporaryFile() as old_cert_file, tempfile.NamedTemporaryFile() as old_key_file: + + old_cert_file.write(certificate) + old_cert_file.flush() + + old_key_file.write(private_key) + old_key_file.flush() + + # pylint: disable=line-too-long + access_token_response = requests.post( + token_url, + cert=(old_cert_file.name, old_key_file.name), + data={ + "grant_type": "client_credentials", + "client_id": client_id, + }, + timeout=30, + ).json() + + return access_token_response["access_token"] + + +# TODO(kacpermalachowski): Move it to common package +def prepare_log_fields() -> Dict[str, Any]: + """prepare_log_fields prapares basic log fields""" + log_fields: Dict[str, Any] = {} + request_is_defined = "request" in globals() or "request" in locals() + if request_is_defined and request: + trace_header = request.headers.get("X-Cloud-Trace-Context") + if trace_header and project_id: + trace = trace_header.split("/") + log_fields["logging.googleapis.com/trace"] = ( + f"projects/{project_id}/traces/{trace[0]}" + ) + log_fields["Component"] = "signify-certificate-rotator" + log_fields["labels"] = {"io.kyma.component": "signify-certificate-rotator"} + return log_fields + + +# TODO(kacpermalachowski): Move it to common package +def get_pubsub_message(): + """get_pubsub_message unpacks pubsub message and does basic type checks""" + envelope = request.get_json() + if not envelope: + # pylint: disable=broad-exception-raised + raise Exception("no Pub/Sub message received") + + if not isinstance(envelope, dict) or "message" not in envelope: + # pylint: disable=broad-exception-raised + raise Exception("invalid Pub/Sub message format") + + pubsub_message = envelope["message"] + return pubsub_message + + +# TODO(kacpermalachowski): Move it to common package +def prepare_error_response(err: str, log_fields: Dict[str, Any]) -> Response: + """prepare_error_response return error response with stacktrace""" + _, exc_value, _ = sys.exc_info() + stacktrace = repr(traceback.format_exception(exc_value)) + print( + LogEntry( + severity="ERROR", + message=f"Error: {err}\nStack:\n {stacktrace}", + **log_fields, + ) + ) + resp = make_response() + resp.content_type = "application/json" + resp.status_code = 500 + return resp + + +def get_secret(secret_id: str): + """Get latest secret version from secret manager""" + client = secretmanager.SecretManagerServiceClient() + + response = client.access_secret_version(name=f"{secret_id}/versions/15") + secret_value = response.payload.data.decode("UTF-8") + + return json.loads(secret_value) + + +def set_secret(secret_id: str, data: str): + """Sets new secret version in secret manager""" + client = secretmanager.SecretManagerServiceClient() + + client.add_secret_version(parent=secret_id, payload={"data": data.encode()}) diff --git a/cmd/cloud-run/rotate-signify-certificate/test_fixtures.py b/cmd/cloud-run/rotate-signify-certificate/test_fixtures.py new file mode 100644 index 000000000000..4c59cadb913c --- /dev/null +++ b/cmd/cloud-run/rotate-signify-certificate/test_fixtures.py @@ -0,0 +1,21 @@ +"""Module to keep data required by tests""" + +mocked_access_token_response = {"access_token": "test-access-token"} + +mocked_secret_data = { + "tokenURL": "http://token.example.com", + "clientID": "test-client-id", + "certServiceURL": "http://cert.example.com", + # pylint: disable=line-too-long + "certData": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUY3ekNDQTllZ0F3SUJBZ0lVYUlqenlHUytCUFFJZ0thYlhoU2xhTTk0dGpNd0RRWUpLb1pJaHZjTkFRRUwKQlFBd2dZWXhDekFKQmdOVkJBWVRBbGhZTVJJd0VBWURWUVFJREFsVGRHRjBaVTVoYldVeEVUQVBCZ05WQkFjTQpDRU5wZEhsT1lXMWxNUlF3RWdZRFZRUUtEQXREYjIxd1lXNTVUbUZ0WlRFYk1Ca0dBMVVFQ3d3U1EyOXRjR0Z1CmVWTmxZM1JwYjI1T1lXMWxNUjB3R3dZRFZRUUREQlJEYjIxdGIyNU9ZVzFsVDNKSWIzTjBibUZ0WlRBZUZ3MHkKTkRBNU1USXdPRFUwTWpkYUZ3MHpOREE1TVRBd09EVTBNamRhTUlHR01Rc3dDUVlEVlFRR0V3SllXREVTTUJBRwpBMVVFQ0F3SlUzUmhkR1ZPWVcxbE1SRXdEd1lEVlFRSERBaERhWFI1VG1GdFpURVVNQklHQTFVRUNnd0xRMjl0CmNHRnVlVTVoYldVeEd6QVpCZ05WQkFzTUVrTnZiWEJoYm5sVFpXTjBhVzl1VG1GdFpURWRNQnNHQTFVRUF3d1UKUTI5dGJXOXVUbUZ0WlU5eVNHOXpkRzVoYldVd2dnSWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUNEd0F3Z2dJSwpBb0lDQVFEa01hSWJzREhKSXBmZ3lOeUp2UWUycXZTS1ZhOWhSWGQ3SFB2SG1nZjJxUmkyejN6YmVRc0xuWnk1CmhkOE1LZnVHR25kb0FwdC9SR3RYRmgzTlowcVU2R1pIL2UxNWZWQmpqZW8rd3NtWHRXVmUrMGZuTE5QWXByT0IKdnFhZmYybUZmaytucUJCWE9XRExHV2FEclNrR0gvZ202WnZVOWNhNDh6YlRiOE0rRVc4T3VuWTRyTTZJUEcxNQpxSGJlRjRhMFF2WFJmbSswMytCbEJlUHFjWjg1ZEpjNnkvd1Qyajc0ckEyS1RLdWdrczUyUitGc0hOYS9jbEpTCko5SmlsOEdOa1RObmtRaGErZDZRcVpyUzBuTW4vSXJvdDlBdDN2Z0FWV2Qrb29DVWNxTUFxdURiVE1jRzhYUi8KcHI4bkl5bk5KVFhzYnJsSVZQL1BOaGdhZ09ra0gzYVFzVTIzMy9RUTFCNWlaMUpqMjhaSi9ycjBKZDV4TlJncwpoS3YzNUZxdzJ3SWFqT0tuWFBYaElHS2J0b0c0N09FSEJabGJvTTV6bHpqSCtPVERLSXY2NGg1dTBoWEt5ckZ1CjU5RTBTUFZsMEprYjRITVdtMEtzbGRiSjIzYUVsbmNIdlVBZjVWek01STlhMVVEYThjRW9RdTUrb0thaWVoaW4KTGE2QmNxUHM0R3pRQUdiTEpLdStTYk83SDR4aFJRanN3blY3WWkyVHJFbytiWDFIZXBhZ3oyL05MMS9iSlNSTgpNVmlmOGhRMFRaaHlxY1VGSWFhYmFyVldGb2RZVWFxeFRuUHIzdUM5L2pZak9Vd0tOSG9ySWhqejBIR005cHY3CjRUT0NmR01TY1RYQWo4Y1RqR0RUTjladjRQMGMrOFVycEx0ZjJOVldNUGtIczh3QzF3SURBUUFCbzFNd1VUQWQKQmdOVkhRNEVGZ1FVRHhtQmdjSVJpbVRDaGRkMHBoYnhBcjhZM0s4d0h3WURWUjBqQkJnd0ZvQVVEeG1CZ2NJUgppbVRDaGRkMHBoYnhBcjhZM0s4d0R3WURWUjBUQVFIL0JBVXdBd0VCL3pBTkJna3Foa2lHOXcwQkFRc0ZBQU9DCkFnRUFKazdkYXJISXRtUjVaK05qVk9WV1ZjNWtOMnVGRVdFbEkrazdHbjdEZDhRdE56VlBuQW5aK1IzSVdXQUEKSXdPQ2Z3YmJTZGRWakwxSnp3aVk5cFNhd3FEd3NlM1kzR3lzNUhHMXFqeFFCaFF3NXdVMTc4aHl6cGhwNkY1cwp5SmZxMCtvZ2hPdFNxOUp6bGFYNW5MdXJNZ3hWajBrZmNodXRoNjNFdnBwa29sNHcvb3Z3OTZvdEdUVmcvUm14Cm9yYnN4MVE5aERSalNNRUdaRHd2R0xONUx1K0JDQUFXcFp4N3c1VTl0bE9CaGxFSFF0cGl6cy9aZ1k5S0pabXAKc3dKMm0rTWp5K1RrMWt5ZW1nT3hmUDBZQkZDRnFHZU8xOWxqWjY1SmRTOUlSdnY5MEp5c1hHY0ZJbVkxRTVERQorTXovdkFaRVN4ZmRrSWFqN0xuZGZVNUJITVY0bDRDR0ZaRDdMYWpLcFJybzlybmU5QmRINHkrOVd0R1RHRlZCCmFzOVE3VGZDRTE1YkpYWE5QYzJXL0VQM1RjVTdlejI1WFcyR1JyVHAxVXA0MlhmRCtVUm9CUEdnU0lWSE4vdFoKbWxZbTlrQW8xajdKM2RnV3VlU1lFamxhRG1OSGlBbGtWOTJnSm55aDVvK3cxU1BudnZtMEkxejNyM0NVT25qYgp2UGFmVWE3TjdYcE1QREs5NW5rc2RZYkVlUWEzUDZ6V3VuL0VQUjdJblV1VVFBWFpPZUlEakVkTFZObzZqcDJuCjFpQUtlT3dNQU1FU00rWWZmSHVSN0FJY3ZHUXNBUU9hSlRjQVVjeldzSHFaR0RtRlVVU0tYSTBnL1ZyUFhsMVUKb0dBQ3pzNnZCbDZHaHo2RURhaURJVVlHTXM3em0yVjY0WTVRcGVJdTIvclJFWjQ9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K", + "privateKeyData": "LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUpRZ0lCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQ1N3d2dna29BZ0VBQW9JQ0FRRGtNYUlic0RISklwZmcKeU55SnZRZTJxdlNLVmE5aFJYZDdIUHZIbWdmMnFSaTJ6M3piZVFzTG5aeTVoZDhNS2Z1R0duZG9BcHQvUkd0WApGaDNOWjBxVTZHWkgvZTE1ZlZCamplbyt3c21YdFdWZSswZm5MTlBZcHJPQnZxYWZmMm1GZmsrbnFCQlhPV0RMCkdXYURyU2tHSC9nbTZadlU5Y2E0OHpiVGI4TStFVzhPdW5ZNHJNNklQRzE1cUhiZUY0YTBRdlhSZm0rMDMrQmwKQmVQcWNaODVkSmM2eS93VDJqNzRyQTJLVEt1Z2tzNTJSK0ZzSE5hL2NsSlNKOUppbDhHTmtUTm5rUWhhK2Q2UQpxWnJTMG5Nbi9Jcm90OUF0M3ZnQVZXZCtvb0NVY3FNQXF1RGJUTWNHOFhSL3ByOG5JeW5OSlRYc2JybElWUC9QCk5oZ2FnT2trSDNhUXNVMjMzL1FRMUI1aVoxSmoyOFpKL3JyMEpkNXhOUmdzaEt2MzVGcXcyd0lhak9LblhQWGgKSUdLYnRvRzQ3T0VIQlpsYm9NNXpsempIK09UREtJdjY0aDV1MGhYS3lyRnU1OUUwU1BWbDBKa2I0SE1XbTBLcwpsZGJKMjNhRWxuY0h2VUFmNVZ6TTVJOWExVURhOGNFb1F1NStvS2FpZWhpbkxhNkJjcVBzNEd6UUFHYkxKS3UrClNiTzdINHhoUlFqc3duVjdZaTJUckVvK2JYMUhlcGFnejIvTkwxL2JKU1JOTVZpZjhoUTBUWmh5cWNVRklhYWIKYXJWV0ZvZFlVYXF4VG5QcjN1QzkvallqT1V3S05Ib3JJaGp6MEhHTTlwdjc0VE9DZkdNU2NUWEFqOGNUakdEVApOOVp2NFAwYys4VXJwTHRmMk5WV01Qa0hzOHdDMXdJREFRQUJBb0lDQUJLdjA5VjQ5aUxCVlV4Vm9Uc2JBcGVJCkRyMklxeDR5bVBNT2hROHI3Y3l2dlVrMGhadUN5K1R0YzhqODR3UDFLTmFyQjlpUEpNVVB5U0NEdkU0TCt0OWwKM0FFTTZpdTI3T1NIaEhBVG9IeEVJUTNkcUY0N01yVlNZQm5YRHlzTzU5NXU0UlFGaWMwSUhhL3BkUmFCdkNoMwpGQlJsQmdZSVdIZmdodWxhcjM0L2pEelpzb0VOVHdrallGaFpuak90Y1RKYTEybUtuKzJMeXkyZ0JWdzNaaHdjCjVaNHRnb2VYcVpkN1NNMCtDUmRUQ1FXYm82U29XZXovUTM3Y2Z2aU84bzA4SDUyeXZVdEI4ZzF6cml6bk5LeWUKSER3Qmd0S2hmZVZTYm5GcTJrKzRjWXZHUitLb09oWTNUZ2NwYWJiejcySTYwckhPd1cwMXloQmh5WDNiZDZhbwoxaTM3dHBKODBzZFR6dmR5bEdCR1BDejZ2TXY0cDA4SnpBLzFYdG55MDFHczJBbktVWkVnY21rUDVPbGNEdXA5ClBlNEJCNlpwajR6MFc5dTVScTF1ZjZKdGkvY2JpdytQT0pGS3hHU2p3d0dwbWNXVUNmRC8yVUlhbG11SUhOdXcKclFvZDFFQ0t1Wm9oaDNTeWFRVFhzVUdKSi9OSVYyZ3AzazZScGlvcVNGQWh0empBaEIvUnpKSitWOTdKSmpoTgpNa3RUZXQ0dzI1K2VKejJvREorcTl1M05TVWNmZDYwQ0RhZkNmSldKSjh2S0JjUWY2cGNGSDF0TCt4NUVHQ281Cmc2MEhob0t6RGs4U3Yybm50a2pRcUJuMTMwaXdlZlgzQVNHd2p1Wk9BYW1Eb3Y0bWRyWVJkOHVGeHhYeG5WWVoKckNhc0ZEaDg5V0p0d2YyY1VhZVpBb0lCQVFEOXpvS3ZLMndNNm5WbmpVUGxkdzZiUEdUYUQ0MGdCQmdGckIzWQpYRWp3d0ZoNlJvcmkzRFYxUXUrZ2JMYTFNOTRHaUFhZlp6VXpFRkI3YmsvZXcza3d5dE9jZnYxT0ZDV2RpdDIyCmJCQW1pazN4RHJUY0tGRVVrUjRJV0hhWUEwWGlzSExFTFYxcCtWZUJWc1NiSVhFRnVsbk9ITm9vUGtyTUVhVkQKMThMbDZaMzQ4SWN0Yi9vVi94SmZZTEYwcFNCcUF5WlJSVk0rdHdlazB6ZnlvRElYRzhDSTEwc0p4dkNka3h4VQpETE9Jb25NM1Fjc05ianpRUXJxQmJvOHp0L2dySzhyMGNYZjBFUzFYUkI0M1ZXY0FmUVMwcTdBck5NcVZtZ0JzClJwMFEzU2c1ZzZjL0lNeHZFQ0xqSUdqTTJWcHhSVy9LUkFmMit6a3R4ZW5CaDZzNUFvSUJBUURtS25YVFRDamIKWlhCbThyT3lLV3p1RjBiWWdMRlRsMWdUTy95clhjZVFnaTFRQXU0eWZxMGw5azNIRVREZnZlQ05mbUVFckU0NwppQ25Xc0o2YkU2c2tGcjlPNitwcGIrNTZlWkN4S09CTUQ4NW8xVnJpMUU1TkZVVFF0Y21DMjNDZWU0RGpyNWpFCjQwOTFveDl2aFZ4Nzl6cTNidVBEUmJJbGRtd3pMdFQ2dDhySjZRanhJZkFLMlFUWlNUS0hyU1FHVUdTMDlncW8KS0pMc3NCdWZCKzZXaSt5WWxzRjVLNGhKb3VXSjdIdGQwRUJZT2RwQ2drbk1aa0JrTERBa2d6OFg2WDMvVmxLOQpDcmtucm0vTmhoSStrd3NBUUxtY3F2OW53TFAxOFZTMW9KSUxpR01hQmZhV2RQTVJkdGlMbTVyV2hYa2tNVFpyCmVVTlpUNGRobUU2UEFvSUJBQ3dSVmlsNS9abkVmN3dLRUJiZ2RCM0ovNnpJOUhUeG9RWWpHMDk4dm9GbXREWU8KMTRDS1FaSHBuRmViYUtIUHJxWXRxaEMxUjh4azhjUnNvOXVHengwRXFuZEtxVXFNUHZlNnE4Skh3d29lQzJobQphRXF0aVY1cnorNGRCZ0pnMXhxRURuazNjemU2UGxKTHNYbEo5aWpMY1ZwVE1pRzRweDRGbGs5UVFCdlJVMGRzCjQ5dWt6S1JURkdxUkRGNjNhUWhmSWFFTXkyWmhPeWVJTVBla2p1M3FPS1RKMU5LT3d5cG81NHRFWlV1OUFRcEEKRnpSdXhvcFdlN0dMYzd2RUhvZklZOFhMaWN0THpEYVJzamxqVDY0TEJ1MGp2a1BTTjdaLzljRFhCZUJ1dGRRKwpvcVJZV29hU0k2eVI3UUJjWU5LMy9xLzgzZkVwTDRWZURyYzJpaWtDZ2dFQkFOYzlaOWhFUllUK2RNSGhQNGhoCjB2dElpemtXQ0Q1YXJzTzErbENyTDJBRlRLaWhSTzIxcHVoMFVFSFQ0cmVwa2wvZmlvemJNWUhja1B1elNXOU8KVW1JNlg0ZEhlWHhHYjJiYlhpTStUWnJ6d1J3cVFZY1Q4WEdHYlVjY2FTalZXNWpwZUJ2MGIxSFlITXV1MDB2dwpGQS9kb0d3LzZBRUpvVklGVDZRQnJLd1Z5aTlObk00YTVhYlBVZ2g4dWlORkdBWkxraEhrY1F6V2ZLZVkvUXVZCldGY3kzUHZKSjM3UDVmQ3V4Q2RhSGZnYU1zSU92L3dvYVNrYmlpWGphTllNWXFsUzhrQWhFdGkwT1hoUldUNkMKblhjay9VbXNQYWUwQllxYUcvRG9VVVpVdFo3UldaUFJkY3MzN01NdE44NCtvdXJ3QU55R3BSU1dYeFFiR2toNwpMU01DZ2dFQVZSell6MEUzY1VGcjUycUY0V3J6dU1JanNiZWpOb25MbjhOMWpUR2VDNjk4eUJOSDUzemwyS3ZzClYwSVhlQWlNN2xqdlhsSjRrb1p6QS9RclpMMzFpYkxnVTJsRzcyazZaRUNRQms4TVprQU5hMFJsZ2pxVTU2RHgKcGVDMHRBQ2x3aFhuSC9oRVVpTzduaThMWlZaSWtzaFlzaFVtTkM4RDRTRlJPVlF2eG5hdUg3TXhUeDJ6VFZlaQpmcnBsaEhYTC9EN0w0Qm96UGlIRkZzaFB4cVlIMkFsMjYxZ1pVd1RaT2c3dld3MHNjaHI1aHJ3QmlVTDdjaFUwCmpURXNYb0xmMDd0aC93NThsUjZ2ZUhJZTBnVzNBZnVQZjB6QjU0ZHY2aVYxWTZZUTdsSXpTbnpDajFTTnFIT2wKQzhOVDZQMEFPZDJBZnVRT2kwVVY3MzRHNjg3TlhnPT0KLS0tLS1FTkQgUFJJVkFURSBLRVktLS0tLQo=", + "password": "", +} + + +mocked_cert_create_response = { + "certificateChain": { + # pylint: disable=line-too-long + "value": "-----BEGIN PKCS7-----\nMIIGHgYJKoZIhvcNAQcCoIIGDzCCBgsCAQExADALBgkqhkiG9w0BBwGgggXzMIIF\n7zCCA9egAwIBAgIUaIjzyGS+BPQIgKabXhSlaM94tjMwDQYJKoZIhvcNAQELBQAw\ngYYxCzAJBgNVBAYTAlhYMRIwEAYDVQQIDAlTdGF0ZU5hbWUxETAPBgNVBAcMCENp\ndHlOYW1lMRQwEgYDVQQKDAtDb21wYW55TmFtZTEbMBkGA1UECwwSQ29tcGFueVNl\nY3Rpb25OYW1lMR0wGwYDVQQDDBRDb21tb25OYW1lT3JIb3N0bmFtZTAeFw0yNDA5\nMTIwODU0MjdaFw0zNDA5MTAwODU0MjdaMIGGMQswCQYDVQQGEwJYWDESMBAGA1UE\nCAwJU3RhdGVOYW1lMREwDwYDVQQHDAhDaXR5TmFtZTEUMBIGA1UECgwLQ29tcGFu\neU5hbWUxGzAZBgNVBAsMEkNvbXBhbnlTZWN0aW9uTmFtZTEdMBsGA1UEAwwUQ29t\nbW9uTmFtZU9ySG9zdG5hbWUwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoIC\nAQDkMaIbsDHJIpfgyNyJvQe2qvSKVa9hRXd7HPvHmgf2qRi2z3zbeQsLnZy5hd8M\nKfuGGndoApt/RGtXFh3NZ0qU6GZH/e15fVBjjeo+wsmXtWVe+0fnLNPYprOBvqaf\nf2mFfk+nqBBXOWDLGWaDrSkGH/gm6ZvU9ca48zbTb8M+EW8OunY4rM6IPG15qHbe\nF4a0QvXRfm+03+BlBePqcZ85dJc6y/wT2j74rA2KTKugks52R+FsHNa/clJSJ9Ji\nl8GNkTNnkQha+d6QqZrS0nMn/Irot9At3vgAVWd+ooCUcqMAquDbTMcG8XR/pr8n\nIynNJTXsbrlIVP/PNhgagOkkH3aQsU233/QQ1B5iZ1Jj28ZJ/rr0Jd5xNRgshKv3\n5Fqw2wIajOKnXPXhIGKbtoG47OEHBZlboM5zlzjH+OTDKIv64h5u0hXKyrFu59E0\nSPVl0Jkb4HMWm0KsldbJ23aElncHvUAf5VzM5I9a1UDa8cEoQu5+oKaiehinLa6B\ncqPs4GzQAGbLJKu+SbO7H4xhRQjswnV7Yi2TrEo+bX1Hepagz2/NL1/bJSRNMVif\n8hQ0TZhyqcUFIaabarVWFodYUaqxTnPr3uC9/jYjOUwKNHorIhjz0HGM9pv74TOC\nfGMScTXAj8cTjGDTN9Zv4P0c+8UrpLtf2NVWMPkHs8wC1wIDAQABo1MwUTAdBgNV\nHQ4EFgQUDxmBgcIRimTChdd0phbxAr8Y3K8wHwYDVR0jBBgwFoAUDxmBgcIRimTC\nhdd0phbxAr8Y3K8wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAgEA\nJk7darHItmR5Z+NjVOVWVc5kN2uFEWElI+k7Gn7Dd8QtNzVPnAnZ+R3IWWAAIwOC\nfwbbSddVjL1JzwiY9pSawqDwse3Y3Gys5HG1qjxQBhQw5wU178hyzphp6F5syJfq\n0+oghOtSq9JzlaX5nLurMgxVj0kfchuth63Evppkol4w/ovw96otGTVg/Rmxorbs\nx1Q9hDRjSMEGZDwvGLN5Lu+BCAAWpZx7w5U9tlOBhlEHQtpizs/ZgY9KJZmpswJ2\nm+Mjy+Tk1kyemgOxfP0YBFCFqGeO19ljZ65JdS9IRvv90JysXGcFImY1E5DE+Mz/\nvAZESxfdkIaj7LndfU5BHMV4l4CGFZD7LajKpRro9rne9BdH4y+9WtGTGFVBas9Q\n7TfCE15bJXXNPc2W/EP3TcU7ez25XW2GRrTp1Up42XfD+URoBPGgSIVHN/tZmlYm\n9kAo1j7J3dgWueSYEjlaDmNHiAlkV92gJnyh5o+w1SPnvvm0I1z3r3CUOnjbvPaf\nUa7N7XpMPDK95nksdYbEeQa3P6zWun/EPR7InUuUQAXZOeIDjEdLVNo6jp2n1iAK\neOwMAMESM+YffHuR7AIcvGQsAQOaJTcAUczWsHqZGDmFUUSKXI0g/VrPXl1UoGAC\nzs6vBl6Ghz6EDaiDIUYGMs7zm2V64Y5QpeIu2/rREZ4xAA==\n-----END PKCS7-----\n" + } +} diff --git a/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py new file mode 100644 index 000000000000..4902a449362e --- /dev/null +++ b/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py @@ -0,0 +1,100 @@ +"""Module containing tests for signify secret rotator""" + +import base64 +import json +from typing import Any, Dict +import unittest +from unittest.mock import MagicMock, Mock, patch +from flask.testing import FlaskClient +import test_fixtures +from rotate_signify_certificate import app, get_pubsub_message, prepare_log_fields + +project_id: str = "test-project" +component_name: str = "test_component" +application_name: str = "test_application" + + +class TestPubSubMessageHandler(unittest.TestCase): + """Test cases for fucntion to handle pub sub message""" + + def setUp(self) -> None: + self.app: FlaskClient = app.test_client() + self.app.testing = True + + @patch("rotate_signify_certificate.project_id", project_id) + @patch("rotate_signify_certificate.component_name", component_name) + @patch("rotate_signify_certificate.application_name", application_name) + def test_prepare_log_fields(self): + """Test that prepare log fields based on current env variables and cloud trace context""" + with app.test_request_context( + "/", headers={"X-Cloud-Trace-Context": "trace-id/other-info"} + ): + log_fields: Dict[str, Any] = prepare_log_fields() + self.assertIn("logging.googleapis.com/trace", log_fields) + self.assertEqual( + log_fields["logging.googleapis.com/trace"], + f"projects/{project_id}/traces/trace-id", + ) + self.assertEqual(log_fields["Component"], "signify-certificate-rotator") + + def test_get_pubsub_message_no_message(self): + """It should raise exception when no pubsub message is received""" + with app.test_request_context("/", json={}): + with self.assertRaises(Exception): + get_pubsub_message() + + def test_get_pubsub_message_invalid_format(self): + """It should raise exception when invalid pubsub message is received""" + with app.test_request_context("/", json={"invalid_key": "value"}): + with self.assertRaises(Exception): + get_pubsub_message() + + +class TestRotateSignifySecret(unittest.TestCase): + """Tests for rotating signify secret""" + + @classmethod + def setUpClass(cls) -> None: + cls.app: FlaskClient = app.test_client() + cls.app.testing = True + + def setUp(self): + """Setup for each test""" + self.pubsub_message = { + "message": { + "attributes": {"eventType": "SECRET_ROTATE"}, + "data": base64.b64encode( + json.dumps( + {"labels": {"type": "signify"}, "name": "test-secret"} + ).encode() + ).decode(), + } + } + + self.secret_data = test_fixtures.mocked_secret_data + self.mocked_token_response = test_fixtures.mocked_access_token_response + self.mocked_cert_create_response = test_fixtures.mocked_cert_create_response + + @patch("requests.post") + @patch("rotate_signify_certificate.set_secret") + @patch("rotate_signify_certificate.get_secret") + def test_rotate_signify_secret_success( + self, + mock_get_secret: Mock, + mock_set_secret: Mock, + mock_post: Mock, + ): + """Test success repsonse for valid pubsub""" + mock_get_secret.return_value = self.secret_data + mock_post.side_effect = [ + MagicMock(json=lambda: self.mocked_token_response), + MagicMock(json=lambda: self.mocked_cert_create_response), + ] + + response = self.app.post("/", json=self.pubsub_message) + self.assertEqual(response.status_code, 200) + mock_set_secret.assert_called_once() + + +if __name__ == "__main__": + unittest.main() From dfdc915c76517cc928ca44d4db0c444009dec1e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 12 Sep 2024 11:03:42 +0200 Subject: [PATCH 02/24] Add workflows and fix get_secret --- .../pull-build-rotate-signify-certificate.yml | 15 +++++++++++++++ .../push-build-rotate-signify-certificate.yml | 16 ++++++++++++++++ .../rotate_signify_certificate.py | 2 +- 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/pull-build-rotate-signify-certificate.yml create mode 100644 .github/workflows/push-build-rotate-signify-certificate.yml diff --git a/.github/workflows/pull-build-rotate-signify-certificate.yml b/.github/workflows/pull-build-rotate-signify-certificate.yml new file mode 100644 index 000000000000..2a1a8cc48898 --- /dev/null +++ b/.github/workflows/pull-build-rotate-signify-certificate.yml @@ -0,0 +1,15 @@ +name: pull-build-rotate-signify-certificate +# description: "Build rotate-signify-certificate image for rotating signify certificates. +on: + pull_request_target: + types: [ opened, edited, synchronize, reopened, ready_for_review ] + paths: + - "cmd/cloud-run/rotate-signify-certificate/**" + +jobs: + build-image: + uses: ./.github/workflows/image-builder.yml + with: + name: test-infra/rotatesignifycertificate + dockerfile: cmd/cloud-run/rotate-signify-certificate/Dockerfile + context: . \ No newline at end of file diff --git a/.github/workflows/push-build-rotate-signify-certificate.yml b/.github/workflows/push-build-rotate-signify-certificate.yml new file mode 100644 index 000000000000..86f94e7ad691 --- /dev/null +++ b/.github/workflows/push-build-rotate-signify-certificate.yml @@ -0,0 +1,16 @@ +name: push-build-rotate-signify-certificate +# description: "Build rotate-signify-certificate image for rotating signify certificates. +on: + push: + branches: + - main + paths: + - "cmd/cloud-run/rotate-signify-certificate/**" + +jobs: + build-image: + uses: ./.github/workflows/image-builder.yml + with: + name: test-infra/rotatesignifycertificate + dockerfile: cmd/cloud-run/rotate-signify-certificate/Dockerfile + context: . \ No newline at end of file diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py index 653ddae2b38e..1a91e6eb2068 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -266,7 +266,7 @@ def get_secret(secret_id: str): """Get latest secret version from secret manager""" client = secretmanager.SecretManagerServiceClient() - response = client.access_secret_version(name=f"{secret_id}/versions/15") + response = client.access_secret_version(name=f"{secret_id}/versions/latest") secret_value = response.payload.data.decode("UTF-8") return json.loads(secret_value) From d328aec080b2aa902006f0d60f26ab5b069a5c5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= <38684517+KacperMalachowski@users.noreply.github.com> Date: Fri, 13 Sep 2024 13:30:23 +0200 Subject: [PATCH 03/24] Apply suggestions from code review Co-authored-by: Patryk Dobrowolski --- .../rotate_signify_certificate.py | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py index 1a91e6eb2068..ca65277f7c78 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -24,7 +24,7 @@ # TODO(kacpermalachowski): Move it to common package class LogEntry(dict): - """LogEntry simplifies logging by returning JSON string""" + """Simplifies logging by returning a JSON string.""" def __str__(self): return json.dumps(self) @@ -33,7 +33,7 @@ def __str__(self): # pylint: disable-msg=too-many-locals @app.route("/", methods=["POST"]) def rotate_signify_secret() -> Response: - """HTTP webhook handler for rotating signify secrets""" + """HTTP webhook handler for rotating Signify secrets.""" log_fields: Dict[str, Any] = prepare_log_fields() log_fields["labels"]["io.kyma.app"] = "signify-certificate-rotate" @@ -136,7 +136,7 @@ def prepare_new_secret( secret_data: Dict[str, Any], created_at: str, ) -> Dict[str, Any]: - """Prepare new secret data""" + """Prepares new secret data with updated certificates and private key.""" # format certificates certs_string = "" @@ -162,7 +162,7 @@ def prepare_new_secret( def extract_message_data(pubsub_message: Any) -> Any: - """Extract secret rotate emssage from pubsub message""" + """Extracts secret rotation message from the Pub/Sub message.""" if pubsub_message["attributes"]["eventType"] != "SECRET_ROTATE": # pylint: disable=broad-exception-raised raise Exception("Unsupported event type") @@ -172,7 +172,7 @@ def extract_message_data(pubsub_message: Any) -> Any: def decrypt_private_key(private_key_data: bytes, password: bytes) -> bytes: - """Decrypr encrypted private key""" + """Decrypts an encrypted private key.""" # pylint: disable=line-too-long private_key = serialization.load_pem_private_key(private_key_data, password) @@ -230,23 +230,22 @@ def prepare_log_fields() -> Dict[str, Any]: # TODO(kacpermalachowski): Move it to common package def get_pubsub_message(): - """get_pubsub_message unpacks pubsub message and does basic type checks""" + """Parses the Pub/Sub message from the request.""" envelope = request.get_json() if not envelope: # pylint: disable=broad-exception-raised - raise Exception("no Pub/Sub message received") + raise ValueError("No Pub/Sub message received") if not isinstance(envelope, dict) or "message" not in envelope: # pylint: disable=broad-exception-raised - raise Exception("invalid Pub/Sub message format") + raise ValueError("Invalid Pub/Sub message format") - pubsub_message = envelope["message"] - return pubsub_message + return envelope["message"] # TODO(kacpermalachowski): Move it to common package def prepare_error_response(err: str, log_fields: Dict[str, Any]) -> Response: - """prepare_error_response return error response with stacktrace""" + """Prepares an error response with logging.""" _, exc_value, _ = sys.exc_info() stacktrace = repr(traceback.format_exception(exc_value)) print( @@ -263,7 +262,7 @@ def prepare_error_response(err: str, log_fields: Dict[str, Any]) -> Response: def get_secret(secret_id: str): - """Get latest secret version from secret manager""" + """Retrieves the latest version of the secret from Secret Manager""" client = secretmanager.SecretManagerServiceClient() response = client.access_secret_version(name=f"{secret_id}/versions/latest") @@ -273,7 +272,7 @@ def get_secret(secret_id: str): def set_secret(secret_id: str, data: str): - """Sets new secret version in secret manager""" + """Adds a new version of the secret in Secret Manager.""" client = secretmanager.SecretManagerServiceClient() client.add_secret_version(parent=secret_id, payload={"data": data.encode()}) From cb6eed1835e61f0fb86355ca1d5f501d7018c8fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= <38684517+KacperMalachowski@users.noreply.github.com> Date: Fri, 13 Sep 2024 13:33:11 +0200 Subject: [PATCH 04/24] Apply suggestions from code review Co-authored-by: Patryk Dobrowolski --- .../rotate-signify-certificate/rotate_signify_certificate.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py index ca65277f7c78..83036f4d45fd 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -163,9 +163,8 @@ def prepare_new_secret( def extract_message_data(pubsub_message: Any) -> Any: """Extracts secret rotation message from the Pub/Sub message.""" - if pubsub_message["attributes"]["eventType"] != "SECRET_ROTATE": - # pylint: disable=broad-exception-raised - raise Exception("Unsupported event type") + if pubsub_message.get("attributes", {}).get("eventType") != "SECRET_ROTATE": + raise ValueError("Unsupported event type") data = base64.b64decode(pubsub_message["data"]) return json.loads(data) From 90429ffc3404e24946a285443deb408c7ca73918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Fri, 13 Sep 2024 13:42:59 +0200 Subject: [PATCH 05/24] Improve readibility of the code --- .../rotate_signify_certificate.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py index 83036f4d45fd..1dbb08ce2f84 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -30,7 +30,6 @@ def __str__(self): return json.dumps(self) -# pylint: disable-msg=too-many-locals @app.route("/", methods=["POST"]) def rotate_signify_secret() -> Response: """HTTP webhook handler for rotating Signify secrets.""" @@ -47,14 +46,9 @@ def rotate_signify_secret() -> Response: secret_data = get_secret(secret_rotate_msg["name"]) - token_url = secret_data["tokenURL"] - client_id = secret_data["clientID"] - cert_service_url = secret_data["certServiceURL"] old_cert_data = base64.b64decode(secret_data["certData"]) old_pk_data = base64.b64decode(secret_data["privateKeyData"]) - old_cert = x509.load_pem_x509_certificate(old_cert_data) - if "password" in secret_data and secret_data["password"] != "": old_pk_data = decrypt_private_key( old_pk_data, secret_data["password"].encode() @@ -62,20 +56,14 @@ def rotate_signify_secret() -> Response: new_private_key = rsa.generate_private_key(public_exponent=65537, key_size=4096) - csr = ( - x509.CertificateSigningRequestBuilder() - .subject_name(old_cert.subject) - .sign(new_private_key, hashes.SHA256()) - ) - access_token = fetch_access_token( - old_cert_data, old_pk_data, token_url, client_id + old_cert_data, old_pk_data, secret_data["tokenURL"], secret_data["clientID"] ) created_at = datetime.datetime.now().strftime("%d-%m-%Y %H:%M:%S") new_certs: List[x509.Certificate] = fetch_new_certificate( - csr, access_token, cert_service_url + old_cert_data, new_private_key, access_token, secret_data["certServiceURL"] ) new_secret_data = prepare_new_secret( @@ -95,15 +83,25 @@ def rotate_signify_secret() -> Response: ) return "Certificate rotated successfully" - # pylint: disable=broad-exception-caught - except Exception as exc: + except ValueError as exc: return prepare_error_response(exc, log_fields) def fetch_new_certificate( - csr: x509.CertificateSigningRequest, access_token: str, certificate_service_url: str + cert_data: bytes, + private_key: rsa.RSAPrivateKey, + access_token: str, + certificate_service_url: str, ): """Fetch new certificates from given certificate service""" + old_cert = x509.load_pem_x509_certificate(cert_data) + + csr = ( + x509.CertificateSigningRequestBuilder() + .subject_name(old_cert.subject) + .sign(private_key, hashes.SHA256()) + ) + crt_create_payload = json.dumps( { "csr": { From 13ac3ed6bf176e90658680aabc9a1ace72ffd734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Fri, 13 Sep 2024 13:45:27 +0200 Subject: [PATCH 06/24] Handle empty project id --- .../rotate_signify_certificate.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py index 1dbb08ce2f84..d77f538f5c9d 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -17,9 +17,9 @@ from cryptography.hazmat.primitives.asymmetric import rsa app = Flask(__name__) -project_id: str = os.getenv("PROJECT_ID", "") -component_name: str = os.getenv("COMPONENT_NAME", "") -application_name: str = os.getenv("APPLICATION_NAME", "") +project_id: str = os.getenv("PROJECT_ID") +component_name: str = os.getenv("COMPONENT_NAME", "signify-certificate-rotator") +application_name: str = os.getenv("APPLICATION_NAME", "secret-rotator") # TODO(kacpermalachowski): Move it to common package @@ -37,6 +37,9 @@ def rotate_signify_secret() -> Response: log_fields["labels"]["io.kyma.app"] = "signify-certificate-rotate" try: + if project_id is None: + raise ValueError("Unknown project id") + pubsub_message = get_pubsub_message() secret_rotate_msg = extract_message_data(pubsub_message) From 0638fb001b040b08e3b19d27b408ab098f74d4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= <38684517+KacperMalachowski@users.noreply.github.com> Date: Fri, 13 Sep 2024 13:57:48 +0200 Subject: [PATCH 07/24] Update cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py Co-authored-by: Patryk Dobrowolski --- .../rotate-signify-certificate/rotate_signify_certificate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py index d77f538f5c9d..55213ba7c027 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py @@ -120,7 +120,7 @@ def fetch_new_certificate( headers={ "Authorization": f"Bearer {access_token}", "Content-Type": "application/json", - "Accept": "applicaiton/json", + "Accept": "application/json", }, data=crt_create_payload, timeout=10, From 49c85e60e43b7a695e80a445d262b82b7458f355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Mon, 16 Sep 2024 07:24:53 +0200 Subject: [PATCH 08/24] Add unittests for newly created functions --- .../test_rotate_signify_certificate.py | 316 +++++++++++++++++- 1 file changed, 315 insertions(+), 1 deletion(-) diff --git a/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py b/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py index 4902a449362e..8f6aa7b1b517 100644 --- a/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py +++ b/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py @@ -1,13 +1,28 @@ """Module containing tests for signify secret rotator""" import base64 +from datetime import datetime, timedelta import json from typing import Any, Dict import unittest +from unittest import mock from unittest.mock import MagicMock, Mock, patch from flask.testing import FlaskClient +from cryptography.hazmat.primitives import serialization, hashes +from cryptography import x509 +from cryptography.x509.oid import NameOID +from cryptography.hazmat.primitives.asymmetric import rsa import test_fixtures -from rotate_signify_certificate import app, get_pubsub_message, prepare_log_fields +from rotate_signify_certificate import ( + app, + decrypt_private_key, + extract_message_data, + fetch_access_token, + fetch_new_certificate, + get_pubsub_message, + prepare_log_fields, + prepare_new_secret, +) project_id: str = "test-project" component_name: str = "test_component" @@ -50,6 +65,7 @@ def test_get_pubsub_message_invalid_format(self): get_pubsub_message() +@patch("rotate_signify_certificate.project_id", project_id) class TestRotateSignifySecret(unittest.TestCase): """Tests for rotating signify secret""" @@ -96,5 +112,303 @@ def test_rotate_signify_secret_success( mock_set_secret.assert_called_once() +class TestFetchNewCertificate(unittest.TestCase): + """Test cases for fetch_new_certificate function""" + + def setUp(self): + """Set up test data""" + self.cert_data = base64.b64decode(test_fixtures.mocked_secret_data["certData"]) + self.private_key = rsa.generate_private_key( + public_exponent=65537, + key_size=2048, + ) + self.access_token = "test_token" + self.certificate_service_url = "http://example.com/certificates" + + @patch("requests.post") + def test_fetch_new_certificate(self, mock_post): + """Test fetch_new_certificate success scenario""" + # Create a mock response + mock_response = MagicMock() + mock_response.json.return_value = test_fixtures.mocked_cert_create_response + mock_post.return_value = mock_response + + # Call the function with the test data + certs = fetch_new_certificate( + self.cert_data, + self.private_key, + self.access_token, + self.certificate_service_url, + ) + + # Verify the mock post request + mock_post.assert_called_once_with( + self.certificate_service_url, + headers={ + "Authorization": f"Bearer {self.access_token}", + "Content-Type": "application/json", + "Accept": "application/json", + }, + data=json.dumps( + { + "csr": { + "value": x509.CertificateSigningRequestBuilder() + .subject_name( + x509.load_pem_x509_certificate(self.cert_data).subject + ) + .sign(self.private_key, hashes.SHA256()) + .public_bytes(serialization.Encoding.PEM) + .decode("utf-8") + }, + "validity": {"value": 7, "type": "DAYS"}, + "policy": "sap-cloud-platform-clients", + } + ), + timeout=10, + ) + + # Check that the expected number of certificates is returned + self.assertEqual(len(certs), 1) + + +class TestPrepareNewSecret(unittest.TestCase): + """Tests cases for prepare_new_secret function""" + + @staticmethod + def generate_fake_cert_and_key(): + """ + Generates a fake RSA private key and a self-signed X.509 certificate. + + Returns: + Tuple[x509.Certificate, rsa.RSAPrivateKey]: Tuple contains certificate and private key. + """ + # pylint: disable=invalid-name + ONE_DAY = timedelta(1, 0, 0) + + # Generate private key + key = rsa.generate_private_key( + public_exponent=65537, + key_size=2048, + ) + + # Generate certificates + subject = issuer = x509.Name( + [ + x509.NameAttribute(NameOID.COUNTRY_NAME, "US"), + x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, "California"), + x509.NameAttribute(NameOID.LOCALITY_NAME, "San Francisco"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "My Company"), + x509.NameAttribute(NameOID.COMMON_NAME, "mysite.com"), + ] + ) + cert = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(issuer) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(datetime.now()) + .not_valid_after(datetime.now() + ONE_DAY) + .sign(key, hashes.SHA256()) + ) + + return cert, key + + def setUp(self): + self.cert, self.key = self.generate_fake_cert_and_key() + self.certificates = [self.cert] + self.private_key = self.key + self.secret_data = { + "certServiceURL": "https://cert.example.com", + "tokenURL": "https://token.example.com", + "clientID": "example-client-id", + } + self.created_at = "2023-01-01T00:00:00Z" + + def test_prepare_new_secret_structure(self): + """Test if the output structure of `prepare_new_secret` matches the expected format""" + result = prepare_new_secret( + self.certificates, self.private_key, self.secret_data, self.created_at + ) + + self.assertIn("certData", result) + self.assertIn("privateKeyData", result) + self.assertIn("createdAt", result) + self.assertIn("certServiceURL", result) + self.assertIn("tokenURL", result) + self.assertIn("clientID", result) + self.assertIn("password", result) + + self.assertEqual(result["createdAt"], self.created_at) + self.assertEqual(result["certServiceURL"], self.secret_data["certServiceURL"]) + self.assertEqual(result["tokenURL"], self.secret_data["tokenURL"]) + self.assertEqual(result["clientID"], self.secret_data["clientID"]) + self.assertEqual(result["password"], "") + + def test_prepare_new_secret_cert_data(self): + """Test if the certificate data in the output is correctly formatted and base64 encoded""" + result = prepare_new_secret( + self.certificates, self.private_key, self.secret_data, self.created_at + ) + encoded_cert_data = result["certData"] + cert_data_decoded = base64.b64decode(encoded_cert_data).decode() + + cert_subject = f"subject={self.cert.subject.rfc4514_string()}\n" + cert_issuer = f"issuer={self.cert.issuer.rfc4514_string()}\n" + cert_body = f"{self.cert.public_bytes(serialization.Encoding.PEM).decode()}\n" + + self.assertIn(cert_subject, cert_data_decoded) + self.assertIn(cert_issuer, cert_data_decoded) + self.assertIn(cert_body, cert_data_decoded) + + def test_prepare_new_secret_private_key_data(self): + """Test if the private key data in the output is correctly formatted and base64 encoded""" + result = prepare_new_secret( + self.certificates, self.private_key, self.secret_data, self.created_at + ) + encoded_private_key_data = result["privateKeyData"] + private_key_data_decoded = base64.b64decode(encoded_private_key_data) + + self.assertEqual( + private_key_data_decoded, + self.private_key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ), + ) + + +class TestExtractMessageData(unittest.TestCase): + """Unit tests for the extract_message_data function.""" + + def test_valid_message(self): + """Test a valid Pub/Sub message with correct event type and encoded data.""" + data = {"key": "value"} + encoded_data = base64.b64encode(json.dumps(data).encode()).decode() + pubsub_message = { + "data": encoded_data, + "attributes": {"eventType": "SECRET_ROTATE"}, + } + + result = extract_message_data(pubsub_message) + self.assertEqual(result, data) + + def test_unsupported_event_type(self): + """Test with an unsupported event type to ensure ValueError is raised.""" + pubsub_message = { + "data": base64.b64encode(json.dumps({"key": "value"}).encode()).decode(), + "attributes": {"eventType": "UNSUPPORTED_EVENT"}, + } + + with self.assertRaises(ValueError) as context: + extract_message_data(pubsub_message) + + self.assertEqual(str(context.exception), "Unsupported event type") + + def test_no_attributes(self): + """Test with no attributes to ensure ValueError is raised.""" + pubsub_message = { + "data": base64.b64encode(json.dumps({"key": "value"}).encode()).decode() + } + + with self.assertRaises(ValueError) as context: + extract_message_data(pubsub_message) + + self.assertEqual(str(context.exception), "Unsupported event type") + + def test_invalid_base64_data(self): + """Test with invalid base64-encoded data to ensure an exception is raised.""" + pubsub_message = { + "data": "invalid_base64_data", + "attributes": {"eventType": "SECRET_ROTATE"}, + } + + with self.assertRaises(Exception): + extract_message_data(pubsub_message) + + +class TestDecryptPrivateKey(unittest.TestCase): + """Unit tests for decrypt_private_key function.""" + + def setUp(self): + """Set up test data for the tests.""" + # Generate private key + self.private_key = rsa.generate_private_key( + public_exponent=65537, + key_size=2048, + ) + + self.password = b"password" + + # Encrypt private key with the password + self.private_key_data = self.private_key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=serialization.BestAvailableEncryption(self.password), + ) + + def test_decrypt_private_key(self): + """Test the decryption of an encrypted private key.""" + # Decrypt the private key + decrypted_private_key = decrypt_private_key( + self.private_key_data, self.password + ) + + # Convert the decrypted private key back to an object for validation + decrypted_private_key_obj = serialization.load_pem_private_key( + decrypted_private_key, password=None + ) + + # Serialize the private key object with no encryption to compare + expected_private_key_data = decrypted_private_key_obj.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ) + + self.assertEqual(decrypted_private_key, expected_private_key_data) + self.assertTrue( + decrypted_private_key.startswith(b"-----BEGIN PRIVATE KEY-----") + ) + self.assertNotIn(b"ENCRYPTED", decrypted_private_key) + + +class TestFetchAccessToken(unittest.TestCase): + """Test case for the fetch_access_token function.""" + + @patch("requests.post") + def test_fetch_access_token(self, mock_post): + """Test the fetch_access_token function.""" + certificate = b"dummy_certificate" + private_key = b"dummy_private_key" + token_url = "https://dummy_token_url" + client_id = "dummy_client_id" + + expected_token = "dummy_access_token" + + # Setup the mock response + mock_response = unittest.mock.Mock() + mock_response.json.return_value = {"access_token": expected_token} + mock_post.return_value = mock_response + + # Call the function + result = fetch_access_token(certificate, private_key, token_url, client_id) + + # Assert the result + self.assertEqual(result, expected_token) + + # Assert that post was called with the expected parameters + self.assertTrue(mock_post.called) + self.assertEqual( + mock_post.call_args[1]["cert"], (mock.ANY, mock.ANY) + ) # file paths are temporary + self.assertEqual( + mock_post.call_args[1]["data"]["grant_type"], "client_credentials" + ) + self.assertEqual(mock_post.call_args[1]["data"]["client_id"], client_id) + self.assertEqual(mock_post.call_args[1]["timeout"], 30) + + if __name__ == "__main__": unittest.main() From 0ad29511d1e8360f602a63d2a855d02dce91ae82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Mon, 16 Sep 2024 15:31:34 +0200 Subject: [PATCH 09/24] Make it better --- .gitignore | 5 ++- .../rotate-signify-certificate/Dockerfile | 14 -------- .../requirements.txt | 5 --- cmd/cloud-run/signifysecretrotator/Dockerfile | 13 +++++++ .../signifysecretrotator.py} | 22 +++++------- .../test_fixtures.py | 0 .../test_rotate_signify_certificate.py | 0 .../slack-message-sender/__init__.py | 0 pkg/pylogger/__init__.py | 0 pkg/secretmanager/__init__.py | 0 pkg/secretmanager/client.py | 17 ++++++++++ pkg/test_infra.egg-info/PKG-INFO | 16 +++++++++ pkg/test_infra.egg-info/SOURCES.txt | 18 ++++++++++ pkg/test_infra.egg-info/dependency_links.txt | 1 + pkg/test_infra.egg-info/entry_points.txt | 2 ++ pkg/test_infra.egg-info/requires.txt | 12 +++++++ pkg/test_infra.egg-info/top_level.txt | 4 +++ pyproject.toml | 34 +++++++++++++++++++ 18 files changed, 129 insertions(+), 34 deletions(-) delete mode 100644 cmd/cloud-run/rotate-signify-certificate/Dockerfile delete mode 100644 cmd/cloud-run/rotate-signify-certificate/requirements.txt create mode 100644 cmd/cloud-run/signifysecretrotator/Dockerfile rename cmd/cloud-run/{rotate-signify-certificate/rotate_signify_certificate.py => signifysecretrotator/signifysecretrotator.py} (93%) rename cmd/cloud-run/{rotate-signify-certificate => signifysecretrotator}/test_fixtures.py (100%) rename cmd/cloud-run/{rotate-signify-certificate => signifysecretrotator}/test_rotate_signify_certificate.py (100%) create mode 100644 cmd/cloud-run/slack-message-sender/__init__.py create mode 100644 pkg/pylogger/__init__.py create mode 100644 pkg/secretmanager/__init__.py create mode 100644 pkg/secretmanager/client.py create mode 100644 pkg/test_infra.egg-info/PKG-INFO create mode 100644 pkg/test_infra.egg-info/SOURCES.txt create mode 100644 pkg/test_infra.egg-info/dependency_links.txt create mode 100644 pkg/test_infra.egg-info/entry_points.txt create mode 100644 pkg/test_infra.egg-info/requires.txt create mode 100644 pkg/test_infra.egg-info/top_level.txt create mode 100644 pyproject.toml diff --git a/.gitignore b/.gitignore index e37dca995ec5..5f16074a854c 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,7 @@ __debug_bin .terraform # Ignore generated credentials from google-github-actions/auth -gha-creds-*.json \ No newline at end of file +gha-creds-*.json + +# Ignore egg info from local package isntallation +*.egg-info/* \ No newline at end of file diff --git a/cmd/cloud-run/rotate-signify-certificate/Dockerfile b/cmd/cloud-run/rotate-signify-certificate/Dockerfile deleted file mode 100644 index 0a15710d2f51..000000000000 --- a/cmd/cloud-run/rotate-signify-certificate/Dockerfile +++ /dev/null @@ -1,14 +0,0 @@ -FROM python:3.12.5-alpine3.20 - -# Allow statements and log messages to immediately appear in the Knative logs -ENV PYTHONUNBUFFERED True - -WORKDIR /app - -COPY ./cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py . -COPY ./cmd/cloud-run/rotate-signify-certificate/requirements.txt . - -RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ - apk add --no-cache ca-certificates - -CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "rotate-signify-certificate:app"] diff --git a/cmd/cloud-run/rotate-signify-certificate/requirements.txt b/cmd/cloud-run/rotate-signify-certificate/requirements.txt deleted file mode 100644 index 0aff4ef06eb4..000000000000 --- a/cmd/cloud-run/rotate-signify-certificate/requirements.txt +++ /dev/null @@ -1,5 +0,0 @@ - -flask==2.3.2 -google-cloud-secret-manager==2.20.2 -cloudevents==1.9.0 -gunicorn==22.0.0 \ No newline at end of file diff --git a/cmd/cloud-run/signifysecretrotator/Dockerfile b/cmd/cloud-run/signifysecretrotator/Dockerfile new file mode 100644 index 000000000000..5e06f329ba4e --- /dev/null +++ b/cmd/cloud-run/signifysecretrotator/Dockerfile @@ -0,0 +1,13 @@ +FROM python:3.12.5-alpine3.20 + +# Allow statements and log messages to immediately appear in the Knative logs +ENV PYTHONUNBUFFERED True + +WORKDIR /app + +COPY . . + +RUN pip install --no-cache-dir --upgrade '.[signify-secret-rotator]' && \ + apk add --no-cache ca-certificates + +CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "signify-secret-rotator"] diff --git a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py similarity index 93% rename from cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py rename to cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index 55213ba7c027..a87a8cd5029e 100644 --- a/cmd/cloud-run/rotate-signify-certificate/rotate_signify_certificate.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -9,12 +9,12 @@ import traceback from typing import Any, Dict, List import requests -from google.cloud import secretmanager from flask import Flask, Response, request, make_response from cryptography import x509 from cryptography.hazmat.primitives import serialization, hashes from cryptography.hazmat.primitives.serialization import pkcs7, Encoding, PrivateFormat from cryptography.hazmat.primitives.asymmetric import rsa +from secretmanager import client app = Flask(__name__) project_id: str = os.getenv("PROJECT_ID") @@ -37,6 +37,8 @@ def rotate_signify_secret() -> Response: log_fields["labels"]["io.kyma.app"] = "signify-certificate-rotate" try: + sm_client = client.SecretManagerClient() + if project_id is None: raise ValueError("Unknown project id") @@ -47,7 +49,7 @@ def rotate_signify_secret() -> Response: if secret_rotate_msg["labels"]["type"] != "signify": return prepare_error_response("Unsupported resource type", log_fields) - secret_data = get_secret(secret_rotate_msg["name"]) + secret_data = sm_client.get_secret(secret_rotate_msg["name"]) old_cert_data = base64.b64decode(secret_data["certData"]) old_pk_data = base64.b64decode(secret_data["privateKeyData"]) @@ -261,18 +263,10 @@ def prepare_error_response(err: str, log_fields: Dict[str, Any]) -> Response: return resp -def get_secret(secret_id: str): - """Retrieves the latest version of the secret from Secret Manager""" - client = secretmanager.SecretManagerServiceClient() - - response = client.access_secret_version(name=f"{secret_id}/versions/latest") - secret_value = response.payload.data.decode("UTF-8") - - return json.loads(secret_value) - - def set_secret(secret_id: str, data: str): """Adds a new version of the secret in Secret Manager.""" - client = secretmanager.SecretManagerServiceClient() + pass - client.add_secret_version(parent=secret_id, payload={"data": data.encode()}) +def setup_app(): + print("test") + pass \ No newline at end of file diff --git a/cmd/cloud-run/rotate-signify-certificate/test_fixtures.py b/cmd/cloud-run/signifysecretrotator/test_fixtures.py similarity index 100% rename from cmd/cloud-run/rotate-signify-certificate/test_fixtures.py rename to cmd/cloud-run/signifysecretrotator/test_fixtures.py diff --git a/cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py b/cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py similarity index 100% rename from cmd/cloud-run/rotate-signify-certificate/test_rotate_signify_certificate.py rename to cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py diff --git a/cmd/cloud-run/slack-message-sender/__init__.py b/cmd/cloud-run/slack-message-sender/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/pkg/pylogger/__init__.py b/pkg/pylogger/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/pkg/secretmanager/__init__.py b/pkg/secretmanager/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/pkg/secretmanager/client.py b/pkg/secretmanager/client.py new file mode 100644 index 000000000000..91dc0f11e2fc --- /dev/null +++ b/pkg/secretmanager/client.py @@ -0,0 +1,17 @@ +"""Custom wrapper for Google's Secret Manager Service client""" + +from google.cloud.secretmanager import SecretManagerServiceClient + + +class SecretManagerClient: + """ + Wraps the google's secret manager client implementation. + Provides more efficient way to retrieve and set secrets in kyma-project secret manager + """ + + def __init__(self, client=SecretManagerServiceClient()) -> None: + self.client: SecretManagerClient = client + + def get_secret(self, secret_id: str, secret_version: str = "latest"): + """Gets given secret in given version""" + return "Test" diff --git a/pkg/test_infra.egg-info/PKG-INFO b/pkg/test_infra.egg-info/PKG-INFO new file mode 100644 index 000000000000..a231862dfa95 --- /dev/null +++ b/pkg/test_infra.egg-info/PKG-INFO @@ -0,0 +1,16 @@ +Metadata-Version: 2.1 +Name: test_infra +Version: 0.1.0 +Requires-Python: >=3.11 +License-File: LICENSE +License-File: NOTICE.md +Requires-Dist: flask==2.3.2 +Requires-Dist: cloudevents==1.9.0 +Requires-Dist: gunicorn==22.0.0 +Provides-Extra: signify-secret-rotator +Requires-Dist: google-cloud-secret-manager==2.20.2; extra == "signify-secret-rotator" +Provides-Extra: slack-message-sender +Requires-Dist: slack_bolt==1.18.0; extra == "slack-message-sender" +Requires-Dist: slack_sdk==3.21.3; extra == "slack-message-sender" +Requires-Dist: pygithub==2.1.1; extra == "slack-message-sender" +Requires-Dist: pyyaml==6.0.1; extra == "slack-message-sender" diff --git a/pkg/test_infra.egg-info/SOURCES.txt b/pkg/test_infra.egg-info/SOURCES.txt new file mode 100644 index 000000000000..73a77df79032 --- /dev/null +++ b/pkg/test_infra.egg-info/SOURCES.txt @@ -0,0 +1,18 @@ +LICENSE +NOTICE.md +README.md +pyproject.toml +cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +cmd/cloud-run/signifysecretrotator/test_fixtures.py +cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py +cmd/cloud-run/slack-message-sender/__init__.py +cmd/cloud-run/slack-message-sender/slack-message-sender.py +pkg/pylogger/__init__.py +pkg/secretmanager/__init__.py +pkg/secretmanager/client.py +pkg/test_infra.egg-info/PKG-INFO +pkg/test_infra.egg-info/SOURCES.txt +pkg/test_infra.egg-info/dependency_links.txt +pkg/test_infra.egg-info/entry_points.txt +pkg/test_infra.egg-info/requires.txt +pkg/test_infra.egg-info/top_level.txt \ No newline at end of file diff --git a/pkg/test_infra.egg-info/dependency_links.txt b/pkg/test_infra.egg-info/dependency_links.txt new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/pkg/test_infra.egg-info/dependency_links.txt @@ -0,0 +1 @@ + diff --git a/pkg/test_infra.egg-info/entry_points.txt b/pkg/test_infra.egg-info/entry_points.txt new file mode 100644 index 000000000000..9ad64d1ec1a3 --- /dev/null +++ b/pkg/test_infra.egg-info/entry_points.txt @@ -0,0 +1,2 @@ +[console_scripts] +signify-secret-rotator = signifysecretrotator.signifysecretrotator:setup_app diff --git a/pkg/test_infra.egg-info/requires.txt b/pkg/test_infra.egg-info/requires.txt new file mode 100644 index 000000000000..26413b78077d --- /dev/null +++ b/pkg/test_infra.egg-info/requires.txt @@ -0,0 +1,12 @@ +flask==2.3.2 +cloudevents==1.9.0 +gunicorn==22.0.0 + +[signify-secret-rotator] +google-cloud-secret-manager==2.20.2 + +[slack-message-sender] +slack_bolt==1.18.0 +slack_sdk==3.21.3 +pygithub==2.1.1 +pyyaml==6.0.1 diff --git a/pkg/test_infra.egg-info/top_level.txt b/pkg/test_infra.egg-info/top_level.txt new file mode 100644 index 000000000000..b6f5f7fb95cb --- /dev/null +++ b/pkg/test_infra.egg-info/top_level.txt @@ -0,0 +1,4 @@ +pylogger +secretmanager +signifysecretrotator +slackmessagesender diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 000000000000..e04f44878511 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,34 @@ +[build-system] +requires = ["setuptools"] +build-backend = "setuptools.build_meta" + +[project] +name = "test_infra" +version = "0.1.0" +requires-python = ">=3.11" +dependencies = [ + "flask == 2.3.2", + "cloudevents == 1.9.0", + "gunicorn == 22.0.0" +] + +[project.optional-dependencies] +signify-secret-rotator = ["google-cloud-secret-manager == 2.20.2"] +slack-message-sender = [ + "slack_bolt == 1.18.0", + "slack_sdk == 3.21.3", + "pygithub == 2.1.1", + "pyyaml == 6.0.1" +] + +[project.scripts] +signify-secret-rotator = "signifysecretrotator.signifysecretrotator:setup_app" + +[tool.setuptools] +packages = [ + "slackmessagesender", + "signifysecretrotator", + "secretmanager", + "pylogger" +] +package-dir = {"slackmessagesender" = "cmd/cloud-run/slack-message-sender","signifysecretrotator" = "cmd/cloud-run/signifysecretrotator","" = "pkg"} \ No newline at end of file From f9c5504c57d515aafc52fed232576752f4a2a3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Tue, 17 Sep 2024 13:56:11 +0200 Subject: [PATCH 10/24] Move packages as subpackages --- .gitignore | 3 - cmd/cloud-run/signifysecretrotator/Dockerfile | 5 +- .../pylogger/__init__.py | 0 .../signifysecretrotator/requirements.txt | 5 + .../secretmanager/__init__.py | 0 .../secretmanager/client.py | 52 +++ .../secretmanager/test_client.py | 83 ++++ .../signifysecretrotator.py | 15 +- .../test_rotate_signify_certificate.py | 414 ------------------ pkg/secretmanager/client.py | 17 - pkg/test_infra.egg-info/PKG-INFO | 16 - pkg/test_infra.egg-info/SOURCES.txt | 18 - pkg/test_infra.egg-info/dependency_links.txt | 1 - pkg/test_infra.egg-info/entry_points.txt | 2 - pkg/test_infra.egg-info/requires.txt | 12 - pkg/test_infra.egg-info/top_level.txt | 4 - pyproject.toml | 34 -- 17 files changed, 148 insertions(+), 533 deletions(-) rename {pkg => cmd/cloud-run/signifysecretrotator}/pylogger/__init__.py (100%) create mode 100644 cmd/cloud-run/signifysecretrotator/requirements.txt rename {pkg => cmd/cloud-run/signifysecretrotator}/secretmanager/__init__.py (100%) create mode 100644 cmd/cloud-run/signifysecretrotator/secretmanager/client.py create mode 100644 cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py delete mode 100644 cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py delete mode 100644 pkg/secretmanager/client.py delete mode 100644 pkg/test_infra.egg-info/PKG-INFO delete mode 100644 pkg/test_infra.egg-info/SOURCES.txt delete mode 100644 pkg/test_infra.egg-info/dependency_links.txt delete mode 100644 pkg/test_infra.egg-info/entry_points.txt delete mode 100644 pkg/test_infra.egg-info/requires.txt delete mode 100644 pkg/test_infra.egg-info/top_level.txt delete mode 100644 pyproject.toml diff --git a/.gitignore b/.gitignore index 5f16074a854c..c942f9e13fe7 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,3 @@ __debug_bin # Ignore generated credentials from google-github-actions/auth gha-creds-*.json - -# Ignore egg info from local package isntallation -*.egg-info/* \ No newline at end of file diff --git a/cmd/cloud-run/signifysecretrotator/Dockerfile b/cmd/cloud-run/signifysecretrotator/Dockerfile index 5e06f329ba4e..79a858951d45 100644 --- a/cmd/cloud-run/signifysecretrotator/Dockerfile +++ b/cmd/cloud-run/signifysecretrotator/Dockerfile @@ -5,9 +5,10 @@ ENV PYTHONUNBUFFERED True WORKDIR /app -COPY . . +COPY ./cmd/cloud-run/slack-message-sender/slack-message-sender.py . +COPY ./cmd/cloud-run/slack-message-sender/requirements.txt . -RUN pip install --no-cache-dir --upgrade '.[signify-secret-rotator]' && \ +RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ apk add --no-cache ca-certificates CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "signify-secret-rotator"] diff --git a/pkg/pylogger/__init__.py b/cmd/cloud-run/signifysecretrotator/pylogger/__init__.py similarity index 100% rename from pkg/pylogger/__init__.py rename to cmd/cloud-run/signifysecretrotator/pylogger/__init__.py diff --git a/cmd/cloud-run/signifysecretrotator/requirements.txt b/cmd/cloud-run/signifysecretrotator/requirements.txt new file mode 100644 index 000000000000..1821d3243438 --- /dev/null +++ b/cmd/cloud-run/signifysecretrotator/requirements.txt @@ -0,0 +1,5 @@ +flask==2.3.2 +cloudevents==1.9.0 +gunicorn==22.0.0 +google-cloud-secret-manager==2.20.2 +cryptography==43.0.1 \ No newline at end of file diff --git a/pkg/secretmanager/__init__.py b/cmd/cloud-run/signifysecretrotator/secretmanager/__init__.py similarity index 100% rename from pkg/secretmanager/__init__.py rename to cmd/cloud-run/signifysecretrotator/secretmanager/__init__.py diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py new file mode 100644 index 000000000000..4d6d3cb5a815 --- /dev/null +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py @@ -0,0 +1,52 @@ +"""Custom wrapper for Google's Secret Manager Service client""" + +import json +from typing import Any, Dict +from google.cloud import secretmanager + + +class SecretManagerClient: + """ + Wraps the google's secret manager client implementation. + Provides more efficient way to retrieve and set secrets in kyma-project secret manager + """ + + def __init__(self, client=secretmanager.SecretManagerServiceClient()) -> None: + self.client: secretmanager.SecretManagerServiceClient = client + + def get_secret( + self, secret_id: str, secret_version: str = "latest", is_json: bool = True + ) -> Dict[str, Any]: + """Fetches value of secret with given version + + Args: + secret_id (str): Secret id in format "projects//secrets/" + secret_version (str, optional): Version of the secret. Defaults to "latest". + is_json (bool): Secret is json struct. Defaults to True + + Returns: + Dict[str, Any]: JSON decoded or str depending on is_json value + """ + + secret_name = f"{secret_id}/versions/{secret_version}" + + response: secretmanager.AccessSecretVersionResponse = ( + self.client.access_secret_version(secret_name=secret_name) + ) + secret_value = response.payload.data.decode() + + if is_json: + return json.loads(secret_value) + + return secret_value + + def set_secret(self, secret_id: str, data: str): + """Adds new secret version with given data + + Args: + secret_id (str): Secret id in format "projects//secrets/" + data (str): Value that should be set as new secret version + """ + payload = {"data": data.encode()} + + self.client.add_secret_version(parent=secret_id, payload=payload) diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py new file mode 100644 index 000000000000..5b4c8dabef9d --- /dev/null +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py @@ -0,0 +1,83 @@ +"""Contains tests for secret manager client module""" + +import json +import unittest +from unittest.mock import MagicMock, patch +from google.cloud import secretmanager + +# pylint: disable=import-error +# False positive see: https://github.com/pylint-dev/pylint/issues/3984 +from client import SecretManagerClient + + +class TestSecretManagerClient(unittest.TestCase): + """Tests for secret manager client""" + + def setUp(self): + access_secret_patcher = patch.object( + secretmanager.SecretManagerServiceClient, "access_secret_version" + ) + add_secret_version_patcher = patch.object( + secretmanager.SecretManagerServiceClient, "add_secret_version" + ) + + self.mock_access_secret_version = access_secret_patcher.start() + self.mock_add_secret_version = add_secret_version_patcher.start() + + self.addCleanup(access_secret_patcher.stop) + self.addCleanup(add_secret_version_patcher.stop) + + self.client = SecretManagerClient() + + def test_get_secret_json(self): + """Tests fetching json secret data""" + # Arrange + mock_response = MagicMock() + mock_response.payload.data.decode.return_value = json.dumps({"key": "value"}) + self.mock_access_secret_version.return_value = mock_response + + # Act + secret = self.client.get_secret("projects/test-project/secrets/test-secret") + + # Assert + self.assertEqual(secret, {"key": "value"}) + self.mock_access_secret_version.assert_called_once_with( + secret_name="projects/test-project/secrets/test-secret/versions/latest" + ) + + def test_get_secret_plain_string(self): + """Tests fetching string secret data""" + # Arrange + mock_response = MagicMock() + mock_response.payload.data.decode.return_value = "some-secret-value" + self.mock_access_secret_version.return_value = mock_response + + # Act + secret = self.client.get_secret( + "projects/test-project/secrets/test-secret", is_json=False + ) + + # Assert + self.assertEqual(secret, "some-secret-value") + self.mock_access_secret_version.assert_called_once_with( + secret_name="projects/test-project/secrets/test-secret/versions/latest" + ) + + def test_set_secret(self): + """Tests setting a new secret version""" + # Arrange + secret_id = "projects/test-project/secrets/test-secret" + secret_data = "new-secret-value" + + # Act + self.client.set_secret(secret_id, secret_data) + + # Assert + payload = {"data": secret_data.encode()} + self.mock_add_secret_version.assert_called_once_with( + parent=secret_id, payload=payload + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index a87a8cd5029e..337c162734d9 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -75,10 +75,6 @@ def rotate_signify_secret() -> Response: new_certs, new_private_key, secret_data, created_at ) - set_secret( - secret_id=secret_rotate_msg["name"], data=json.dumps(new_secret_data) - ) - print( LogEntry( severity="INFO", @@ -191,7 +187,10 @@ def fetch_access_token( # Use temporary file for old cert and key because requests library needs file paths, # it's not a security concern because the code is running in known environment controlled by us # pylint: disable=line-too-long - with tempfile.NamedTemporaryFile() as old_cert_file, tempfile.NamedTemporaryFile() as old_key_file: + with ( + tempfile.NamedTemporaryFile() as old_cert_file, + tempfile.NamedTemporaryFile() as old_key_file, + ): old_cert_file.write(certificate) old_cert_file.flush() @@ -263,10 +262,6 @@ def prepare_error_response(err: str, log_fields: Dict[str, Any]) -> Response: return resp -def set_secret(secret_id: str, data: str): - """Adds a new version of the secret in Secret Manager.""" - pass - def setup_app(): print("test") - pass \ No newline at end of file + pass diff --git a/cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py b/cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py deleted file mode 100644 index 8f6aa7b1b517..000000000000 --- a/cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py +++ /dev/null @@ -1,414 +0,0 @@ -"""Module containing tests for signify secret rotator""" - -import base64 -from datetime import datetime, timedelta -import json -from typing import Any, Dict -import unittest -from unittest import mock -from unittest.mock import MagicMock, Mock, patch -from flask.testing import FlaskClient -from cryptography.hazmat.primitives import serialization, hashes -from cryptography import x509 -from cryptography.x509.oid import NameOID -from cryptography.hazmat.primitives.asymmetric import rsa -import test_fixtures -from rotate_signify_certificate import ( - app, - decrypt_private_key, - extract_message_data, - fetch_access_token, - fetch_new_certificate, - get_pubsub_message, - prepare_log_fields, - prepare_new_secret, -) - -project_id: str = "test-project" -component_name: str = "test_component" -application_name: str = "test_application" - - -class TestPubSubMessageHandler(unittest.TestCase): - """Test cases for fucntion to handle pub sub message""" - - def setUp(self) -> None: - self.app: FlaskClient = app.test_client() - self.app.testing = True - - @patch("rotate_signify_certificate.project_id", project_id) - @patch("rotate_signify_certificate.component_name", component_name) - @patch("rotate_signify_certificate.application_name", application_name) - def test_prepare_log_fields(self): - """Test that prepare log fields based on current env variables and cloud trace context""" - with app.test_request_context( - "/", headers={"X-Cloud-Trace-Context": "trace-id/other-info"} - ): - log_fields: Dict[str, Any] = prepare_log_fields() - self.assertIn("logging.googleapis.com/trace", log_fields) - self.assertEqual( - log_fields["logging.googleapis.com/trace"], - f"projects/{project_id}/traces/trace-id", - ) - self.assertEqual(log_fields["Component"], "signify-certificate-rotator") - - def test_get_pubsub_message_no_message(self): - """It should raise exception when no pubsub message is received""" - with app.test_request_context("/", json={}): - with self.assertRaises(Exception): - get_pubsub_message() - - def test_get_pubsub_message_invalid_format(self): - """It should raise exception when invalid pubsub message is received""" - with app.test_request_context("/", json={"invalid_key": "value"}): - with self.assertRaises(Exception): - get_pubsub_message() - - -@patch("rotate_signify_certificate.project_id", project_id) -class TestRotateSignifySecret(unittest.TestCase): - """Tests for rotating signify secret""" - - @classmethod - def setUpClass(cls) -> None: - cls.app: FlaskClient = app.test_client() - cls.app.testing = True - - def setUp(self): - """Setup for each test""" - self.pubsub_message = { - "message": { - "attributes": {"eventType": "SECRET_ROTATE"}, - "data": base64.b64encode( - json.dumps( - {"labels": {"type": "signify"}, "name": "test-secret"} - ).encode() - ).decode(), - } - } - - self.secret_data = test_fixtures.mocked_secret_data - self.mocked_token_response = test_fixtures.mocked_access_token_response - self.mocked_cert_create_response = test_fixtures.mocked_cert_create_response - - @patch("requests.post") - @patch("rotate_signify_certificate.set_secret") - @patch("rotate_signify_certificate.get_secret") - def test_rotate_signify_secret_success( - self, - mock_get_secret: Mock, - mock_set_secret: Mock, - mock_post: Mock, - ): - """Test success repsonse for valid pubsub""" - mock_get_secret.return_value = self.secret_data - mock_post.side_effect = [ - MagicMock(json=lambda: self.mocked_token_response), - MagicMock(json=lambda: self.mocked_cert_create_response), - ] - - response = self.app.post("/", json=self.pubsub_message) - self.assertEqual(response.status_code, 200) - mock_set_secret.assert_called_once() - - -class TestFetchNewCertificate(unittest.TestCase): - """Test cases for fetch_new_certificate function""" - - def setUp(self): - """Set up test data""" - self.cert_data = base64.b64decode(test_fixtures.mocked_secret_data["certData"]) - self.private_key = rsa.generate_private_key( - public_exponent=65537, - key_size=2048, - ) - self.access_token = "test_token" - self.certificate_service_url = "http://example.com/certificates" - - @patch("requests.post") - def test_fetch_new_certificate(self, mock_post): - """Test fetch_new_certificate success scenario""" - # Create a mock response - mock_response = MagicMock() - mock_response.json.return_value = test_fixtures.mocked_cert_create_response - mock_post.return_value = mock_response - - # Call the function with the test data - certs = fetch_new_certificate( - self.cert_data, - self.private_key, - self.access_token, - self.certificate_service_url, - ) - - # Verify the mock post request - mock_post.assert_called_once_with( - self.certificate_service_url, - headers={ - "Authorization": f"Bearer {self.access_token}", - "Content-Type": "application/json", - "Accept": "application/json", - }, - data=json.dumps( - { - "csr": { - "value": x509.CertificateSigningRequestBuilder() - .subject_name( - x509.load_pem_x509_certificate(self.cert_data).subject - ) - .sign(self.private_key, hashes.SHA256()) - .public_bytes(serialization.Encoding.PEM) - .decode("utf-8") - }, - "validity": {"value": 7, "type": "DAYS"}, - "policy": "sap-cloud-platform-clients", - } - ), - timeout=10, - ) - - # Check that the expected number of certificates is returned - self.assertEqual(len(certs), 1) - - -class TestPrepareNewSecret(unittest.TestCase): - """Tests cases for prepare_new_secret function""" - - @staticmethod - def generate_fake_cert_and_key(): - """ - Generates a fake RSA private key and a self-signed X.509 certificate. - - Returns: - Tuple[x509.Certificate, rsa.RSAPrivateKey]: Tuple contains certificate and private key. - """ - # pylint: disable=invalid-name - ONE_DAY = timedelta(1, 0, 0) - - # Generate private key - key = rsa.generate_private_key( - public_exponent=65537, - key_size=2048, - ) - - # Generate certificates - subject = issuer = x509.Name( - [ - x509.NameAttribute(NameOID.COUNTRY_NAME, "US"), - x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, "California"), - x509.NameAttribute(NameOID.LOCALITY_NAME, "San Francisco"), - x509.NameAttribute(NameOID.ORGANIZATION_NAME, "My Company"), - x509.NameAttribute(NameOID.COMMON_NAME, "mysite.com"), - ] - ) - cert = ( - x509.CertificateBuilder() - .subject_name(subject) - .issuer_name(issuer) - .public_key(key.public_key()) - .serial_number(x509.random_serial_number()) - .not_valid_before(datetime.now()) - .not_valid_after(datetime.now() + ONE_DAY) - .sign(key, hashes.SHA256()) - ) - - return cert, key - - def setUp(self): - self.cert, self.key = self.generate_fake_cert_and_key() - self.certificates = [self.cert] - self.private_key = self.key - self.secret_data = { - "certServiceURL": "https://cert.example.com", - "tokenURL": "https://token.example.com", - "clientID": "example-client-id", - } - self.created_at = "2023-01-01T00:00:00Z" - - def test_prepare_new_secret_structure(self): - """Test if the output structure of `prepare_new_secret` matches the expected format""" - result = prepare_new_secret( - self.certificates, self.private_key, self.secret_data, self.created_at - ) - - self.assertIn("certData", result) - self.assertIn("privateKeyData", result) - self.assertIn("createdAt", result) - self.assertIn("certServiceURL", result) - self.assertIn("tokenURL", result) - self.assertIn("clientID", result) - self.assertIn("password", result) - - self.assertEqual(result["createdAt"], self.created_at) - self.assertEqual(result["certServiceURL"], self.secret_data["certServiceURL"]) - self.assertEqual(result["tokenURL"], self.secret_data["tokenURL"]) - self.assertEqual(result["clientID"], self.secret_data["clientID"]) - self.assertEqual(result["password"], "") - - def test_prepare_new_secret_cert_data(self): - """Test if the certificate data in the output is correctly formatted and base64 encoded""" - result = prepare_new_secret( - self.certificates, self.private_key, self.secret_data, self.created_at - ) - encoded_cert_data = result["certData"] - cert_data_decoded = base64.b64decode(encoded_cert_data).decode() - - cert_subject = f"subject={self.cert.subject.rfc4514_string()}\n" - cert_issuer = f"issuer={self.cert.issuer.rfc4514_string()}\n" - cert_body = f"{self.cert.public_bytes(serialization.Encoding.PEM).decode()}\n" - - self.assertIn(cert_subject, cert_data_decoded) - self.assertIn(cert_issuer, cert_data_decoded) - self.assertIn(cert_body, cert_data_decoded) - - def test_prepare_new_secret_private_key_data(self): - """Test if the private key data in the output is correctly formatted and base64 encoded""" - result = prepare_new_secret( - self.certificates, self.private_key, self.secret_data, self.created_at - ) - encoded_private_key_data = result["privateKeyData"] - private_key_data_decoded = base64.b64decode(encoded_private_key_data) - - self.assertEqual( - private_key_data_decoded, - self.private_key.private_bytes( - serialization.Encoding.PEM, - serialization.PrivateFormat.PKCS8, - serialization.NoEncryption(), - ), - ) - - -class TestExtractMessageData(unittest.TestCase): - """Unit tests for the extract_message_data function.""" - - def test_valid_message(self): - """Test a valid Pub/Sub message with correct event type and encoded data.""" - data = {"key": "value"} - encoded_data = base64.b64encode(json.dumps(data).encode()).decode() - pubsub_message = { - "data": encoded_data, - "attributes": {"eventType": "SECRET_ROTATE"}, - } - - result = extract_message_data(pubsub_message) - self.assertEqual(result, data) - - def test_unsupported_event_type(self): - """Test with an unsupported event type to ensure ValueError is raised.""" - pubsub_message = { - "data": base64.b64encode(json.dumps({"key": "value"}).encode()).decode(), - "attributes": {"eventType": "UNSUPPORTED_EVENT"}, - } - - with self.assertRaises(ValueError) as context: - extract_message_data(pubsub_message) - - self.assertEqual(str(context.exception), "Unsupported event type") - - def test_no_attributes(self): - """Test with no attributes to ensure ValueError is raised.""" - pubsub_message = { - "data": base64.b64encode(json.dumps({"key": "value"}).encode()).decode() - } - - with self.assertRaises(ValueError) as context: - extract_message_data(pubsub_message) - - self.assertEqual(str(context.exception), "Unsupported event type") - - def test_invalid_base64_data(self): - """Test with invalid base64-encoded data to ensure an exception is raised.""" - pubsub_message = { - "data": "invalid_base64_data", - "attributes": {"eventType": "SECRET_ROTATE"}, - } - - with self.assertRaises(Exception): - extract_message_data(pubsub_message) - - -class TestDecryptPrivateKey(unittest.TestCase): - """Unit tests for decrypt_private_key function.""" - - def setUp(self): - """Set up test data for the tests.""" - # Generate private key - self.private_key = rsa.generate_private_key( - public_exponent=65537, - key_size=2048, - ) - - self.password = b"password" - - # Encrypt private key with the password - self.private_key_data = self.private_key.private_bytes( - encoding=serialization.Encoding.PEM, - format=serialization.PrivateFormat.TraditionalOpenSSL, - encryption_algorithm=serialization.BestAvailableEncryption(self.password), - ) - - def test_decrypt_private_key(self): - """Test the decryption of an encrypted private key.""" - # Decrypt the private key - decrypted_private_key = decrypt_private_key( - self.private_key_data, self.password - ) - - # Convert the decrypted private key back to an object for validation - decrypted_private_key_obj = serialization.load_pem_private_key( - decrypted_private_key, password=None - ) - - # Serialize the private key object with no encryption to compare - expected_private_key_data = decrypted_private_key_obj.private_bytes( - encoding=serialization.Encoding.PEM, - format=serialization.PrivateFormat.PKCS8, - encryption_algorithm=serialization.NoEncryption(), - ) - - self.assertEqual(decrypted_private_key, expected_private_key_data) - self.assertTrue( - decrypted_private_key.startswith(b"-----BEGIN PRIVATE KEY-----") - ) - self.assertNotIn(b"ENCRYPTED", decrypted_private_key) - - -class TestFetchAccessToken(unittest.TestCase): - """Test case for the fetch_access_token function.""" - - @patch("requests.post") - def test_fetch_access_token(self, mock_post): - """Test the fetch_access_token function.""" - certificate = b"dummy_certificate" - private_key = b"dummy_private_key" - token_url = "https://dummy_token_url" - client_id = "dummy_client_id" - - expected_token = "dummy_access_token" - - # Setup the mock response - mock_response = unittest.mock.Mock() - mock_response.json.return_value = {"access_token": expected_token} - mock_post.return_value = mock_response - - # Call the function - result = fetch_access_token(certificate, private_key, token_url, client_id) - - # Assert the result - self.assertEqual(result, expected_token) - - # Assert that post was called with the expected parameters - self.assertTrue(mock_post.called) - self.assertEqual( - mock_post.call_args[1]["cert"], (mock.ANY, mock.ANY) - ) # file paths are temporary - self.assertEqual( - mock_post.call_args[1]["data"]["grant_type"], "client_credentials" - ) - self.assertEqual(mock_post.call_args[1]["data"]["client_id"], client_id) - self.assertEqual(mock_post.call_args[1]["timeout"], 30) - - -if __name__ == "__main__": - unittest.main() diff --git a/pkg/secretmanager/client.py b/pkg/secretmanager/client.py deleted file mode 100644 index 91dc0f11e2fc..000000000000 --- a/pkg/secretmanager/client.py +++ /dev/null @@ -1,17 +0,0 @@ -"""Custom wrapper for Google's Secret Manager Service client""" - -from google.cloud.secretmanager import SecretManagerServiceClient - - -class SecretManagerClient: - """ - Wraps the google's secret manager client implementation. - Provides more efficient way to retrieve and set secrets in kyma-project secret manager - """ - - def __init__(self, client=SecretManagerServiceClient()) -> None: - self.client: SecretManagerClient = client - - def get_secret(self, secret_id: str, secret_version: str = "latest"): - """Gets given secret in given version""" - return "Test" diff --git a/pkg/test_infra.egg-info/PKG-INFO b/pkg/test_infra.egg-info/PKG-INFO deleted file mode 100644 index a231862dfa95..000000000000 --- a/pkg/test_infra.egg-info/PKG-INFO +++ /dev/null @@ -1,16 +0,0 @@ -Metadata-Version: 2.1 -Name: test_infra -Version: 0.1.0 -Requires-Python: >=3.11 -License-File: LICENSE -License-File: NOTICE.md -Requires-Dist: flask==2.3.2 -Requires-Dist: cloudevents==1.9.0 -Requires-Dist: gunicorn==22.0.0 -Provides-Extra: signify-secret-rotator -Requires-Dist: google-cloud-secret-manager==2.20.2; extra == "signify-secret-rotator" -Provides-Extra: slack-message-sender -Requires-Dist: slack_bolt==1.18.0; extra == "slack-message-sender" -Requires-Dist: slack_sdk==3.21.3; extra == "slack-message-sender" -Requires-Dist: pygithub==2.1.1; extra == "slack-message-sender" -Requires-Dist: pyyaml==6.0.1; extra == "slack-message-sender" diff --git a/pkg/test_infra.egg-info/SOURCES.txt b/pkg/test_infra.egg-info/SOURCES.txt deleted file mode 100644 index 73a77df79032..000000000000 --- a/pkg/test_infra.egg-info/SOURCES.txt +++ /dev/null @@ -1,18 +0,0 @@ -LICENSE -NOTICE.md -README.md -pyproject.toml -cmd/cloud-run/signifysecretrotator/signifysecretrotator.py -cmd/cloud-run/signifysecretrotator/test_fixtures.py -cmd/cloud-run/signifysecretrotator/test_rotate_signify_certificate.py -cmd/cloud-run/slack-message-sender/__init__.py -cmd/cloud-run/slack-message-sender/slack-message-sender.py -pkg/pylogger/__init__.py -pkg/secretmanager/__init__.py -pkg/secretmanager/client.py -pkg/test_infra.egg-info/PKG-INFO -pkg/test_infra.egg-info/SOURCES.txt -pkg/test_infra.egg-info/dependency_links.txt -pkg/test_infra.egg-info/entry_points.txt -pkg/test_infra.egg-info/requires.txt -pkg/test_infra.egg-info/top_level.txt \ No newline at end of file diff --git a/pkg/test_infra.egg-info/dependency_links.txt b/pkg/test_infra.egg-info/dependency_links.txt deleted file mode 100644 index 8b137891791f..000000000000 --- a/pkg/test_infra.egg-info/dependency_links.txt +++ /dev/null @@ -1 +0,0 @@ - diff --git a/pkg/test_infra.egg-info/entry_points.txt b/pkg/test_infra.egg-info/entry_points.txt deleted file mode 100644 index 9ad64d1ec1a3..000000000000 --- a/pkg/test_infra.egg-info/entry_points.txt +++ /dev/null @@ -1,2 +0,0 @@ -[console_scripts] -signify-secret-rotator = signifysecretrotator.signifysecretrotator:setup_app diff --git a/pkg/test_infra.egg-info/requires.txt b/pkg/test_infra.egg-info/requires.txt deleted file mode 100644 index 26413b78077d..000000000000 --- a/pkg/test_infra.egg-info/requires.txt +++ /dev/null @@ -1,12 +0,0 @@ -flask==2.3.2 -cloudevents==1.9.0 -gunicorn==22.0.0 - -[signify-secret-rotator] -google-cloud-secret-manager==2.20.2 - -[slack-message-sender] -slack_bolt==1.18.0 -slack_sdk==3.21.3 -pygithub==2.1.1 -pyyaml==6.0.1 diff --git a/pkg/test_infra.egg-info/top_level.txt b/pkg/test_infra.egg-info/top_level.txt deleted file mode 100644 index b6f5f7fb95cb..000000000000 --- a/pkg/test_infra.egg-info/top_level.txt +++ /dev/null @@ -1,4 +0,0 @@ -pylogger -secretmanager -signifysecretrotator -slackmessagesender diff --git a/pyproject.toml b/pyproject.toml deleted file mode 100644 index e04f44878511..000000000000 --- a/pyproject.toml +++ /dev/null @@ -1,34 +0,0 @@ -[build-system] -requires = ["setuptools"] -build-backend = "setuptools.build_meta" - -[project] -name = "test_infra" -version = "0.1.0" -requires-python = ">=3.11" -dependencies = [ - "flask == 2.3.2", - "cloudevents == 1.9.0", - "gunicorn == 22.0.0" -] - -[project.optional-dependencies] -signify-secret-rotator = ["google-cloud-secret-manager == 2.20.2"] -slack-message-sender = [ - "slack_bolt == 1.18.0", - "slack_sdk == 3.21.3", - "pygithub == 2.1.1", - "pyyaml == 6.0.1" -] - -[project.scripts] -signify-secret-rotator = "signifysecretrotator.signifysecretrotator:setup_app" - -[tool.setuptools] -packages = [ - "slackmessagesender", - "signifysecretrotator", - "secretmanager", - "pylogger" -] -package-dir = {"slackmessagesender" = "cmd/cloud-run/slack-message-sender","signifysecretrotator" = "cmd/cloud-run/signifysecretrotator","" = "pkg"} \ No newline at end of file From 5abddd186641fb0f2fbaacbd4cf73c6ec01bb6fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Tue, 17 Sep 2024 13:56:56 +0200 Subject: [PATCH 11/24] Update dockerfile --- cmd/cloud-run/signifysecretrotator/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/Dockerfile b/cmd/cloud-run/signifysecretrotator/Dockerfile index 79a858951d45..369ee8bdb59b 100644 --- a/cmd/cloud-run/signifysecretrotator/Dockerfile +++ b/cmd/cloud-run/signifysecretrotator/Dockerfile @@ -5,10 +5,10 @@ ENV PYTHONUNBUFFERED True WORKDIR /app -COPY ./cmd/cloud-run/slack-message-sender/slack-message-sender.py . +COPY ./cmd/cloud-run/slack-message-sender/**/*.py . COPY ./cmd/cloud-run/slack-message-sender/requirements.txt . RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ apk add --no-cache ca-certificates -CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "signify-secret-rotator"] +CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "signifysecretrotator:app"] From fd5f94e4cbb0f1c6fe005b4ab4757b96948f5445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Tue, 17 Sep 2024 16:50:59 +0200 Subject: [PATCH 12/24] Add logger and fix client --- cmd/cloud-run/signifysecretrotator/Dockerfile | 4 +- .../__pycache__/logger.cpython-312.pyc | Bin 0 -> 2635 bytes .../signifysecretrotator/pylogger/logger.py | 58 +++++++++++++ .../pylogger/test_logger.py | 81 ++++++++++++++++++ .../secretmanager/client.py | 2 +- .../signifysecretrotator.py | 4 +- 6 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 cmd/cloud-run/signifysecretrotator/pylogger/__pycache__/logger.cpython-312.pyc create mode 100644 cmd/cloud-run/signifysecretrotator/pylogger/logger.py create mode 100644 cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py diff --git a/cmd/cloud-run/signifysecretrotator/Dockerfile b/cmd/cloud-run/signifysecretrotator/Dockerfile index 369ee8bdb59b..03a3ff6079af 100644 --- a/cmd/cloud-run/signifysecretrotator/Dockerfile +++ b/cmd/cloud-run/signifysecretrotator/Dockerfile @@ -5,8 +5,8 @@ ENV PYTHONUNBUFFERED True WORKDIR /app -COPY ./cmd/cloud-run/slack-message-sender/**/*.py . -COPY ./cmd/cloud-run/slack-message-sender/requirements.txt . +COPY ./cmd/cloud-run/signifysecretrotator/**/*.py . +COPY ./cmd/cloud-run/signifysecretrotator/requirements.txt . RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ apk add --no-cache ca-certificates diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/__pycache__/logger.cpython-312.pyc b/cmd/cloud-run/signifysecretrotator/pylogger/__pycache__/logger.cpython-312.pyc new file mode 100644 index 0000000000000000000000000000000000000000..73a5eaca2c16a495d7294ff15840150a5cc20df3 GIT binary patch literal 2635 zcmcIm&2JM&6rc6(+H1#&O+vtswpkif;()zHDGiDM38g6oqNECKRr#{oc-GE_wb#t7 z3$~2lkVBB#Le)be(MljyDu@fcRP@w8pqEgJ2&+n^qCIdk4N|3?`rfW>g37U-$Zy`f zc{}qye(z0w>h6va7{7e^q!14i@)#HGA-99hI`CLt9rdwrYw8T(PoSK|c+<+Y;a&R6HMd8ed7u-uGQE-*vOm#F5)Dg&|7xK%7$g&H^W z-~)49!*!TesbOGErUg^BQu89Z4nFC6J=u;$wk3&!349BXi{#ft-{tzH`pWKmiNRH6 za2>swPRpL6>t&;4>be)x^^!xYHu7;@KT|dA)(%`ioxHBI9{A`cVF{R6Hxi{yqQGzj z;39b_9bA_LSq;=si zNvy)01vBsJ7WE=5K%^HA@RM(s5Sos>kCyL&StboAqDIJqg8C}dAUbOJqam@A4e4bF zTG)JAOPh*qRGpU_GRrhXHqww6BCYLB6O}reVJbJot5BegFpca0ovY+)sX-PtX}4Lb z90Io1`b$Tr>su@*d%9LK5QRZx0LszM!VBBRoN4oV|HqjLEa1!s%*dOW38(Cu=iCiJ zt4CWA<62$OT-@av!^qSJ5T00OMaL=Hrctp1tfLN8Z49<`s8pMBFEVc$6iUPsi>B+z z9J1o3BMh<2wj)U)LFd>`AYR))RQXvT z7fCZob`Gy?A6eaZXm#7+r?R*=wj_O;c&d9D9+7w3*VpTo- zbMi=2CcE}7HJTAP*slvOQZhL=ie`RugOR{0A6Ub{0I!H= zKD0C!TdoRthlblaJQi!kL_Nxie7v(lX-h>NhTf4LS0}36bxIwnJ8!rupLeP@Rp(5# zVng1jYL!)}A6Dxevoo{PH=*h2aQ)3tkheWxD@V^%HAqJk6YksQcPS{#Y)9fN z_kuDcBu=8;m!p@WSN5!o-B*VESp6pl^@9HX#Xnko?Jl$rlsfU_q9FQ#|uYbBj|!x zV=t`gT~mH{&LV8aK)`nafp;pIT20Pe8@_$)ZgS?1`{RWlE&#J6|Dkj*nV-dd-=R&x z+ZAI;_=4f{#xPI}3)>R`&J3f#-awK;@+K0z)VJF|A}|F1vtUT@Bwhl7&sPReIPmJ) zz~IxaUb**CYM?2@+zgX}*P0RJ6w;q+Mv;q=-W^TUf=^;S>)pr=hvnVZ`ql|d&16^} zx+Wq&)J%$U|H?#@z;t8u8BT#Ect1FU)-!xTy|7!WKTB None: + self.log_fields: Dict[str, Any] = { + "component": component_name, + "labels": {"io.kyma.component": application_name}, + } + if request: + trace_header: str = request.headers.get("X-Cloud-Trace-Context") + + if trace_header and project_id: + trace = trace_header.split("/") + self.log_fields["logging.googleapi.com/trace"] = ( + f"projects/{project_id}/traces/{trace[0]}" + ) + + def log_error(self, message: str): + """Print log error message + + Args: + message (str): Custom message that should be placed in entry + """ + self._log(message, "ERROR") + + def log_info(self, message: str): + """Print log info message + + Args: + message (str): Custom message that should be placed in entry + """ + self._log(message, "INFO") + + def _log(self, message: str, severity: str): + entry = LogEntry(severity=severity, message=message, **self.log_fields) + + print(entry) diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py new file mode 100644 index 000000000000..6d7cd700354a --- /dev/null +++ b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py @@ -0,0 +1,81 @@ +"""Tests for logger module""" + +import unittest +from unittest.mock import patch +import json + +# pylint: disable=import-error +# False positive see: https://github.com/pylint-dev/pylint/issues/3984 +from logger import Logger, LogEntry + + +class TestLogger(unittest.TestCase): + """Tests for logger class""" + + @patch("flask.Request") + def test_logger_initialization(self, mock_request): + """Test logger initialization""" + mock_request = mock_request() + mock_request.headers.get.return_value = "trace-id/other-info" + + logger = Logger("component1", "application1", "project1", mock_request) + + self.assertEqual(logger.log_fields["component"], "component1") + self.assertEqual( + logger.log_fields["labels"]["io.kyma.component"], "application1" + ) + self.assertTrue("logging.googleapi.com/trace" in logger.log_fields) + + def test_logger_initialization_without_request(self): + """Test logger initialization without flas request""" + logger = Logger("component1", "application1") + + self.assertEqual(logger.log_fields["component"], "component1") + self.assertEqual( + logger.log_fields["labels"]["io.kyma.component"], "application1" + ) + self.assertFalse("logging.googleapi.com/trace" in logger.log_fields) + + @patch("builtins.print") + def test_log_error(self, mock_print): + """Test log_error method""" + logger = Logger("component1", "application1") + logger.log_error("An error occurred") + + expected_output = LogEntry( + severity="ERROR", + message="An error occurred", + component="component1", + labels={"io.kyma.component": "application1"}, + ) + + mock_print.assert_called_once_with(expected_output) + + @patch("builtins.print") + def test_log_info(self, mock_print): + """Test log_info method""" + logger = Logger("component1", "application1") + logger.log_info("Information message") + + expected_output = LogEntry( + severity="INFO", + message="Information message", + component="component1", + labels={"io.kyma.component": "application1"}, + ) + + mock_print.assert_called_once_with(expected_output) + + def test_log_entry_str(self): + """Test log entry to json""" + log_entry = LogEntry( + severity="INFO", message="Test", component="test_component" + ) + expected_output = json.dumps( + {"severity": "INFO", "message": "Test", "component": "test_component"} + ) + self.assertEqual(str(log_entry), expected_output) + + +if __name__ == "__main__": + unittest.main() diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py index 4d6d3cb5a815..e540f91e5e31 100644 --- a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py @@ -31,7 +31,7 @@ def get_secret( secret_name = f"{secret_id}/versions/{secret_version}" response: secretmanager.AccessSecretVersionResponse = ( - self.client.access_secret_version(secret_name=secret_name) + self.client.access_secret_version(name=secret_name) ) secret_value = response.payload.data.decode() diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index 337c162734d9..d6d9ebda92be 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -17,7 +17,7 @@ from secretmanager import client app = Flask(__name__) -project_id: str = os.getenv("PROJECT_ID") +project_id: str = os.getenv("PROJECT_ID", "sap-kyma-prow") component_name: str = os.getenv("COMPONENT_NAME", "signify-certificate-rotator") application_name: str = os.getenv("APPLICATION_NAME", "secret-rotator") @@ -75,6 +75,8 @@ def rotate_signify_secret() -> Response: new_certs, new_private_key, secret_data, created_at ) + sm_client.set_secret(secret_rotate_msg["name"], json.dumps(new_secret_data)) + print( LogEntry( severity="INFO", From a6ace7c6504661b0c7bc75e451e8f68d31219bb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Wed, 18 Sep 2024 08:24:05 +0200 Subject: [PATCH 13/24] Clean up code, add subpackages --- cmd/cloud-run/signifysecretrotator/Dockerfile | 1 + .../__pycache__/logger.cpython-312.pyc | Bin 2635 -> 0 bytes .../signifysecretrotator/requirements.txt | 3 +- .../signifysecretrotator/signify/__init__.py | 0 .../signifysecretrotator/signify/client.py | 123 +++++++++++++ .../signify/test_client.py | 141 +++++++++++++++ .../{ => signify}/test_fixtures.py | 0 .../signifysecretrotator.py | 169 ++++-------------- 8 files changed, 300 insertions(+), 137 deletions(-) delete mode 100644 cmd/cloud-run/signifysecretrotator/pylogger/__pycache__/logger.cpython-312.pyc create mode 100644 cmd/cloud-run/signifysecretrotator/signify/__init__.py create mode 100644 cmd/cloud-run/signifysecretrotator/signify/client.py create mode 100644 cmd/cloud-run/signifysecretrotator/signify/test_client.py rename cmd/cloud-run/signifysecretrotator/{ => signify}/test_fixtures.py (100%) diff --git a/cmd/cloud-run/signifysecretrotator/Dockerfile b/cmd/cloud-run/signifysecretrotator/Dockerfile index 03a3ff6079af..8477656cbe0c 100644 --- a/cmd/cloud-run/signifysecretrotator/Dockerfile +++ b/cmd/cloud-run/signifysecretrotator/Dockerfile @@ -8,6 +8,7 @@ WORKDIR /app COPY ./cmd/cloud-run/signifysecretrotator/**/*.py . COPY ./cmd/cloud-run/signifysecretrotator/requirements.txt . +RUN apk add g++ RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ apk add --no-cache ca-certificates diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/__pycache__/logger.cpython-312.pyc b/cmd/cloud-run/signifysecretrotator/pylogger/__pycache__/logger.cpython-312.pyc deleted file mode 100644 index 73a5eaca2c16a495d7294ff15840150a5cc20df3..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2635 zcmcIm&2JM&6rc6(+H1#&O+vtswpkif;()zHDGiDM38g6oqNECKRr#{oc-GE_wb#t7 z3$~2lkVBB#Le)be(MljyDu@fcRP@w8pqEgJ2&+n^qCIdk4N|3?`rfW>g37U-$Zy`f zc{}qye(z0w>h6va7{7e^q!14i@)#HGA-99hI`CLt9rdwrYw8T(PoSK|c+<+Y;a&R6HMd8ed7u-uGQE-*vOm#F5)Dg&|7xK%7$g&H^W z-~)49!*!TesbOGErUg^BQu89Z4nFC6J=u;$wk3&!349BXi{#ft-{tzH`pWKmiNRH6 za2>swPRpL6>t&;4>be)x^^!xYHu7;@KT|dA)(%`ioxHBI9{A`cVF{R6Hxi{yqQGzj z;39b_9bA_LSq;=si zNvy)01vBsJ7WE=5K%^HA@RM(s5Sos>kCyL&StboAqDIJqg8C}dAUbOJqam@A4e4bF zTG)JAOPh*qRGpU_GRrhXHqww6BCYLB6O}reVJbJot5BegFpca0ovY+)sX-PtX}4Lb z90Io1`b$Tr>su@*d%9LK5QRZx0LszM!VBBRoN4oV|HqjLEa1!s%*dOW38(Cu=iCiJ zt4CWA<62$OT-@av!^qSJ5T00OMaL=Hrctp1tfLN8Z49<`s8pMBFEVc$6iUPsi>B+z z9J1o3BMh<2wj)U)LFd>`AYR))RQXvT z7fCZob`Gy?A6eaZXm#7+r?R*=wj_O;c&d9D9+7w3*VpTo- zbMi=2CcE}7HJTAP*slvOQZhL=ie`RugOR{0A6Ub{0I!H= zKD0C!TdoRthlblaJQi!kL_Nxie7v(lX-h>NhTf4LS0}36bxIwnJ8!rupLeP@Rp(5# zVng1jYL!)}A6Dxevoo{PH=*h2aQ)3tkheWxD@V^%HAqJk6YksQcPS{#Y)9fN z_kuDcBu=8;m!p@WSN5!o-B*VESp6pl^@9HX#Xnko?Jl$rlsfU_q9FQ#|uYbBj|!x zV=t`gT~mH{&LV8aK)`nafp;pIT20Pe8@_$)ZgS?1`{RWlE&#J6|Dkj*nV-dd-=R&x z+ZAI;_=4f{#xPI}3)>R`&J3f#-awK;@+K0z)VJF|A}|F1vtUT@Bwhl7&sPReIPmJ) zz~IxaUb**CYM?2@+zgX}*P0RJ6w;q+Mv;q=-W^TUf=^;S>)pr=hvnVZ`ql|d&16^} zx+Wq&)J%$U|H?#@z;t8u8BT#Ect1FU)-!xTy|7!WKTB None: + self.token_url = token_url + self.certificate_service_url = certificate_service_url + self.client_id = client_id + + def fetch_access_token(self, certificate: bytes, private_key: bytes) -> str: + """fetches access token from given token_url using certificate and private key""" + # Use temporary file for old cert and key because requests library needs file paths, + # the code is running in known environment controlled by us + with ( + tempfile.NamedTemporaryFile() as old_cert_file, + tempfile.NamedTemporaryFile() as old_key_file, + ): + + old_cert_file.write(certificate) + old_cert_file.flush() + + old_key_file.write(private_key) + old_key_file.flush() + + access_token_response = requests.post( + self.token_url, + cert=(old_cert_file.name, old_key_file.name), + data={ + "grant_type": "client_credentials", + "client_id": self.client_id, + }, + timeout=30, + ) + + if access_token_response.status_code != 200: + raise requests.HTTPError( + f"Got not-success status code {access_token_response.status_code}", + response=access_token_response, + ) + + decoded_response = access_token_response.json() + + if "access_token" not in decoded_response: + raise ValueError( + f"Got unexpected response structure: {decoded_response}" + ) + + return decoded_response["access_token"] + + def fetch_new_certificate( + self, cert_data: bytes, private_key: rsa.RSAPrivateKey, access_token: str + ): + """Fetch new certificates from given certificate service""" + + csr = self._prepare_csr(cert_data, private_key) + + crt_create_payload = self._prepare_cert_request_paylaod(csr) + + cert_create_response = requests.post( + self.certificate_service_url, + headers={ + "Authorization": f"Bearer {access_token}", + "Content-Type": "application/json", + "Accept": "application/json", + }, + data=crt_create_payload, + timeout=10, + ) + + if cert_create_response.status_code != 200: + raise requests.HTTPError( + f"Got un-success statsu code {cert_create_response.status_code}" + ) + + decoded_response = cert_create_response.json() + + if ( + "certificateChain" not in decoded_response + or "value" not in decoded_response["certificateChain"] + ): + raise ValueError( + f"Cannot issue new certifacte, invalid response format: {decoded_response}" + ) + + pkcs7_certs = decoded_response["certificateChain"]["value"].encode() + + return pkcs7.load_pem_pkcs7_certificates(pkcs7_certs) + + def _prepare_cert_request_paylaod(self, csr: x509.CertificateSigningRequest): + return json.dumps( + { + "csr": { + "value": csr.public_bytes(serialization.Encoding.PEM).decode( + "utf-8" + ) + }, + "validity": {"value": 7, "type": "DAYS"}, + "policy": "sap-cloud-platform-clients", + } + ) + + def _prepare_csr(self, cert_data: bytes, private_key: rsa.RSAPrivateKey): + old_cert = x509.load_pem_x509_certificate(cert_data) + + csr = ( + x509.CertificateSigningRequestBuilder() + .subject_name(old_cert.subject) + .sign(private_key, hashes.SHA256()) + ) + + return csr diff --git a/cmd/cloud-run/signifysecretrotator/signify/test_client.py b/cmd/cloud-run/signifysecretrotator/signify/test_client.py new file mode 100644 index 000000000000..79eecb8e4577 --- /dev/null +++ b/cmd/cloud-run/signifysecretrotator/signify/test_client.py @@ -0,0 +1,141 @@ +"""Tests for signify client module""" + +import base64 +import unittest +from unittest.mock import patch, MagicMock + +import requests + +# pylint: disable=import-error +# False positive see: https://github.com/pylint-dev/pylint/issues/3984 +from client import SignifyClient +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.serialization import Encoding, PrivateFormat + +# pylint: disable=import-error +# False positive see: https://github.com/pylint-dev/pylint/issues/3984 +import test_fixtures + + +class TestSignifyClient(unittest.TestCase): + """ + Unit tests for the SignifyClient class. + """ + + def setUp(self): + """ + Set up method to initialize the SignifyClient object and necessary data for tests. + """ + self.token_url = "https://example.com/token" + self.certificate_service_url = "https://example.com/certificate" + self.client = SignifyClient( + token_url=self.token_url, + certificate_service_url=self.certificate_service_url, + client_id="fake_client_id", + ) + self.certificate = base64.b64decode( + test_fixtures.mocked_secret_data["certData"] + ) + self.private_key = rsa.generate_private_key( + public_exponent=65537, key_size=2048 + ) + self.access_token = "fake_access_token" + + @patch("requests.post") + def test_fetch_access_token_success(self, mock_post): + """ + Test successful fetch of access token. + + Mock the requests.post to return a successful response with the expected access token. + """ + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"access_token": self.access_token} + mock_post.return_value = mock_response + private_key_bytes = self.private_key.private_bytes( + Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() + ) + + token = self.client.fetch_access_token( + self.certificate, + private_key_bytes, + ) + + self.assertEqual(token, self.access_token) + mock_post.assert_called_once() + + @patch("requests.post") + def test_fetch_access_token_failed_status_code(self, mock_post): + """ + Test fetch of access token when the response status code is not 200. + """ + mock_response = MagicMock() + mock_response.status_code = 400 + mock_response.json.return_value = {} + mock_post.return_value = mock_response + private_key_bytes = self.private_key.private_bytes( + Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() + ) + + with self.assertRaises(requests.HTTPError): + self.client.fetch_access_token( + certificate=self.certificate, private_key=private_key_bytes + ) + mock_post.assert_called_once() + + @patch("requests.post") + def test_fetch_access_token_unexpected_response(self, mock_post): + """ + Test fetch of access token when the response does not contain the expected structure. + """ + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {} + mock_post.return_value = mock_response + private_key_bytes = self.private_key.private_bytes( + Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() + ) + + with self.assertRaises(ValueError): + self.client.fetch_access_token( + certificate=self.certificate, private_key=private_key_bytes + ) + mock_post.assert_called_once() + + @patch("requests.post") + def test_fetch_new_certificate_success(self, mock_post): + """ + Test successful fetch of a new certificate. + """ + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = test_fixtures.mocked_cert_create_response + mock_post.return_value = mock_response + + certs = self.client.fetch_new_certificate( + self.certificate, self.private_key, self.access_token + ) + + self.assertEqual(len(certs), 1) + mock_post.assert_called_once() + + @patch("requests.post") + def test_fetch_new_certificate_failed_status_code(self, mock_post): + """ + Test fetch of a new certificate when the response status code is not 200.s + """ + mock_response = MagicMock() + mock_response.status_code = 400 + mock_response.json.return_value = {} + mock_post.return_value = mock_response + + with self.assertRaises(KeyError): + self.client.fetch_new_certificate( + self.certificate, self.private_key, self.access_token + ) + mock_post.assert_called_once() + + +if __name__ == "__main__": + unittest.main() diff --git a/cmd/cloud-run/signifysecretrotator/test_fixtures.py b/cmd/cloud-run/signifysecretrotator/signify/test_fixtures.py similarity index 100% rename from cmd/cloud-run/signifysecretrotator/test_fixtures.py rename to cmd/cloud-run/signifysecretrotator/signify/test_fixtures.py diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index d6d9ebda92be..aaf30e409ee2 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -5,36 +5,33 @@ import base64 import json import sys -import tempfile import traceback from typing import Any, Dict, List -import requests from flask import Flask, Response, request, make_response from cryptography import x509 -from cryptography.hazmat.primitives import serialization, hashes -from cryptography.hazmat.primitives.serialization import pkcs7, Encoding, PrivateFormat from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.serialization import Encoding, PrivateFormat +from requests import HTTPError from secretmanager import client +from pylogger.logger import Logger +from signify.client import SignifyClient app = Flask(__name__) project_id: str = os.getenv("PROJECT_ID", "sap-kyma-prow") component_name: str = os.getenv("COMPONENT_NAME", "signify-certificate-rotator") application_name: str = os.getenv("APPLICATION_NAME", "secret-rotator") - - -# TODO(kacpermalachowski): Move it to common package -class LogEntry(dict): - """Simplifies logging by returning a JSON string.""" - - def __str__(self): - return json.dumps(self) +secret_rotate_message_type = os.getenv("SECRET_ROTATE_MESSAGE_TYPE", "signify") @app.route("/", methods=["POST"]) def rotate_signify_secret() -> Response: """HTTP webhook handler for rotating Signify secrets.""" - log_fields: Dict[str, Any] = prepare_log_fields() - log_fields["labels"]["io.kyma.app"] = "signify-certificate-rotate" + logger = Logger( + component_name=component_name, + application_name=application_name, + request=request, + ) try: sm_client = client.SecretManagerClient() @@ -46,11 +43,19 @@ def rotate_signify_secret() -> Response: secret_rotate_msg = extract_message_data(pubsub_message) - if secret_rotate_msg["labels"]["type"] != "signify": - return prepare_error_response("Unsupported resource type", log_fields) + # Pub/Sub topic handle multiple secret rotator components + # verify if we should handle that message + if secret_rotate_msg["labels"]["type"] != secret_rotate_message_type: + return prepare_error_response("Unsupported event type", logger) secret_data = sm_client.get_secret(secret_rotate_msg["name"]) + signify_client = SignifyClient( + token_url=secret_data["tokenURL"], + certificate_service_url=secret_data["certServiceURL"], + client_id=secret_data["clientID"], + ) + old_cert_data = base64.b64decode(secret_data["certData"]) old_pk_data = base64.b64decode(secret_data["privateKeyData"]) @@ -61,14 +66,17 @@ def rotate_signify_secret() -> Response: new_private_key = rsa.generate_private_key(public_exponent=65537, key_size=4096) - access_token = fetch_access_token( - old_cert_data, old_pk_data, secret_data["tokenURL"], secret_data["clientID"] + access_token = signify_client.fetch_access_token( + certificate=old_cert_data, + private_key=old_pk_data, ) created_at = datetime.datetime.now().strftime("%d-%m-%Y %H:%M:%S") - new_certs: List[x509.Certificate] = fetch_new_certificate( - old_cert_data, new_private_key, access_token, secret_data["certServiceURL"] + new_certs: List[x509.Certificate] = signify_client.fetch_new_certificate( + cert_data=old_cert_data, + private_key=new_private_key, + access_token=access_token, ) new_secret_data = prepare_new_secret( @@ -77,58 +85,13 @@ def rotate_signify_secret() -> Response: sm_client.set_secret(secret_rotate_msg["name"], json.dumps(new_secret_data)) - print( - LogEntry( - severity="INFO", - message="Certificate rotated successfully", - **log_fields, - ) - ) + logger.log_info(f"Certificate rotated successfully at {created_at}") return "Certificate rotated successfully" + except HTTPError as exc: + return prepare_error_response(exc, logger) except ValueError as exc: - return prepare_error_response(exc, log_fields) - - -def fetch_new_certificate( - cert_data: bytes, - private_key: rsa.RSAPrivateKey, - access_token: str, - certificate_service_url: str, -): - """Fetch new certificates from given certificate service""" - old_cert = x509.load_pem_x509_certificate(cert_data) - - csr = ( - x509.CertificateSigningRequestBuilder() - .subject_name(old_cert.subject) - .sign(private_key, hashes.SHA256()) - ) - - crt_create_payload = json.dumps( - { - "csr": { - "value": csr.public_bytes(serialization.Encoding.PEM).decode("utf-8") - }, - "validity": {"value": 7, "type": "DAYS"}, - "policy": "sap-cloud-platform-clients", - } - ) - - cert_create_response = requests.post( - certificate_service_url, - headers={ - "Authorization": f"Bearer {access_token}", - "Content-Type": "application/json", - "Accept": "application/json", - }, - data=crt_create_payload, - timeout=10, - ).json() - - pkcs7_certs = cert_create_response["certificateChain"]["value"].encode() - - return pkcs7.load_pem_pkcs7_certificates(pkcs7_certs) + return prepare_error_response(exc, logger) def prepare_new_secret( @@ -173,97 +136,33 @@ def extract_message_data(pubsub_message: Any) -> Any: def decrypt_private_key(private_key_data: bytes, password: bytes) -> bytes: """Decrypts an encrypted private key.""" - # pylint: disable=line-too-long private_key = serialization.load_pem_private_key(private_key_data, password) - # pylint: disable=line-too-long return private_key.private_bytes( Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() ) -def fetch_access_token( - certificate: bytes, private_key: bytes, token_url: str, client_id: str -) -> str: - """fetches access token from given token_url using certificate and private key""" - # Use temporary file for old cert and key because requests library needs file paths, - # it's not a security concern because the code is running in known environment controlled by us - # pylint: disable=line-too-long - with ( - tempfile.NamedTemporaryFile() as old_cert_file, - tempfile.NamedTemporaryFile() as old_key_file, - ): - - old_cert_file.write(certificate) - old_cert_file.flush() - - old_key_file.write(private_key) - old_key_file.flush() - - # pylint: disable=line-too-long - access_token_response = requests.post( - token_url, - cert=(old_cert_file.name, old_key_file.name), - data={ - "grant_type": "client_credentials", - "client_id": client_id, - }, - timeout=30, - ).json() - - return access_token_response["access_token"] - - -# TODO(kacpermalachowski): Move it to common package -def prepare_log_fields() -> Dict[str, Any]: - """prepare_log_fields prapares basic log fields""" - log_fields: Dict[str, Any] = {} - request_is_defined = "request" in globals() or "request" in locals() - if request_is_defined and request: - trace_header = request.headers.get("X-Cloud-Trace-Context") - if trace_header and project_id: - trace = trace_header.split("/") - log_fields["logging.googleapis.com/trace"] = ( - f"projects/{project_id}/traces/{trace[0]}" - ) - log_fields["Component"] = "signify-certificate-rotator" - log_fields["labels"] = {"io.kyma.component": "signify-certificate-rotator"} - return log_fields - - # TODO(kacpermalachowski): Move it to common package def get_pubsub_message(): """Parses the Pub/Sub message from the request.""" envelope = request.get_json() if not envelope: - # pylint: disable=broad-exception-raised raise ValueError("No Pub/Sub message received") if not isinstance(envelope, dict) or "message" not in envelope: - # pylint: disable=broad-exception-raised raise ValueError("Invalid Pub/Sub message format") return envelope["message"] # TODO(kacpermalachowski): Move it to common package -def prepare_error_response(err: str, log_fields: Dict[str, Any]) -> Response: +def prepare_error_response(err: str, logger: Logger) -> Response: """Prepares an error response with logging.""" _, exc_value, _ = sys.exc_info() stacktrace = repr(traceback.format_exception(exc_value)) - print( - LogEntry( - severity="ERROR", - message=f"Error: {err}\nStack:\n {stacktrace}", - **log_fields, - ) - ) + logger.log_error(f"Error: {err}\nStack:\n {stacktrace}") resp = make_response() resp.content_type = "application/json" resp.status_code = 500 return resp - - -def setup_app(): - print("test") - pass From 140a4133f91a2edb69087b6514bfc022b2bc3168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Wed, 18 Sep 2024 09:44:37 +0200 Subject: [PATCH 14/24] Fix last issues --- cmd/cloud-run/signifysecretrotator/Dockerfile | 9 ++++----- cmd/cloud-run/signifysecretrotator/requirements.txt | 3 ++- cmd/cloud-run/slack-message-sender/__init__.py | 0 3 files changed, 6 insertions(+), 6 deletions(-) delete mode 100644 cmd/cloud-run/slack-message-sender/__init__.py diff --git a/cmd/cloud-run/signifysecretrotator/Dockerfile b/cmd/cloud-run/signifysecretrotator/Dockerfile index 8477656cbe0c..8f2b293ad716 100644 --- a/cmd/cloud-run/signifysecretrotator/Dockerfile +++ b/cmd/cloud-run/signifysecretrotator/Dockerfile @@ -1,15 +1,14 @@ -FROM python:3.12.5-alpine3.20 +FROM python:3.12.6-slim # Allow statements and log messages to immediately appear in the Knative logs ENV PYTHONUNBUFFERED True WORKDIR /app -COPY ./cmd/cloud-run/signifysecretrotator/**/*.py . +COPY ./cmd/cloud-run/signifysecretrotator . COPY ./cmd/cloud-run/signifysecretrotator/requirements.txt . -RUN apk add g++ -RUN pip install --no-cache-dir --upgrade -r requirements.txt && \ - apk add --no-cache ca-certificates +RUN pip3 install --upgrade -r requirements.txt && \ + apt-get install ca-certificates CMD ["gunicorn", "--bind", "0.0.0.0:8080", "--workers", "4", "--timeout", "0", "signifysecretrotator:app"] diff --git a/cmd/cloud-run/signifysecretrotator/requirements.txt b/cmd/cloud-run/signifysecretrotator/requirements.txt index b3d01df69b6b..1821d3243438 100644 --- a/cmd/cloud-run/signifysecretrotator/requirements.txt +++ b/cmd/cloud-run/signifysecretrotator/requirements.txt @@ -1,4 +1,5 @@ flask==2.3.2 cloudevents==1.9.0 gunicorn==22.0.0 -google-cloud-secret-manager==2.20.2 \ No newline at end of file +google-cloud-secret-manager==2.20.2 +cryptography==43.0.1 \ No newline at end of file diff --git a/cmd/cloud-run/slack-message-sender/__init__.py b/cmd/cloud-run/slack-message-sender/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 From aa7842a603a8bbb7dcfd5ab251fa1a7635b928e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= <38684517+KacperMalachowski@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:31:19 +0200 Subject: [PATCH 15/24] Apply suggestions from code review Co-authored-by: Patryk Dobrowolski --- cmd/cloud-run/signifysecretrotator/signify/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/signify/client.py b/cmd/cloud-run/signifysecretrotator/signify/client.py index 63b023ac18ec..81ede21766e0 100644 --- a/cmd/cloud-run/signifysecretrotator/signify/client.py +++ b/cmd/cloud-run/signifysecretrotator/signify/client.py @@ -46,7 +46,7 @@ def fetch_access_token(self, certificate: bytes, private_key: bytes) -> str: if access_token_response.status_code != 200: raise requests.HTTPError( - f"Got not-success status code {access_token_response.status_code}", + f"Got non-success status code {access_token_response.status_code}", response=access_token_response, ) @@ -81,7 +81,7 @@ def fetch_new_certificate( if cert_create_response.status_code != 200: raise requests.HTTPError( - f"Got un-success statsu code {cert_create_response.status_code}" + f"Got non-success status code {cert_create_response.status_code}" ) decoded_response = cert_create_response.json() From 3f32d5e21c0f6f46a2fc1a57083523161f28fe73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Wed, 18 Sep 2024 10:50:26 +0200 Subject: [PATCH 16/24] Add types, fix magic numbers in csr --- .../signifysecretrotator/pylogger/logger.py | 8 +-- .../pylogger/test_logger.py | 10 +-- .../secretmanager/client.py | 2 +- .../secretmanager/test_client.py | 20 +++--- .../signifysecretrotator/signify/client.py | 65 ++++++++++++++----- .../signify/test_client.py | 16 ++--- .../signifysecretrotator.py | 22 ++++--- 7 files changed, 89 insertions(+), 54 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/logger.py b/cmd/cloud-run/signifysecretrotator/pylogger/logger.py index b5292e22491b..3c784f695f32 100644 --- a/cmd/cloud-run/signifysecretrotator/pylogger/logger.py +++ b/cmd/cloud-run/signifysecretrotator/pylogger/logger.py @@ -9,7 +9,7 @@ class LogEntry(dict): """Simplifies logging by returning a JSON string.""" - def __str__(self): + def __str__(self) -> str: return json.dumps(self) @@ -36,7 +36,7 @@ def __init__( f"projects/{project_id}/traces/{trace[0]}" ) - def log_error(self, message: str): + def log_error(self, message: str) -> None: """Print log error message Args: @@ -44,7 +44,7 @@ def log_error(self, message: str): """ self._log(message, "ERROR") - def log_info(self, message: str): + def log_info(self, message: str) -> None: """Print log info message Args: @@ -52,7 +52,7 @@ def log_info(self, message: str): """ self._log(message, "INFO") - def _log(self, message: str, severity: str): + def _log(self, message: str, severity: str) -> None: entry = LogEntry(severity=severity, message=message, **self.log_fields) print(entry) diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py index 6d7cd700354a..262f3bd1e8c9 100644 --- a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py +++ b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py @@ -13,7 +13,7 @@ class TestLogger(unittest.TestCase): """Tests for logger class""" @patch("flask.Request") - def test_logger_initialization(self, mock_request): + def test_logger_initialization(self, mock_request) -> None: """Test logger initialization""" mock_request = mock_request() mock_request.headers.get.return_value = "trace-id/other-info" @@ -26,7 +26,7 @@ def test_logger_initialization(self, mock_request): ) self.assertTrue("logging.googleapi.com/trace" in logger.log_fields) - def test_logger_initialization_without_request(self): + def test_logger_initialization_without_request(self) -> None: """Test logger initialization without flas request""" logger = Logger("component1", "application1") @@ -37,7 +37,7 @@ def test_logger_initialization_without_request(self): self.assertFalse("logging.googleapi.com/trace" in logger.log_fields) @patch("builtins.print") - def test_log_error(self, mock_print): + def test_log_error(self, mock_print) -> None: """Test log_error method""" logger = Logger("component1", "application1") logger.log_error("An error occurred") @@ -52,7 +52,7 @@ def test_log_error(self, mock_print): mock_print.assert_called_once_with(expected_output) @patch("builtins.print") - def test_log_info(self, mock_print): + def test_log_info(self, mock_print) -> None: """Test log_info method""" logger = Logger("component1", "application1") logger.log_info("Information message") @@ -66,7 +66,7 @@ def test_log_info(self, mock_print): mock_print.assert_called_once_with(expected_output) - def test_log_entry_str(self): + def test_log_entry_str(self) -> None: """Test log entry to json""" log_entry = LogEntry( severity="INFO", message="Test", component="test_component" diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py index e540f91e5e31..cb388f190ec0 100644 --- a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py @@ -40,7 +40,7 @@ def get_secret( return secret_value - def set_secret(self, secret_id: str, data: str): + def set_secret(self, secret_id: str, data: str) -> None: """Adds new secret version with given data Args: diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py index 5b4c8dabef9d..623a4c525007 100644 --- a/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py @@ -2,7 +2,7 @@ import json import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch from google.cloud import secretmanager # pylint: disable=import-error @@ -13,7 +13,7 @@ class TestSecretManagerClient(unittest.TestCase): """Tests for secret manager client""" - def setUp(self): + def setUp(self) -> None: access_secret_patcher = patch.object( secretmanager.SecretManagerServiceClient, "access_secret_version" ) @@ -21,15 +21,19 @@ def setUp(self): secretmanager.SecretManagerServiceClient, "add_secret_version" ) - self.mock_access_secret_version = access_secret_patcher.start() - self.mock_add_secret_version = add_secret_version_patcher.start() + self.mock_access_secret_version: MagicMock | AsyncMock = ( + access_secret_patcher.start() + ) + self.mock_add_secret_version: MagicMock | AsyncMock = ( + add_secret_version_patcher.start() + ) self.addCleanup(access_secret_patcher.stop) self.addCleanup(add_secret_version_patcher.stop) self.client = SecretManagerClient() - def test_get_secret_json(self): + def test_get_secret_json(self) -> None: """Tests fetching json secret data""" # Arrange mock_response = MagicMock() @@ -45,7 +49,7 @@ def test_get_secret_json(self): secret_name="projects/test-project/secrets/test-secret/versions/latest" ) - def test_get_secret_plain_string(self): + def test_get_secret_plain_string(self) -> None: """Tests fetching string secret data""" # Arrange mock_response = MagicMock() @@ -63,7 +67,7 @@ def test_get_secret_plain_string(self): secret_name="projects/test-project/secrets/test-secret/versions/latest" ) - def test_set_secret(self): + def test_set_secret(self) -> None: """Tests setting a new secret version""" # Arrange secret_id = "projects/test-project/secrets/test-secret" @@ -73,7 +77,7 @@ def test_set_secret(self): self.client.set_secret(secret_id, secret_data) # Assert - payload = {"data": secret_data.encode()} + payload: dict[str, bytes] = {"data": secret_data.encode()} self.mock_add_secret_version.assert_called_once_with( parent=secret_id, payload=payload ) diff --git a/cmd/cloud-run/signifysecretrotator/signify/client.py b/cmd/cloud-run/signifysecretrotator/signify/client.py index 81ede21766e0..7384942f48d7 100644 --- a/cmd/cloud-run/signifysecretrotator/signify/client.py +++ b/cmd/cloud-run/signifysecretrotator/signify/client.py @@ -1,7 +1,9 @@ """Custom client for signify API""" +import dataclasses import json import tempfile +from typing import Any import requests from cryptography.hazmat.primitives.asymmetric import rsa from cryptography import x509 @@ -9,17 +11,42 @@ from cryptography.hazmat.primitives.serialization import pkcs7 +@dataclasses.dataclass +class CertificateSigningRequestConfig: + """CSR configuration""" + + def __init__( + self, validity_time: int, validity_unit: str, policy_name: str + ) -> None: + self.validity: dict[str, Any] = {"value": validity_time, "type": validity_unit} + self.policy: str = policy_name + + class SignifyClient: """Wraps signify API""" def __init__( - self, token_url: str, certificate_service_url: str, client_id: str + self, + token_url: str, + certificate_service_url: str, + client_id: str, + csr_config: CertificateSigningRequestConfig = CertificateSigningRequestConfig( + validity_time=7, + validity_unit="DAYS", + policy_name="sap-cloud-platform-clients", + ), ) -> None: - self.token_url = token_url - self.certificate_service_url = certificate_service_url - self.client_id = client_id - - def fetch_access_token(self, certificate: bytes, private_key: bytes) -> str: + self.token_url: str = token_url + self.certificate_service_url: str = certificate_service_url + self.client_id: str = client_id + self.csr_config: CertificateSigningRequestConfig = csr_config + + def fetch_access_token( + self, + certificate: bytes, + private_key: bytes, + timeout=30, + ) -> str: """fetches access token from given token_url using certificate and private key""" # Use temporary file for old cert and key because requests library needs file paths, # the code is running in known environment controlled by us @@ -34,14 +61,14 @@ def fetch_access_token(self, certificate: bytes, private_key: bytes) -> str: old_key_file.write(private_key) old_key_file.flush() - access_token_response = requests.post( + access_token_response: requests.Response = requests.post( self.token_url, cert=(old_cert_file.name, old_key_file.name), data={ "grant_type": "client_credentials", "client_id": self.client_id, }, - timeout=30, + timeout=timeout, ) if access_token_response.status_code != 200: @@ -61,14 +88,14 @@ def fetch_access_token(self, certificate: bytes, private_key: bytes) -> str: def fetch_new_certificate( self, cert_data: bytes, private_key: rsa.RSAPrivateKey, access_token: str - ): + ) -> list[x509.Certificate]: """Fetch new certificates from given certificate service""" - csr = self._prepare_csr(cert_data, private_key) + csr: x509.CertificateSigningRequest = self._prepare_csr(cert_data, private_key) - crt_create_payload = self._prepare_cert_request_paylaod(csr) + crt_create_payload: str = self._prepare_cert_request_payload(csr) - cert_create_response = requests.post( + cert_create_response: requests.Response = requests.post( self.certificate_service_url, headers={ "Authorization": f"Bearer {access_token}", @@ -98,7 +125,7 @@ def fetch_new_certificate( return pkcs7.load_pem_pkcs7_certificates(pkcs7_certs) - def _prepare_cert_request_paylaod(self, csr: x509.CertificateSigningRequest): + def _prepare_cert_request_payload(self, csr: x509.CertificateSigningRequest) -> str: return json.dumps( { "csr": { @@ -106,15 +133,17 @@ def _prepare_cert_request_paylaod(self, csr: x509.CertificateSigningRequest): "utf-8" ) }, - "validity": {"value": 7, "type": "DAYS"}, - "policy": "sap-cloud-platform-clients", + "validity": self.csr_config.validity, + "policy": self.csr_config.policy, } ) - def _prepare_csr(self, cert_data: bytes, private_key: rsa.RSAPrivateKey): - old_cert = x509.load_pem_x509_certificate(cert_data) + def _prepare_csr( + self, cert_data: bytes, private_key: rsa.RSAPrivateKey + ) -> x509.CertificateSigningRequest: + old_cert: x509.Certificate = x509.load_pem_x509_certificate(cert_data) - csr = ( + csr: x509.CertificateSigningRequest = ( x509.CertificateSigningRequestBuilder() .subject_name(old_cert.subject) .sign(private_key, hashes.SHA256()) diff --git a/cmd/cloud-run/signifysecretrotator/signify/test_client.py b/cmd/cloud-run/signifysecretrotator/signify/test_client.py index 79eecb8e4577..25639315bb2d 100644 --- a/cmd/cloud-run/signifysecretrotator/signify/test_client.py +++ b/cmd/cloud-run/signifysecretrotator/signify/test_client.py @@ -23,7 +23,7 @@ class TestSignifyClient(unittest.TestCase): Unit tests for the SignifyClient class. """ - def setUp(self): + def setUp(self) -> None: """ Set up method to initialize the SignifyClient object and necessary data for tests. """ @@ -34,16 +34,16 @@ def setUp(self): certificate_service_url=self.certificate_service_url, client_id="fake_client_id", ) - self.certificate = base64.b64decode( + self.certificate: bytes = base64.b64decode( test_fixtures.mocked_secret_data["certData"] ) - self.private_key = rsa.generate_private_key( + self.private_key: rsa.RSAPrivateKey = rsa.generate_private_key( public_exponent=65537, key_size=2048 ) self.access_token = "fake_access_token" @patch("requests.post") - def test_fetch_access_token_success(self, mock_post): + def test_fetch_access_token_success(self, mock_post) -> None: """ Test successful fetch of access token. @@ -66,7 +66,7 @@ def test_fetch_access_token_success(self, mock_post): mock_post.assert_called_once() @patch("requests.post") - def test_fetch_access_token_failed_status_code(self, mock_post): + def test_fetch_access_token_failed_status_code(self, mock_post) -> None: """ Test fetch of access token when the response status code is not 200. """ @@ -85,7 +85,7 @@ def test_fetch_access_token_failed_status_code(self, mock_post): mock_post.assert_called_once() @patch("requests.post") - def test_fetch_access_token_unexpected_response(self, mock_post): + def test_fetch_access_token_unexpected_response(self, mock_post) -> None: """ Test fetch of access token when the response does not contain the expected structure. """ @@ -104,7 +104,7 @@ def test_fetch_access_token_unexpected_response(self, mock_post): mock_post.assert_called_once() @patch("requests.post") - def test_fetch_new_certificate_success(self, mock_post): + def test_fetch_new_certificate_success(self, mock_post) -> None: """ Test successful fetch of a new certificate. """ @@ -121,7 +121,7 @@ def test_fetch_new_certificate_success(self, mock_post): mock_post.assert_called_once() @patch("requests.post") - def test_fetch_new_certificate_failed_status_code(self, mock_post): + def test_fetch_new_certificate_failed_status_code(self, mock_post) -> None: """ Test fetch of a new certificate when the response status code is not 200.s """ diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index aaf30e409ee2..4383658ac650 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -48,7 +48,7 @@ def rotate_signify_secret() -> Response: if secret_rotate_msg["labels"]["type"] != secret_rotate_message_type: return prepare_error_response("Unsupported event type", logger) - secret_data = sm_client.get_secret(secret_rotate_msg["name"]) + secret_data: Dict[str, Any] = sm_client.get_secret(secret_rotate_msg["name"]) signify_client = SignifyClient( token_url=secret_data["tokenURL"], @@ -56,22 +56,24 @@ def rotate_signify_secret() -> Response: client_id=secret_data["clientID"], ) - old_cert_data = base64.b64decode(secret_data["certData"]) - old_pk_data = base64.b64decode(secret_data["privateKeyData"]) + old_cert_data: bytes = base64.b64decode(secret_data["certData"]) + old_pk_data: bytes = base64.b64decode(secret_data["privateKeyData"]) if "password" in secret_data and secret_data["password"] != "": old_pk_data = decrypt_private_key( old_pk_data, secret_data["password"].encode() ) - new_private_key = rsa.generate_private_key(public_exponent=65537, key_size=4096) + new_private_key: rsa.RSAPrivateKey = rsa.generate_private_key( + public_exponent=65537, key_size=4096 + ) - access_token = signify_client.fetch_access_token( + access_token: str = signify_client.fetch_access_token( certificate=old_cert_data, private_key=old_pk_data, ) - created_at = datetime.datetime.now().strftime("%d-%m-%Y %H:%M:%S") + created_at: str = datetime.datetime.now().strftime("%d-%m-%Y %H:%M:%S") new_certs: List[x509.Certificate] = signify_client.fetch_new_certificate( cert_data=old_cert_data, @@ -79,7 +81,7 @@ def rotate_signify_secret() -> Response: access_token=access_token, ) - new_secret_data = prepare_new_secret( + new_secret_data: Dict[str, Any] = prepare_new_secret( new_certs, new_private_key, secret_data, created_at ) @@ -103,14 +105,14 @@ def prepare_new_secret( """Prepares new secret data with updated certificates and private key.""" # format certificates - certs_string = "" + certs_string: str = "" for cert in certificates: certs_string += f"subject={cert.subject.rfc4514_string()}\n" certs_string += f"issuer={cert.issuer.rfc4514_string()}\n" certs_string += f"{cert.public_bytes(Encoding.PEM).decode()}\n" - private_key_bytes = private_key.private_bytes( + private_key_bytes: bytes = private_key.private_bytes( Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() ) @@ -144,7 +146,7 @@ def decrypt_private_key(private_key_data: bytes, password: bytes) -> bytes: # TODO(kacpermalachowski): Move it to common package -def get_pubsub_message(): +def get_pubsub_message() -> Dict[str, Any]: """Parses the Pub/Sub message from the request.""" envelope = request.get_json() if not envelope: From 44c1366ef281c2f79e829efd7628a80b9b9a7273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= <38684517+KacperMalachowski@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:52:44 +0200 Subject: [PATCH 17/24] Update cmd/cloud-run/signifysecretrotator/signifysecretrotator.py Co-authored-by: Patryk Dobrowolski --- cmd/cloud-run/signifysecretrotator/signifysecretrotator.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index 4383658ac650..d239027d3c98 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -90,9 +90,7 @@ def rotate_signify_secret() -> Response: logger.log_info(f"Certificate rotated successfully at {created_at}") return "Certificate rotated successfully" - except HTTPError as exc: - return prepare_error_response(exc, logger) - except ValueError as exc: + except (HTTPError, ValueError) as exc: return prepare_error_response(exc, logger) From 2e04f20632bc75dbea7b4f188b08b12aa23946ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Wed, 18 Sep 2024 11:22:40 +0200 Subject: [PATCH 18/24] Hide message vlaidation --- .../signifysecretrotator.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index d239027d3c98..642fffe18e27 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -36,17 +36,11 @@ def rotate_signify_secret() -> Response: try: sm_client = client.SecretManagerClient() - if project_id is None: - raise ValueError("Unknown project id") + pubsub_message: Dict[str, Any] = get_pubsub_message() - pubsub_message = get_pubsub_message() + secret_rotate_msg: Dict[str, Any] = extract_message_data(pubsub_message) - secret_rotate_msg = extract_message_data(pubsub_message) - - # Pub/Sub topic handle multiple secret rotator components - # verify if we should handle that message - if secret_rotate_msg["labels"]["type"] != secret_rotate_message_type: - return prepare_error_response("Unsupported event type", logger) + validate_message(secret_rotate_msg) secret_data: Dict[str, Any] = sm_client.get_secret(secret_rotate_msg["name"]) @@ -90,10 +84,19 @@ def rotate_signify_secret() -> Response: logger.log_info(f"Certificate rotated successfully at {created_at}") return "Certificate rotated successfully" - except (HTTPError, ValueError) as exc: + except (HTTPError, ValueError, TypeError) as exc: return prepare_error_response(exc, logger) +def validate_message(message: dict[str, Any]) -> None: + """Raises error when received message struct is invalid""" + + # Pub/Sub topic handle multiple secret rotator components + # verify if we should handle that message + if message.get("labels", {}).get("type") != secret_rotate_message_type: + raise TypeError("Invalid or unknown type value") + + def prepare_new_secret( certificates: List[x509.Certificate], private_key: rsa.RSAPrivateKey, From b017ecefcb65fdadf60205a77551797b139f1523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Wed, 18 Sep 2024 11:34:00 +0200 Subject: [PATCH 19/24] Extract key size to the config seciton" --- .../signifysecretrotator/signify/client.py | 10 +++++++++- .../signifysecretrotator/signifysecretrotator.py | 15 +++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/signify/client.py b/cmd/cloud-run/signifysecretrotator/signify/client.py index 7384942f48d7..bfbf7a6647bf 100644 --- a/cmd/cloud-run/signifysecretrotator/signify/client.py +++ b/cmd/cloud-run/signifysecretrotator/signify/client.py @@ -1,6 +1,7 @@ """Custom client for signify API""" import dataclasses +from enum import Enum import json import tempfile from typing import Any @@ -11,6 +12,13 @@ from cryptography.hazmat.primitives.serialization import pkcs7 +@dataclasses.dataclass +class OAuthGrantTypes(Enum): + """Contains possible values for grant types option""" + + CLIENT_CREDENTIALS = "client_credentials" + + @dataclasses.dataclass class CertificateSigningRequestConfig: """CSR configuration""" @@ -65,7 +73,7 @@ def fetch_access_token( self.token_url, cert=(old_cert_file.name, old_key_file.name), data={ - "grant_type": "client_credentials", + "grant_type": OAuthGrantTypes.CLIENT_CREDENTIALS, "client_id": self.client_id, }, timeout=timeout, diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index 642fffe18e27..68239060908e 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -22,6 +22,7 @@ component_name: str = os.getenv("COMPONENT_NAME", "signify-certificate-rotator") application_name: str = os.getenv("APPLICATION_NAME", "secret-rotator") secret_rotate_message_type = os.getenv("SECRET_ROTATE_MESSAGE_TYPE", "signify") +rsa_key_size: int = 4096 @app.route("/", methods=["POST"]) @@ -42,7 +43,8 @@ def rotate_signify_secret() -> Response: validate_message(secret_rotate_msg) - secret_data: Dict[str, Any] = sm_client.get_secret(secret_rotate_msg["name"]) + secret_id: str = secret_rotate_msg["name"] + secret_data: Dict[str, Any] = sm_client.get_secret(secret_id) signify_client = SignifyClient( token_url=secret_data["tokenURL"], @@ -59,7 +61,10 @@ def rotate_signify_secret() -> Response: ) new_private_key: rsa.RSAPrivateKey = rsa.generate_private_key( - public_exponent=65537, key_size=4096 + # Public exponent is standarised as 65537 + # see: https://www.daemonology.net/blog/2009-06-11-cryptographic-right-answers.html + public_exponent=65537, + key_size=rsa_key_size, ) access_token: str = signify_client.fetch_access_token( @@ -79,7 +84,7 @@ def rotate_signify_secret() -> Response: new_certs, new_private_key, secret_data, created_at ) - sm_client.set_secret(secret_rotate_msg["name"], json.dumps(new_secret_data)) + sm_client.set_secret(secret_id, json.dumps(new_secret_data)) logger.log_info(f"Certificate rotated successfully at {created_at}") @@ -139,7 +144,9 @@ def extract_message_data(pubsub_message: Any) -> Any: def decrypt_private_key(private_key_data: bytes, password: bytes) -> bytes: """Decrypts an encrypted private key.""" - private_key = serialization.load_pem_private_key(private_key_data, password) + private_key: rsa.RSAPrivateKey = serialization.load_pem_private_key( + private_key_data, password + ) return private_key.private_bytes( Encoding.PEM, PrivateFormat.PKCS8, serialization.NoEncryption() From 81a74b2c6d3eb6244fe3f609d612108c3145a34e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Wed, 18 Sep 2024 11:36:26 +0200 Subject: [PATCH 20/24] Fix comment --- cmd/cloud-run/signifysecretrotator/signifysecretrotator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index 68239060908e..f54c1ef5128d 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -62,7 +62,7 @@ def rotate_signify_secret() -> Response: new_private_key: rsa.RSAPrivateKey = rsa.generate_private_key( # Public exponent is standarised as 65537 - # see: https://www.daemonology.net/blog/2009-06-11-cryptographic-right-answers.html + # see: https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#cryptography.hazmat.primitives.asymmetric.rsa.generate_private_key public_exponent=65537, key_size=rsa_key_size, ) From d729b827d8868d9f795a92cafa0fba146120f7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Mon, 23 Sep 2024 13:51:46 +0200 Subject: [PATCH 21/24] Fix enum --- cmd/cloud-run/signifysecretrotator/signify/client.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/cloud-run/signifysecretrotator/signify/client.py b/cmd/cloud-run/signifysecretrotator/signify/client.py index bfbf7a6647bf..97ae6c25dbdc 100644 --- a/cmd/cloud-run/signifysecretrotator/signify/client.py +++ b/cmd/cloud-run/signifysecretrotator/signify/client.py @@ -5,6 +5,7 @@ import json import tempfile from typing import Any +from urllib import response import requests from cryptography.hazmat.primitives.asymmetric import rsa from cryptography import x509 @@ -18,6 +19,9 @@ class OAuthGrantTypes(Enum): CLIENT_CREDENTIALS = "client_credentials" + def __str__(self) -> str: + return str(self.value) + @dataclasses.dataclass class CertificateSigningRequestConfig: @@ -80,6 +84,7 @@ def fetch_access_token( ) if access_token_response.status_code != 200: + print(access_token_response.json(), OAuthGrantTypes.CLIENT_CREDENTIALS) raise requests.HTTPError( f"Got non-success status code {access_token_response.status_code}", response=access_token_response, From fb73db18f4146aed29e1e48e552b4dd1cee5c294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Tue, 24 Sep 2024 09:11:30 +0200 Subject: [PATCH 22/24] Rewrite logger to use logging library --- .../signifysecretrotator/pylogger/logger.py | 103 +++++++------ .../pylogger/test_logger.py | 139 +++++++++++------- .../signifysecretrotator.py | 11 +- 3 files changed, 146 insertions(+), 107 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/logger.py b/cmd/cloud-run/signifysecretrotator/pylogger/logger.py index 3c784f695f32..5dcb1df2327b 100644 --- a/cmd/cloud-run/signifysecretrotator/pylogger/logger.py +++ b/cmd/cloud-run/signifysecretrotator/pylogger/logger.py @@ -1,58 +1,69 @@ """Encapsulates logging for cloud runs""" import json +import logging from typing import Any, Dict from flask import Request -class LogEntry(dict): - """Simplifies logging by returning a JSON string.""" +class GoogleCloudFormatter(logging.Formatter): + """Wraps the formatting the logs using google cloud format""" - def __str__(self) -> str: - return json.dumps(self) + def __init__( + self, component_name: str, application_name: str, log_fields: Dict[str, Any] + ) -> None: + self.component_name: str = component_name + self.application_name: str = application_name + self.log_fields: Dict[str, Any] = log_fields + super().__init__() -class Logger: - """Encapsulates logging for cloud runs""" + def format(self, record: logging.LogRecord) -> str: + """Formats record into cloud event log""" - def __init__( - self, - component_name: str, - application_name: str, - project_id: str = "", - request: Request = None, - ) -> None: - self.log_fields: Dict[str, Any] = { - "component": component_name, - "labels": {"io.kyma.component": application_name}, - } - if request: - trace_header: str = request.headers.get("X-Cloud-Trace-Context") - - if trace_header and project_id: - trace = trace_header.split("/") - self.log_fields["logging.googleapi.com/trace"] = ( - f"projects/{project_id}/traces/{trace[0]}" - ) - - def log_error(self, message: str) -> None: - """Print log error message - - Args: - message (str): Custom message that should be placed in entry - """ - self._log(message, "ERROR") - - def log_info(self, message: str) -> None: - """Print log info message - - Args: - message (str): Custom message that should be placed in entry - """ - self._log(message, "INFO") - - def _log(self, message: str, severity: str) -> None: - entry = LogEntry(severity=severity, message=message, **self.log_fields) - - print(entry) + return json.dumps( + { + "timestamp": record.created, + "severity": record.levelname, + "message": record.getMessage(), + } + ) + + +def create_logger( + component_name: str, + application_name: str, + project_id: str = None, + request: Request = None, + log_level=logging.INFO, +) -> logging.Logger: + """Creates instance of stdout logger for aplication's component""" + logger: logging.Logger = logging.getLogger(f"{application_name}/{component_name}") + logger.setLevel(log_level) + + log_fields = { + "component": component_name, + "labels": {"io.kyma.component": application_name}, + } + + if request: + trace_header: str | None = request.headers.get("X-Cloud-Trace-Context") + + if trace_header and project_id: + trace: list[str] = trace_header.split("/") + log_fields["logging.googleapi.com/trace"] = ( + f"projects/{project_id}/traces/{trace[0]}" + ) + + formatter = GoogleCloudFormatter( + component_name=component_name, + application_name=application_name, + log_fields=log_fields, + ) + handler = logging.StreamHandler() + handler.setFormatter(formatter) + + logger.addHandler(handler) + + return logger diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py index 262f3bd1e8c9..21b100dc0796 100644 --- a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py +++ b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py @@ -1,80 +1,105 @@ """Tests for logger module""" +import time +from typing import Any import unittest -from unittest.mock import patch +import logging import json +from unittest.mock import MagicMock, Mock, patch + +from flask import Request # pylint: disable=import-error # False positive see: https://github.com/pylint-dev/pylint/issues/3984 -from logger import Logger, LogEntry - - -class TestLogger(unittest.TestCase): - """Tests for logger class""" +from logger import GoogleCloudFormatter, create_logger - @patch("flask.Request") - def test_logger_initialization(self, mock_request) -> None: - """Test logger initialization""" - mock_request = mock_request() - mock_request.headers.get.return_value = "trace-id/other-info" - logger = Logger("component1", "application1", "project1", mock_request) +class TestGoogleCloudFormatter(unittest.TestCase): + """Tests for custom formatter""" - self.assertEqual(logger.log_fields["component"], "component1") - self.assertEqual( - logger.log_fields["labels"]["io.kyma.component"], "application1" + def setUp(self): + self.component_name = "test_component" + self.application_name = "test_application" + self.log_fields = {"key1": "value1"} + self.formatter = GoogleCloudFormatter( + self.component_name, self.application_name, self.log_fields ) - self.assertTrue("logging.googleapi.com/trace" in logger.log_fields) - def test_logger_initialization_without_request(self) -> None: - """Test logger initialization without flas request""" - logger = Logger("component1", "application1") - - self.assertEqual(logger.log_fields["component"], "component1") - self.assertEqual( - logger.log_fields["labels"]["io.kyma.component"], "application1" + def test_format(self) -> None: + """Test format function""" + log_record = logging.LogRecord( + name="test", + level=logging.INFO, + pathname="test_path", + lineno=10, + msg="This is a test message", + args=(), + exc_info=None, ) - self.assertFalse("logging.googleapi.com/trace" in logger.log_fields) - - @patch("builtins.print") - def test_log_error(self, mock_print) -> None: - """Test log_error method""" - logger = Logger("component1", "application1") - logger.log_error("An error occurred") - - expected_output = LogEntry( - severity="ERROR", - message="An error occurred", - component="component1", - labels={"io.kyma.component": "application1"}, + formatted_log = self.formatter.format(log_record) + expected_log = { + "timestamp": log_record.created, + "severity": "INFO", + "message": "This is a test message", + } + self.assertEqual(json.loads(formatted_log), expected_log) + + +class TestCreateLogger(unittest.TestCase): + """Tests for logger factory""" + + def setUp(self): + # Ensure that each test has logger with unique name + self.component_name = f"test_component_{time.time()}" + self.application_name = "test_application" + self.project_id = "test_project_id" + self.request = Mock(spec=Request) + self.request.headers = {"X-Cloud-Trace-Context": "1234567890/other-info"} + + def test_create_logger(self): + """Tests create logger with trace""" + + logger: logging.Logger = create_logger( + component_name=self.component_name, + application_name=self.application_name, + project_id=self.project_id, + request=self.request, + log_level=logging.INFO, ) - mock_print.assert_called_once_with(expected_output) + self.assertFalse(logger.isEnabledFor(logging.DEBUG)) - @patch("builtins.print") - def test_log_info(self, mock_print) -> None: - """Test log_info method""" - logger = Logger("component1", "application1") - logger.log_info("Information message") + self.assertTrue(logger.hasHandlers()) - expected_output = LogEntry( - severity="INFO", - message="Information message", - component="component1", - labels={"io.kyma.component": "application1"}, - ) + handler = logger.handlers[0] + self.assertIsInstance(handler.formatter, GoogleCloudFormatter) - mock_print.assert_called_once_with(expected_output) + expected_log_fields: dict[str, Any] = { + "component": self.component_name, + "labels": {"io.kyma.component": self.application_name}, + "logging.googleapi.com/trace": f"projects/{self.project_id}/traces/1234567890", + } + self.assertDictEqual(expected_log_fields, handler.formatter.log_fields) - def test_log_entry_str(self) -> None: - """Test log entry to json""" - log_entry = LogEntry( - severity="INFO", message="Test", component="test_component" + def test_create_logger_without_trace(self): + """Tests create logger without request""" + logger: logging.Logger = create_logger( + component_name=self.component_name, + application_name=self.application_name, + log_level=logging.INFO, ) - expected_output = json.dumps( - {"severity": "INFO", "message": "Test", "component": "test_component"} - ) - self.assertEqual(str(log_entry), expected_output) + + self.assertFalse(logger.isEnabledFor(logging.DEBUG)) + self.assertTrue(logger.hasHandlers()) + + handler = logger.handlers[0] + self.assertIsInstance(handler.formatter, GoogleCloudFormatter) + + expected_log_fields: dict[str, Any] = { + "component": self.component_name, + "labels": {"io.kyma.component": self.application_name}, + } + self.assertDictEqual(expected_log_fields, handler.formatter.log_fields) if __name__ == "__main__": diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index f54c1ef5128d..d09ec2b087e6 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -1,6 +1,7 @@ """Simple PubSub handler to rotate signify certificates""" import datetime +import logging import os import base64 import json @@ -14,7 +15,7 @@ from cryptography.hazmat.primitives.serialization import Encoding, PrivateFormat from requests import HTTPError from secretmanager import client -from pylogger.logger import Logger +from pylogger.logger import create_logger from signify.client import SignifyClient app = Flask(__name__) @@ -28,9 +29,10 @@ @app.route("/", methods=["POST"]) def rotate_signify_secret() -> Response: """HTTP webhook handler for rotating Signify secrets.""" - logger = Logger( + logger = create_logger( component_name=component_name, application_name=application_name, + project_id=project_id, request=request, ) @@ -62,7 +64,8 @@ def rotate_signify_secret() -> Response: new_private_key: rsa.RSAPrivateKey = rsa.generate_private_key( # Public exponent is standarised as 65537 - # see: https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#cryptography.hazmat.primitives.asymmetric.rsa.generate_private_key + # see: + # https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#cryptography.hazmat.primitives.asymmetric.rsa.generate_private_key public_exponent=65537, key_size=rsa_key_size, ) @@ -167,7 +170,7 @@ def get_pubsub_message() -> Dict[str, Any]: # TODO(kacpermalachowski): Move it to common package -def prepare_error_response(err: str, logger: Logger) -> Response: +def prepare_error_response(err: str, logger: logging.Logger) -> Response: """Prepares an error response with logging.""" _, exc_value, _ = sys.exc_info() stacktrace = repr(traceback.format_exception(exc_value)) From 3593e648af11f5851a0d2b1f57dbe97f7cd42b57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Tue, 24 Sep 2024 09:33:09 +0200 Subject: [PATCH 23/24] Add error handler in sm client, rename set_secret to add_secret_version --- .../secretmanager/client.py | 32 +++++++++++++------ .../secretmanager/test_client.py | 4 +-- .../signifysecretrotator.py | 8 ++--- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py index cb388f190ec0..348bd3197699 100644 --- a/cmd/cloud-run/signifysecretrotator/secretmanager/client.py +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/client.py @@ -3,6 +3,7 @@ import json from typing import Any, Dict from google.cloud import secretmanager +from google.api_core.exceptions import GoogleAPIError class SecretManagerClient: @@ -30,17 +31,20 @@ def get_secret( secret_name = f"{secret_id}/versions/{secret_version}" - response: secretmanager.AccessSecretVersionResponse = ( - self.client.access_secret_version(name=secret_name) - ) - secret_value = response.payload.data.decode() + try: + response: secretmanager.AccessSecretVersionResponse = ( + self.client.access_secret_version(name=secret_name) + ) + secret_value = response.payload.data.decode() - if is_json: - return json.loads(secret_value) + if is_json: + return json.loads(secret_value) - return secret_value + return secret_value + except GoogleAPIError as e: + raise SecretManagerError(secret_id, e) from e - def set_secret(self, secret_id: str, data: str) -> None: + def add_secret_version(self, secret_id: str, data: str) -> None: """Adds new secret version with given data Args: @@ -49,4 +53,14 @@ def set_secret(self, secret_id: str, data: str) -> None: """ payload = {"data": data.encode()} - self.client.add_secret_version(parent=secret_id, payload=payload) + try: + self.client.add_secret_version(parent=secret_id, payload=payload) + except GoogleAPIError as e: + raise SecretManagerError(secret_id, e) from e + + +class SecretManagerError(Exception): + """Common class for Secret Manager client exceptions""" + + def __init__(self, secret_id: str, e: Exception) -> None: + self.add_note(f"Failed to access secret {secret_id}, error: {e}") diff --git a/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py b/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py index 623a4c525007..e15f0f9601c1 100644 --- a/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py +++ b/cmd/cloud-run/signifysecretrotator/secretmanager/test_client.py @@ -67,14 +67,14 @@ def test_get_secret_plain_string(self) -> None: secret_name="projects/test-project/secrets/test-secret/versions/latest" ) - def test_set_secret(self) -> None: + def test_add_secret_version(self) -> None: """Tests setting a new secret version""" # Arrange secret_id = "projects/test-project/secrets/test-secret" secret_data = "new-secret-value" # Act - self.client.set_secret(secret_id, secret_data) + self.client.add_secret_version(secret_id, secret_data) # Assert payload: dict[str, bytes] = {"data": secret_data.encode()} diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index d09ec2b087e6..86d4d6e46d9e 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -14,7 +14,7 @@ from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.serialization import Encoding, PrivateFormat from requests import HTTPError -from secretmanager import client +from secretmanager.client import SecretManagerClient, SecretManagerError from pylogger.logger import create_logger from signify.client import SignifyClient @@ -37,7 +37,7 @@ def rotate_signify_secret() -> Response: ) try: - sm_client = client.SecretManagerClient() + sm_client = SecretManagerClient() pubsub_message: Dict[str, Any] = get_pubsub_message() @@ -87,12 +87,12 @@ def rotate_signify_secret() -> Response: new_certs, new_private_key, secret_data, created_at ) - sm_client.set_secret(secret_id, json.dumps(new_secret_data)) + sm_client.add_secret_version(secret_id, json.dumps(new_secret_data)) logger.log_info(f"Certificate rotated successfully at {created_at}") return "Certificate rotated successfully" - except (HTTPError, ValueError, TypeError) as exc: + except (HTTPError, ValueError, TypeError, SecretManagerError) as exc: return prepare_error_response(exc, logger) From 2dd71cdeb22848c55f166f8eccb037f8b169d876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Tue, 24 Sep 2024 10:14:30 +0200 Subject: [PATCH 24/24] Fix logger issues --- cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py | 2 +- cmd/cloud-run/signifysecretrotator/signify/client.py | 1 - cmd/cloud-run/signifysecretrotator/signifysecretrotator.py | 7 ++----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py index 21b100dc0796..8166ad92f41c 100644 --- a/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py +++ b/cmd/cloud-run/signifysecretrotator/pylogger/test_logger.py @@ -5,7 +5,7 @@ import unittest import logging import json -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock from flask import Request diff --git a/cmd/cloud-run/signifysecretrotator/signify/client.py b/cmd/cloud-run/signifysecretrotator/signify/client.py index 97ae6c25dbdc..eeff279a606c 100644 --- a/cmd/cloud-run/signifysecretrotator/signify/client.py +++ b/cmd/cloud-run/signifysecretrotator/signify/client.py @@ -5,7 +5,6 @@ import json import tempfile from typing import Any -from urllib import response import requests from cryptography.hazmat.primitives.asymmetric import rsa from cryptography import x509 diff --git a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py index 86d4d6e46d9e..177de4a0c570 100644 --- a/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py +++ b/cmd/cloud-run/signifysecretrotator/signifysecretrotator.py @@ -6,7 +6,6 @@ import base64 import json import sys -import traceback from typing import Any, Dict, List from flask import Flask, Response, request, make_response from cryptography import x509 @@ -89,7 +88,7 @@ def rotate_signify_secret() -> Response: sm_client.add_secret_version(secret_id, json.dumps(new_secret_data)) - logger.log_info(f"Certificate rotated successfully at {created_at}") + logger.info("Certificate rotated successfully at %s", created_at) return "Certificate rotated successfully" except (HTTPError, ValueError, TypeError, SecretManagerError) as exc: @@ -172,9 +171,7 @@ def get_pubsub_message() -> Dict[str, Any]: # TODO(kacpermalachowski): Move it to common package def prepare_error_response(err: str, logger: logging.Logger) -> Response: """Prepares an error response with logging.""" - _, exc_value, _ = sys.exc_info() - stacktrace = repr(traceback.format_exception(exc_value)) - logger.log_error(f"Error: {err}\nStack:\n {stacktrace}") + logger.error(err, exc_info=sys.exc_info()) resp = make_response() resp.content_type = "application/json" resp.status_code = 500