Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent default labels from being forwarded #31

Merged
merged 6 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src-docs/app.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Configure the application.

---

<a href="../webhook_router/app.py#L135"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/app.py#L134"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `health_check`

Expand All @@ -50,7 +50,7 @@ Health check endpoint.

---

<a href="../webhook_router/app.py#L147"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/app.py#L146"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `handle_github_webhook`

Expand Down
5 changes: 3 additions & 2 deletions src-docs/parse.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ Module for parsing the webhook payload.

---

<a href="../webhook_router/parse.py#L50"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/parse.py#L52"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `webhook_to_job`

```python
webhook_to_job(webhook: dict) → Job
webhook_to_job(webhook: dict, ignore_labels: Collection[str]) → Job
```

Parse a raw json payload and extract the required information.
Expand All @@ -26,6 +26,7 @@ Parse a raw json payload and extract the required information.
**Args:**

- <b>`webhook`</b>: The webhook in json to parse.
- <b>`ignore_labels`</b>: The labels to ignore when parsing. For example, "self-hosted" or "linux".



Expand Down
9 changes: 3 additions & 6 deletions src-docs/router.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ Module for routing webhooks to the appropriate message queue.

---

<a href="../webhook_router/router.py#L41"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/router.py#L39"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `to_routing_table`

```python
to_routing_table(
flavor_label_mapping_list: list[tuple[str, list[str]]],
ignore_labels: set[str],
default_flavor: str
) → RoutingTable
```
Expand All @@ -30,7 +29,6 @@ Convert the flavor label mapping to a route table.
**Args:**

- <b>`flavor_label_mapping_list`</b>: The list of mappings of flavors to labels.
- <b>`ignore_labels`</b>: The labels to ignore (e.g. "self-hosted" or "linux").
- <b>`default_flavor`</b>: The default flavor to use if no labels are provided.


Expand All @@ -41,7 +39,7 @@ Convert the flavor label mapping to a route table.

---

<a href="../webhook_router/router.py#L97"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/router.py#L91"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `forward`

Expand All @@ -67,7 +65,7 @@ Forward the job to the appropriate message queue.

---

<a href="../webhook_router/router.py#L123"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/router.py#L117"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `can_forward`

Expand Down Expand Up @@ -102,7 +100,6 @@ A class to represent how to route jobs to the appropriate message queue.
**Attributes:**

- <b>`value`</b>: The mapping of labels to flavors.
- <b>`ignore_labels`</b>: The labels to ignore (e.g. "self-hosted" or "linux").
- <b>`default_flavor`</b>: The default flavor.


Expand Down
7 changes: 4 additions & 3 deletions tests/unit/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from webhook_router.router import RouterError, RoutingTable

TEST_PATH = "/webhook"
TEST_LABELS = ["self-hosted", "linux", "arm64"]
DEFAULT_SELF_HOSTED_LABELS = {"self-hosted", "linux"}


@pytest.fixture(name="flavours_yaml")
Expand Down Expand Up @@ -55,7 +57,6 @@ def route_table_fixture() -> RoutingTable:
("small", "x64"): "small",
},
default_flavor="small",
ignore_labels={"self-hosted", "linux"},
)


Expand Down Expand Up @@ -109,7 +110,7 @@ def test_webhook_logs(
"""
data = _create_valid_data(JobStatus.QUEUED)
expected_job = Job(
labels=data["workflow_job"]["labels"],
labels=set(TEST_LABELS) - DEFAULT_SELF_HOSTED_LABELS,
status=JobStatus.QUEUED,
url=data["workflow_job"]["url"],
)
Expand Down Expand Up @@ -392,7 +393,7 @@ def _create_valid_data(action: str) -> dict:
"run_id": 987654321,
"status": "completed",
"conclusion": "success",
"labels": ["self-hosted", "linux", "arm64"],
"labels": TEST_LABELS,
"url": "https://api.github.com/repos/f/actions/jobs/8200803099",
},
}
77 changes: 68 additions & 9 deletions tests/unit/test_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@

import pytest

from webhook_router.parse import Job, JobStatus, ParseError, webhook_to_job
from webhook_router.parse import Job, JobStatus, Labels, ParseError, webhook_to_job

FAKE_JOB_URL = "https://api.github.com/repos/fakeusergh-runner-test/actions/jobs/8200803099"
FAKE_LABELS = ["self-hosted", "linux", "arm64"]
DEFAULT_SELF_HOSTED_LABELS = {"self-hosted", "linux"}


@pytest.mark.parametrize(
Expand Down Expand Up @@ -56,11 +57,15 @@ def test_webhook_to_job(labels: list[str], status: JobStatus):
},
}

result = webhook_to_job(payload)
result = webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)

# 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=set(labels) - DEFAULT_SELF_HOSTED_LABELS,
status=status,
url=FAKE_JOB_URL, # type: ignore
)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -88,7 +93,7 @@ def test_webhook_invalid_values(labels: list[str], status: JobStatus, url: str):
"workflow_job": {"id": 22428484402, "url": url, "labels": labels},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)
assert "Failed to create Webhook object for webhook " in str(exc_info.value)


Expand All @@ -103,7 +108,7 @@ def test_webhook_workflow_job_not_dict():
"workflow_job": "not a dict",
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)
assert f"workflow_job is not a dict in {payload}" in str(exc_info.value)


Expand All @@ -122,7 +127,7 @@ def test_webhook_missing_keys():
},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)
assert f"action key not found in {payload}" in str(exc_info.value)
# workflow_job key missing
payload = {
Expand All @@ -132,7 +137,7 @@ def test_webhook_missing_keys():
"labels": FAKE_LABELS,
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)
assert f"workflow_job key not found in {payload}" in str(exc_info.value)

# labels key missing
Expand All @@ -141,7 +146,7 @@ def test_webhook_missing_keys():
"workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)
assert f"labels key not found in {payload}" in str(exc_info.value)

# url key missing
Expand All @@ -150,5 +155,59 @@ def test_webhook_missing_keys():
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
webhook_to_job(payload, DEFAULT_SELF_HOSTED_LABELS)
assert f"url key not found in {payload}" in str(exc_info.value)


@pytest.mark.parametrize(
"labels, ignore_labels",
[
pytest.param(["self-hosted", "linux", "arm64"], set(), id="empty"),
pytest.param(["ubuntu-latest"], {"self-hosted"}, id="non overlapping"),
pytest.param(["self-hosted", "linux", "amd"], {"self-hosted", "linux"}, id="overlapping"),
cbartz marked this conversation as resolved.
Show resolved Hide resolved
pytest.param(["self-hosted", "linux"], {"self-hosted", "linux"}, id="equal"),
pytest.param(["self-hosted", "linux"], {"self-hosted", "linux", "arm64"}, id="superset"),
],
)
def test_ignore_labels(labels: list[str], ignore_labels: Labels):
"""
arrange: A valid payload dict with different combinations of ignore labels.
act: Call webhook_to_job with the payload.
assert: The ignore labels are removed from the labels.
"""
payload = {
"action": JobStatus.QUEUED,
"workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL, "labels": labels},
}
result = webhook_to_job(payload, ignore_labels)

# mypy does not understand that we can pass strings instead of HttpUrl objects
# because of the underlying pydantic magic
assert result == Job(
labels=set(labels) - ignore_labels,
cbartz marked this conversation as resolved.
Show resolved Hide resolved
status=JobStatus.QUEUED,
url=FAKE_JOB_URL, # type: ignore
)
cbartz marked this conversation as resolved.
Show resolved Hide resolved


def test_labels_are_parsed_in_lowercase():
"""
arrange: A valid payload dict with labels in mixed_case.
act: Call webhook_to_job with the payload.
assert: The labels are parsed and compared with the ignore labels in lowercase.
"""
labels = ["Self-Hosted", "Linux", "ARM64"]
ignore_labels = {"self-Hosted", "linUX"}
payload = {
"action": JobStatus.QUEUED,
"workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL, "labels": labels},
}
result = webhook_to_job(payload, ignore_labels)

# mypy does not understand that we can pass strings instead of HttpUrl objects
# because of the underlying pydantic magic
assert result == Job(
labels={"arm64"},
status=JobStatus.QUEUED,
url=FAKE_JOB_URL, # type: ignore
)
Loading
Loading