Prevent default labels from being forwarded

src-docs/app.py.md

@@ -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`
 
@@ -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`
 

src-docs/parse.py.md

@@ -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(payload: dict, ignore_labels: Collection[str]) → Job
 ```
 
 Parse a raw json payload and extract the required information. 
@@ -25,7 +25,8 @@ Parse a raw json payload and extract the required information.
 
 **Args:**
 
- - <b>`webhook`</b>:  The webhook in json to parse. 
+ - <b>`payload`</b>:  The webhook's payload in json to parse. 
+ - <b>`ignore_labels`</b>:  The labels to ignore when parsing. For example, "self-hosted" or "linux". 
 
 
 

src-docs/router.py.md

@@ -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
 ```
@@ -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. 
 
 
@@ -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`
 
@@ -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`
 
@@ -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. 
 
 

tests/integration/test_app.py

@@ -100,14 +100,14 @@ async def test_forward_webhook(  # pylint: disable=too-many-locals
     }
 
     for flavour in flavours:
-        actual_jobs = jobs_by_flavour.get(flavour, set())
+        actual_jobs = jobs_by_flavour.get(flavour, [])
         expected_jobs_for_flavour = expected_jobs_by_flavour[flavour]
         assert len(actual_jobs) == len(
             expected_jobs_for_flavour
         ), f"Expected: {expected_jobs_for_flavour}, Actual: {actual_jobs}"
         for job in expected_jobs_for_flavour:
             assert (
-                job.json() in actual_jobs
+                job in actual_jobs
             ), f"Expected: {expected_jobs_for_flavour}, Actual: {actual_jobs}"
 
 
@@ -293,7 +293,9 @@ def _request(payload: dict, webhook_secret: Optional[str], base_url: str) -> req
     return requests.post(f"{base_url}/webhook", data=payload_bytes, headers=headers, timeout=1)
 
 
-async def _get_jobs_from_mq(ops_test: OpsTest, unit: Unit, flavors: list[str]) -> dict[str, set]:
+async def _get_jobs_from_mq(
+    ops_test: OpsTest, unit: Unit, flavors: list[str]
+) -> dict[str, list[Job]]:
     """Get the gunicorn log from the charm unit.
 
     Args:
@@ -349,7 +351,11 @@ async def _get_jobs_from_mq(ops_test: OpsTest, unit: Unit, flavors: list[str]) -
         "ssh", "--container", "flask-app", unit.name, "python3", "/kombu_script.py"
     )
     assert code == 0, f"Failed to execute kombu script: {stderr}"
-    return json.loads(stdout)
+    jobs_raw_by_flavor = json.loads(stdout)
+    jobs_by_flavor = {}
+    for flavor, jobs_raw in jobs_raw_by_flavor.items():
+        jobs_by_flavor[flavor] = [Job.model_validate_json(job_raw) for job_raw in jobs_raw]
+    return jobs_by_flavor
 
 
 async def _get_mongodb_uri_from_integration_data(ops_test: OpsTest, unit: Unit) -> str | None:

tests/unit/test_app.py

@@ -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")
@@ -55,7 +57,6 @@ def route_table_fixture() -> RoutingTable:
             ("small", "x64"): "small",
         },
         default_flavor="small",
-        ignore_labels={"self-hosted", "linux"},
     )
 
 
@@ -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"],
     )
@@ -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",
         },
     }

tests/unit/test_parse.py

@@ -2,13 +2,15 @@
 #  See LICENSE file for licensing details.
 
 """Unit tests for the parse module."""
+from itertools import permutations
 
 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(
@@ -56,11 +58,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(
@@ -88,7 +94,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)
 
 
@@ -103,7 +109,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)
 
 
@@ -122,7 +128,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 = {
@@ -132,7 +138,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
@@ -141,7 +147,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
@@ -150,5 +156,62 @@ 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,expected_labels",
+    [
+        pytest.param(
+            ["self-hosted", "linux", "arm64"], set(), {"self-hosted", "linux", "arm64"}, id="empty"
+        ),
+        pytest.param(["self-hosted", "linux"], {"self-hosted", "linux"}, set(), id="equal"),
+        pytest.param(
+            ["self-hosted", "linux"], {"self-hosted", "linux", "arm64"}, set(), id="superset"
+        ),
+        pytest.param(["ubuntu-latest"], {"self-hosted"}, {"ubuntu-latest"}, id="non overlapping"),
+        pytest.param(
+            ["self-hosted", "linux", "amd"], {"self-hosted", "linux"}, {"amd"}, id="overlapping"
+        ),
+    ],
+)
+def test_ignore_labels(labels: list[str], ignore_labels: Labels, expected_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.
+    """
+    # check all permutations of labels to check if the ignore labels are removed correctly
+    # in order to check for bugs around ignoring certain elements
+    for label_permutation in permutations(labels):
+        payload = {
+            "action": JobStatus.QUEUED,
+            "workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL, "labels": label_permutation},
+        }
+        result = webhook_to_job(payload, ignore_labels)
+
+        assert result.labels == expected_labels
+
+
+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
+    )