diff --git a/src-docs/parse.py.md b/src-docs/parse.py.md index ce6f3a3..2f255d0 100644 --- a/src-docs/parse.py.md +++ b/src-docs/parse.py.md @@ -11,7 +11,7 @@ Module for parsing the webhook payload. --- - + ## function `webhook_to_job` @@ -39,6 +39,44 @@ Parse a raw json payload and extract the required information. - `ParseError`: An error occurred during parsing. +--- + +## class `GitHubRepo` +A class to represent the GitHub repository. + + + +**Attributes:** + + - `owner`: The owner of the repository. + - `name`: The name of the repository. + + +--- + +#### property model_extra + +Get extra fields set during validation. + + + +**Returns:** + A dictionary of extra fields, or `None` if `config.extra` is not set to `"allow"`. + +--- + +#### property model_fields_set + +Returns the set of fields that have been explicitly set on this model instance. + + + +**Returns:** + A set of strings representing the fields that have been set, i.e. that were not filled from defaults. + + + + --- ## class `Job` @@ -50,7 +88,8 @@ A class to translate the payload. - `labels`: The labels of the job. - `status`: The status of the job. - - `url`: The URL of the job to be able to check its status. + - `repository`: The repository of the job. + - `id`: The id of the job. --- diff --git a/tests/integration/test_app.py b/tests/integration/test_app.py index d42a4ee..fc329f6 100644 --- a/tests/integration/test_app.py +++ b/tests/integration/test_app.py @@ -22,7 +22,7 @@ SUPPORTED_GITHUB_EVENT, WEBHOOK_SIGNATURE_HEADER, ) -from webhook_router.parse import Job, JobStatus +from webhook_router.parse import GitHubRepo, Job, JobStatus PORT = 8000 @@ -91,8 +91,12 @@ async def test_forward_webhook( # pylint: disable=too-many-locals flavour: [ Job( status=payload["action"], - url=payload["workflow_job"]["url"], + id=payload["workflow_job"]["id"], labels=payload["workflow_job"]["labels"], + repository=GitHubRepo( + name=payload["repository"]["name"], + owner=payload["repository"]["owner"]["login"], + ), ) for payload in payloads_by_flavour[flavour] ] @@ -237,6 +241,13 @@ def _create_valid_data(action: str, labels: list[str]) -> dict: "labels": labels, "url": f"https://api.github.com/repos/f/actions/jobs/{_id}", }, + "repository": { + "id": 123456789, + "name": "gh-runner-test", + "full_name": "f/gh-runner-test", + "private": False, + "owner": {"login": "f", "id": 123456}, + }, } diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index 084cf48..0a5b995 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -16,7 +16,7 @@ import webhook_router.app as app_module import webhook_router.router from tests.unit.helpers import create_correct_signature, create_incorrect_signature -from webhook_router.parse import Job, JobStatus, ParseError +from webhook_router.parse import GitHubRepo, Job, JobStatus, ParseError from webhook_router.router import RouterError, RoutingTable TEST_PATH = "/webhook" @@ -111,7 +111,10 @@ def test_webhook_logs( expected_job = Job( labels=data["workflow_job"]["labels"], status=JobStatus.QUEUED, - url=data["workflow_job"]["url"], + id=data["workflow_job"]["id"], + repository=GitHubRepo( + owner=data["repository"]["owner"]["login"], name=data["repository"]["name"] + ), ) response = client.post( TEST_PATH, @@ -395,4 +398,8 @@ def _create_valid_data(action: str) -> dict: "labels": ["self-hosted", "linux", "arm64"], "url": "https://api.github.com/repos/f/actions/jobs/8200803099", }, + "repository": { + "name": "actions", + "owner": {"login": "f"}, + }, } diff --git a/tests/unit/test_mq.py b/tests/unit/test_mq.py index 67bb875..052fce1 100644 --- a/tests/unit/test_mq.py +++ b/tests/unit/test_mq.py @@ -10,7 +10,7 @@ from kombu.exceptions import OperationalError from webhook_router import mq -from webhook_router.parse import Job, JobStatus +from webhook_router.parse import GitHubRepo, Job, JobStatus IN_MEMORY_URI = "memory://" @@ -30,8 +30,12 @@ def test_add_job_to_queue(): """ flavor = secrets.token_hex(16) labels = [secrets.token_hex(16), secrets.token_hex(16)] - # mypy: does not recognize that url can be passed as a string - job = Job(labels=labels, status=JobStatus.QUEUED, url="http://example.com") # type: ignore + job = Job( + labels=labels, + status=JobStatus.QUEUED, + repository=GitHubRepo(owner="fake", name="gh-runner"), + id=1234, + ) mq.add_job_to_queue(job, flavor) with Connection(IN_MEMORY_URI) as conn: diff --git a/tests/unit/test_parse.py b/tests/unit/test_parse.py index cc3d9a1..56b485f 100644 --- a/tests/unit/test_parse.py +++ b/tests/unit/test_parse.py @@ -5,10 +5,13 @@ import pytest -from webhook_router.parse import Job, JobStatus, ParseError, webhook_to_job +from webhook_router.parse import GitHubRepo, Job, JobStatus, ParseError, webhook_to_job FAKE_JOB_URL = "https://api.github.com/repos/fakeusergh-runner-test/actions/jobs/8200803099" FAKE_LABELS = ["self-hosted", "linux", "arm64"] +FAKE_JOB_ID = 22428484402 +FAKE_OWNER = "fake" +FAKE_REPO_NAME = "gh-runner-test" @pytest.mark.parametrize( @@ -30,7 +33,7 @@ def test_webhook_to_job(labels: list[str], status: JobStatus): payload = { "action": status, "workflow_job": { - "id": 22428484402, + "id": FAKE_JOB_ID, "run_id": 8200803099, "workflow_name": "Push Event Tests", "head_branch": "github-hosted", @@ -54,30 +57,37 @@ def test_webhook_to_job(labels: list[str], status: JobStatus): "runner_group_id": None, "runner_group_name": None, }, + "repository": { + "id": 688069339, + "name": "gh-runner-test", + "full_name": f"{FAKE_OWNER}/{FAKE_REPO_NAME}", + "private": False, + "owner": { + "login": FAKE_OWNER, + "id": 4183921, + }, + }, } result = webhook_to_job(payload) - # mypy does not understand that we can pass strings instead of HttpUrl objects - # because of the underlying pydantic magic - assert result == Job(labels=labels, status=status, url=FAKE_JOB_URL) # type: ignore + assert result == Job( + labels=labels, + status=status, + id=FAKE_JOB_ID, + repository=GitHubRepo(owner=FAKE_OWNER, name=FAKE_REPO_NAME), + ) @pytest.mark.parametrize( - "labels, status, url", + "labels, status", [ - pytest.param( - ["self-hosted", "linux", "arm64"], "invalid", FAKE_JOB_URL, id="invalid status" - ), - pytest.param(["ubuntu-latest"], JobStatus.IN_PROGRESS, "invalid", id="invalid run url"), - pytest.param(None, JobStatus.IN_PROGRESS, FAKE_JOB_URL, id="missing labels"), - pytest.param(["self-hosted", "linux", "amd"], None, FAKE_JOB_URL, id="missing status"), - pytest.param( - ["self-hosted", "linux", "amd"], JobStatus.COMPLETED, None, id="missing run url" - ), + pytest.param(["self-hosted", "linux", "arm64"], "invalid", id="invalid status"), + pytest.param(None, JobStatus.IN_PROGRESS, id="missing labels"), + pytest.param(["self-hosted", "linux", "amd"], None, id="missing status"), ], ) -def test_webhook_invalid_values(labels: list[str], status: JobStatus, url: str): +def test_webhook_invalid_values(labels: list[str], status: JobStatus): """ arrange: A payload dict with invalid values. act: Call webhook_to_job with the payload. @@ -85,7 +95,8 @@ def test_webhook_invalid_values(labels: list[str], status: JobStatus, url: str): """ payload = { "action": status, - "workflow_job": {"id": 22428484402, "url": url, "labels": labels}, + "workflow_job": {"id": 22428484402, "labels": labels}, + "repository": {"name": "gh-runner-test", "owner": {"login": "fakeuser"}}, } with pytest.raises(ParseError) as exc_info: webhook_to_job(payload) @@ -100,13 +111,46 @@ def test_webhook_workflow_job_not_dict(): """ payload = { "action": "queued", - "workflow_job": "not a dict", + "workflow_job": "labels id", + "repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_REPO_NAME}}, } with pytest.raises(ParseError) as exc_info: webhook_to_job(payload) assert f"workflow_job is not a dict in {payload}" in str(exc_info.value) +def test_webhook_repository_not_dict(): + """ + arrange: A payload dict with repository not a dict. + act: Call webhook_to_job with the payload. + assert: A ParseError is raised. + """ + payload = { + "action": "queued", + "workflow_job": {"id": 22428484402, "labels": FAKE_LABELS}, + "repository": "name owner login", + } + with pytest.raises(ParseError) as exc_info: + webhook_to_job(payload) + assert f"repository is not a dict in {payload}" in str(exc_info.value) + + +def test_webhook_repository_owner_not_dict(): + """ + arrange: A payload dict with repository owner not a dict. + act: Call webhook_to_job with the payload. + assert: A ParseError is raised. + """ + payload = { + "action": "queued", + "workflow_job": {"id": 22428484402, "labels": FAKE_LABELS}, + "repository": {"name": "gh-runner-test", "owner": "not a dict"}, + } + with pytest.raises(ParseError) as exc_info: + webhook_to_job(payload) + assert f"owner is not a dict in {payload['repository']}" in str(exc_info.value) + + def test_webhook_missing_keys(): """ arrange: A payload dict with missing keys. @@ -119,6 +163,7 @@ def test_webhook_missing_keys(): payload = { "payload": { "workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL, "labels": FAKE_LABELS}, + "repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_OWNER}}, }, } with pytest.raises(ParseError) as exc_info: @@ -138,17 +183,59 @@ def test_webhook_missing_keys(): # labels key missing payload = { "action": "queued", - "workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL}, + "workflow_job": {"id": 22428484402}, + "repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_OWNER}}, + } + with pytest.raises(ParseError) as exc_info: + webhook_to_job(payload) + assert f"labels key not found in {payload['workflow_job']}" in str(exc_info.value) + + # id key missing + payload = { + "action": "queued", + "workflow_job": {"labels": FAKE_LABELS}, + "repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_OWNER}}, + } + with pytest.raises(ParseError) as exc_info: + webhook_to_job(payload) + assert f"id key not found in {payload['workflow_job']}" in str(exc_info.value) + + # repository key missing + payload = { + "action": "queued", + "workflow_job": {"id": 22428484402, "labels": FAKE_LABELS}, + } + with pytest.raises(ParseError) as exc_info: + webhook_to_job(payload) + assert f"repository key not found in {payload}" in str(exc_info.value) + + # owner key missing + payload = { + "action": "queued", + "workflow_job": {"id": 22428484402, "labels": FAKE_LABELS}, + "repository": {"name": FAKE_REPO_NAME}, + } + + with pytest.raises(ParseError) as exc_info: + webhook_to_job(payload) + assert f"owner key not found in {payload['repository']}" in str(exc_info.value) + + # name key missing + payload = { + "action": "queued", + "workflow_job": {"id": 22428484402, "labels": FAKE_LABELS}, + "repository": {"owner": {"login": FAKE_OWNER}}, } with pytest.raises(ParseError) as exc_info: webhook_to_job(payload) - assert f"labels key not found in {payload}" in str(exc_info.value) + assert f"name key not found in {payload['repository']}" in str(exc_info.value) - # url key missing + # login key missing payload = { "action": "queued", "workflow_job": {"id": 22428484402, "labels": FAKE_LABELS}, + "repository": {"name": FAKE_REPO_NAME, "owner": {}}, } with pytest.raises(ParseError) as exc_info: webhook_to_job(payload) - assert f"url key not found in {payload}" in str(exc_info.value) + assert f"login key not found in {payload['repository']['owner']}" in str(exc_info.value) diff --git a/tests/unit/test_router.py b/tests/unit/test_router.py index f3f1566..cd0c288 100644 --- a/tests/unit/test_router.py +++ b/tests/unit/test_router.py @@ -8,7 +8,7 @@ import pytest from webhook_router.mq import add_job_to_queue -from webhook_router.parse import Job, JobStatus +from webhook_router.parse import GitHubRepo, Job, JobStatus from webhook_router.router import ( RouterError, RoutingTable, @@ -47,12 +47,11 @@ def test_job_is_forwarded( act: Forward the job to the message queue. assert: The job is added to the queue if the status is "QUEUED". """ - # mypy does not understand that we can pass strings instead of HttpUrl objects - # because of the underlying pydantic magic job = Job( labels=["arm64"], status=job_status, - url="https://api.github.com/repos/f/actions/jobs/8200803099", # type: ignore + id=22428484402, + repository=GitHubRepo(**{"owner": "fake", "name": "gh-runner-test"}), ) forward( job, @@ -70,12 +69,11 @@ def test_invalid_label_combination(): act: Forward the job to the message queue. assert: A RouterError is raised. """ - # mypy does not understand that we can pass strings instead of HttpUrl objects - # because of the underlying pydantic magic job = Job( labels=["self-hosted", "linux", "arm64", "x64"], status=JobStatus.QUEUED, - url="https://api.github.com/repos/f/actions/jobs/8200803099", # type: ignore + id=22428484402, + repository=GitHubRepo(**{"owner": "fake", "name": "gh-runner-test"}), ) with pytest.raises(RouterError) as e: forward( diff --git a/webhook_router/parse.py b/webhook_router/parse.py index bd6c517..d9fb746 100644 --- a/webhook_router/parse.py +++ b/webhook_router/parse.py @@ -5,7 +5,7 @@ from collections import namedtuple from enum import Enum -from pydantic import BaseModel, HttpUrl +from pydantic import BaseModel WORKFLOW_JOB = "workflow_job" @@ -33,18 +33,32 @@ class JobStatus(str, Enum): WAITING = "waiting" +class GitHubRepo(BaseModel): + """A class to represent the GitHub repository. + + Attributes: + owner: The owner of the repository. + name: The name of the repository. + """ + + owner: str + name: str + + class Job(BaseModel): """A class to translate the payload. Attributes: labels: The labels of the job. status: The status of the job. - url: The URL of the job to be able to check its status. + repository: The repository of the job. + id: The id of the job. """ labels: list[str] status: JobStatus - url: HttpUrl + repository: GitHubRepo + id: int def webhook_to_job(webhook: dict) -> Job: @@ -70,21 +84,35 @@ def webhook_to_job(webhook: dict) -> Job: assert ( # nosec "labels" in webhook["workflow_job"] ), f"labels key not found in {webhook['workflow_job']}" - assert ( # nosec - "url" in webhook["workflow_job"] - ), f"url key not found in {webhook['workflow_job']}" + assert ( + "id" in webhook["workflow_job"] + ), f"id key not found in {webhook['workflow_job']}" # nosec + assert "repository" in webhook, f"repository key not found in {webhook}" # nosec + assert ( + "name" in webhook["repository"] + ), f"name key not found in {webhook['repository']}" # nosec + assert ( + "owner" in webhook["repository"] + ), f"owner key not found in {webhook['repository']}" # nosec + assert ( + "login" in webhook["repository"]["owner"] + ), f"login key not found in {webhook['repository']['owner']}" # nosec status = webhook["action"] workflow_job = webhook["workflow_job"] labels = workflow_job["labels"] - job_url = workflow_job["url"] + repository = GitHubRepo( + owner=webhook["repository"]["owner"]["login"], name=webhook["repository"]["name"] + ) + job_id = workflow_job["id"] try: return Job( labels=labels, status=status, - url=job_url, + repository=repository, + id=job_id, ) except ValueError as exc: raise ParseError(f"Failed to create Webhook object for webhook {webhook}: {exc}") from exc @@ -99,35 +127,44 @@ def _validate_webhook(webhook: dict) -> ValidationResult: Returns: (True, "") if the payload is valid otherwise (False, error_msg) """ - validation_result = _validate_missing_keys(webhook) - if not validation_result.is_valid: - return validation_result - - return ValidationResult(True, "") - - -def _validate_missing_keys(webhook: dict) -> ValidationResult: - """Validate the webhook payload for missing keys. - - Uses short-circuit evaluation to check for missing keys. + key_hierachy = { + "action": {}, + "workflow_job": { + "labels": {}, + "id": {}, + }, + "repository": { + "name": {}, + "owner": { + "login": {}, + }, + }, + } + return _validate_missing_keys(webhook, key_hierachy) + + +def _validate_missing_keys(root: dict, key_hierarchy: dict) -> ValidationResult: + """Validate the payload for missing keys. + + This is a recursive function that will validate the payload for missing keys. Args: - webhook: The webhook payload to validate. + root: The root inside the webhook from which to start the validation. + key_hierarchy: The key hierarchy to validate. Returns: (True, "") if all keys are there otherwise (False,error_msg) if the payload is missing keys. """ - for expected_webhook_key in ("action", "workflow_job"): - if expected_webhook_key not in webhook: - return ValidationResult(False, f"{expected_webhook_key} key not found in {webhook}") + for key, sub_keys in key_hierarchy.items(): + if key not in root: + return ValidationResult(False, f"{key} key not found in {root}") + + if sub_keys: + if not isinstance(root[key], dict): + return ValidationResult(False, f"{key} is not a dict in {root}") + validation_result = _validate_missing_keys(root[key], sub_keys) + if not validation_result.is_valid: + return validation_result - workflow_job = webhook["workflow_job"] - if not isinstance(workflow_job, dict): - return ValidationResult(False, f"workflow_job is not a dict in {webhook}") - for expected_workflow_job_key in ("labels", "url"): - if expected_workflow_job_key not in workflow_job: - return ValidationResult( - False, f"{expected_workflow_job_key} key not found in {webhook}" - ) return ValidationResult(True, "")