From 6b58f0189464c2c3a62b4358661796d9dd7f212d Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 6 Jan 2025 08:31:26 +0100 Subject: [PATCH] feat: Provide an action to redeliver failed webhook deliveries (#40) * WIP checkin public interface * change public interface to use an action * simplify public interface * move logic to the workload * fix charmcraft.yaml * use paas-charm * fix wrong script name * lint * revert tox.ini changes * change since argument to take seconds * revert interface to redeliver * WIP checkin * checkin high-level impl * use private function to abstract github api interaction away * impl private function to redeliver * update perms * add support for org webhooks * add exception handling * add arg parsing * add script to coverage check * add first version of charm action * only forward webhooks with action type queued * only redeliver webhooks with workflow_job event * use constants from outer module * checkin auth details parsing * add -id suffix to secret parms * support str|int for app-id * refactor a bit * fix wrong webhook_origin for repo * WIP checkin integration tests * WIP checkin integration tests * more integration tests * fix workflow file and cancel jobs * add non-secrets args to extra-arguments * lint * lint * split modules in integration test * use pytest_asyncio fixture * rename unit test file * bump juju version * use client id * restructure script * use env vars for secrets in integration test * lint * lint * ignore cve * fix test_app * pass over auth via env * fix/add integration tests * remove sys exit from private fcts * lint and refactor integration tests * ignore bandit warnings * recover coverage and fix none_fields check * provide better action failed message on argument parsing error * refmt * use operator-workflows from main * move arg validation to workload * remove action from required non-null fields * add information for integration test arguments. * pass pydantic validation error msg to github app auth details * add note about juju 3.3 * add tox file for charm code * re-enable self-hosted runners * only set env vars if present * _WebhookDeliveryAttempt -> _WebhookDeliveryAttempts * lint * inline function * use edge runners * use edge runners in integration tests * move inline comment * remove nesting * lint * ignore cve * try rockcraft with latest/stable * Revert "try rockcraft with latest/stable" This reverts commit 5b139d257d8221f5187e95c070b197bd5d16650b. * pin paas-charm 1.1.0 * update docs * fix lint --- .github/workflows/integration_test.yaml | 10 +- .github/workflows/test.yaml | 3 +- .../workflows/webhook_redelivery_test.yaml | 15 + .trivyignore | 2 + CONTRIBUTING.md | 23 +- README.md | 10 + charm/charmcraft.yaml | 56 +++ charm/requirements.txt | 2 +- charm/src/charm.py | 131 +++++- charm/tox.ini | 76 ++++ requirements.txt | 1 + rockcraft.yaml | 1 + src-docs/parse.py.md | 5 +- src-docs/router.py.md | 3 - tests/conftest.py | 31 ++ tests/integration/conftest.py | 104 ++++- tests/integration/test_app.py | 23 +- tests/integration/test_webhook_redelivery.py | 315 ++++++++++++++ tests/unit/test_webhook_redelivery.py | 276 ++++++++++++ tox.ini | 9 +- webhook_redelivery.py | 406 ++++++++++++++++++ webhook_router/parse.py | 3 - webhook_router/router.py | 4 +- 23 files changed, 1474 insertions(+), 35 deletions(-) create mode 100644 .github/workflows/webhook_redelivery_test.yaml create mode 100644 charm/tox.ini create mode 100644 tests/integration/test_webhook_redelivery.py create mode 100644 tests/unit/test_webhook_redelivery.py create mode 100644 webhook_redelivery.py diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 7eaef69..f6cdf77 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -3,14 +3,20 @@ name: Integration tests on: pull_request: + jobs: integration-tests: + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit + with: - juju-channel: 3.1/stable + juju-channel: 3.6/stable channel: 1.28-strict/stable trivy-image-config: "trivy.yaml" - self-hosted-runner: false + self-hosted-runner: true + self-hosted-runner-label: 'edge' rockcraft-channel: latest/edge charmcraft-channel: latest/edge + modules: '["test_app", "test_webhook_redelivery"]' + extra-arguments: --webhook-test-repository cbartz-org/gh-runner-test diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 99e540d..57f7b0b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -8,4 +8,5 @@ jobs: uses: canonical/operator-workflows/.github/workflows/test.yaml@main secrets: inherit with: - self-hosted-runner: false + self-hosted-runner: true + self-hosted-runner-label: 'edge' diff --git a/.github/workflows/webhook_redelivery_test.yaml b/.github/workflows/webhook_redelivery_test.yaml new file mode 100644 index 0000000..0070fc4 --- /dev/null +++ b/.github/workflows/webhook_redelivery_test.yaml @@ -0,0 +1,15 @@ +name: Webhook Redelivery Test +# This workflow will be triggered by the integration test used to test webhook redelivery. +# It is not necessary to be picked up by a runner, we only need to ensure a webhook is triggered. + +on: + workflow_dispatch: + + +jobs: + dispatch-job: + runs-on: ["self-hosted", "invalid-flavor"] # The job is not supposed to take a runner, therefore we use an invalid-flavor + steps: + - name: Hello world + run: | + echo "Hello, world" diff --git a/.trivyignore b/.trivyignore index 950b50f..13291b3 100644 --- a/.trivyignore +++ b/.trivyignore @@ -2,3 +2,5 @@ CVE-2024-34156 # Vulnerability in golang.org/x/crypto introduced by statsd-exporter. We have to wait until it is fixed upstream: https://github.com/prometheus/statsd_exporter/blob/master/go.mod CVE-2024-45337 +# Vulnerability in golang.org/x/net introduced by statsd-exporter. We have to wait until it is fixed upstream: https://github.com/prometheus/statsd_exporter/blob/master/go.mod +CVE-2024-45338 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df7cccb..5566f2a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,6 +9,14 @@ tox devenv -e integration source venv/bin/activate ``` +## Repository structure + +The repository contains the charm code in the `charm` directory and the code for the workload +in the root directory. The charm directory has been built using the +[`paas-charm`](https://juju.is/docs/sdk/12-factor-app-charm) approach and then modified to support +the specific actions of this charm. + + ## Generating src docs for every commit Run the following command: @@ -24,13 +32,24 @@ This project uses `tox` for managing test environments. There are some pre-confi that can be used for linting and formatting code when you're preparing contributions to the charm: ```shell -tox run -e format # update your code according to linting rules +tox run -e fmt # update your code according to linting rules tox run -e lint # code style tox run -e unit # unit tests tox run -e integration # integration tests -tox # runs 'format', 'lint', and 'unit' environments +tox # runs 'fmt', 'lint', and 'unit' environments ``` +The integration tests require additional parameters which can be looked up in the `tests/conftest.py` file. +Some of them have environment variable counterparts (see `tests/integration/conftest.py`), +which can be set instead of passing them as arguments, which is more secure for sensitive data. + +There is also a `tox` root in the `charm` directory, which can be used to lint and format the charm code: + +```shell +cd charm +tox run -e fmt # update your code according to linting rules +tox run -e lint # code style +``` ## Development server diff --git a/README.md b/README.md index 8623d5c..420d1ad 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,16 @@ Change the webhook secret used for webhook validation: juju config github-runner-webhook-router webhook-secret= ``` +In an error scenario, you may want to redeliver failed webhook deliveries. You can use +the `redeliver-failed-webhooks` action to redeliver failed webhook deliveries. The following +example redelivers failed deliveries since last minute for a webhook with ID `516986490` + +```shell +juju add-secret github-token token= # the token needs webhook write permissions +# output is: secret:ctik2gfmp25c7648t7j0 +juju run-action github-runner-webhook-router/0 redeliver-failed-webhook github-path=canonical/github-runner-webhook-router webhook-id=516986490 since=60 github-token-secret-id=ctik2gfmp25c7648t7j0 +``` + ### Integrations The charm requires an integration with MongoDB (either the [machine](https://charmhub.io/mongodb) diff --git a/charm/charmcraft.yaml b/charm/charmcraft.yaml index ee6f7f0..d276253 100644 --- a/charm/charmcraft.yaml +++ b/charm/charmcraft.yaml @@ -30,6 +30,61 @@ requires: limit: 1 +actions: + redeliver-failed-webhooks: + description: >- + Redeliver failed webhook deliveries since a certain time period. This action fetches the + Github api for failed deliveries and triggers redelivery. Note that the amount of + webhook deliveries that will be redelivered can be quite large and the requests are counted + against the rate limit of the Github API. The action returns the amount of webhooks + that were redelivered. + Note that this action requires juju user secrets, which have been available since juju 3.3. + params: + since: + description: "The amount of seconds to look back for failed deliveries." + type: integer + github-path: + description: >- + The path of the organisation or repository where the webhooks are registered. Should + be in the format of or /. + type: string + webhook-id: + description: "The id of the webhook to redeliver." + type: integer + github-app-client-id: + description: >- + The client ID of the GitHub App to use for communication with GitHub, + If provided, the other github-app-* params must also be provided. + The Github App needs to have write permission for Webhooks. + Either this or the github-token must be provided. + type: string + github-app-installation-id: + description: >- + The app installation id of the GitHub App to use for communication with GitHub. + If provided, the other github-app-* params must also be provided. + The Github App needs to have write permission for Webhooks. + Either this or the github-token must be provided. + type: integer + github-app-private-key-secret-id: + description: >- + The juju user secret id of the private key to use for communication with GitHub. The + key has to be provided in a field named 'private-key' in the secret. + If provided, the other github-app-* params must also be provided. + The Github App needs to have write permission for Webhooks. + Either this or the github-token must be provided. + type: string + github-token-secret-id: + description: >- + The juju user secret id of the token to use for communication with GitHub.The + token has to be provided in a field named 'token' in the secret. + This can be a PAT with write admin:repo_hook or a fine-grained token with write permission for Webhooks. + Either this or the GitHub App configuration must be provided. + type: string + required: + - since + - github-path + - webhook-id + config: options: default-flavour: @@ -52,6 +107,7 @@ config: If a job matches multiple flavours, the first flavour matching defined in this configuration will be used. Note that labels are treated case-insensitive. required: true + log-level: type: string description: "The log level to use for the application logs. Use any of: CRITICAL, ERROR, WARNING, INFO, DEBUG, NOTSET" diff --git a/charm/requirements.txt b/charm/requirements.txt index daf2d0a..88bdef7 100644 --- a/charm/requirements.txt +++ b/charm/requirements.txt @@ -1 +1 @@ -paas-app-charmer~=1.4.0 +paas-charm~=1.1.0 diff --git a/charm/src/charm.py b/charm/src/charm.py index d5928cf..cd7087d 100755 --- a/charm/src/charm.py +++ b/charm/src/charm.py @@ -3,18 +3,39 @@ # See LICENSE file for licensing details. """Flask Charm entrypoint.""" - +import json import logging import typing import ops -import paas_app_charmer.flask +# we don't have the types for paas_charm.flask +import paas_charm.flask # type: ignore +from ops import ActionEvent +from ops.pebble import ExecError logger = logging.getLogger(__name__) -class FlaskCharm(paas_app_charmer.flask.Charm): +SCRIPT_ARG_PARSE_ERROR_EXIT_CODE = 1 +SINCE_PARAM_NAME = "since" +GITHUB_PATH_PARAM_NAME = "github-path" +WEBHOOK_ID_PARAM_NAME = "webhook-id" +GITHUB_TOKEN_SECRET_ID_PARAM_NAME = "github-token-secret-id" +GITHUB_APP_CLIENT_ID_PARAM_NAME = "github-app-client-id" +GITHUB_APP_INSTALLATION_ID_PARAM_NAME = "github-app-installation-id" +GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME = "github-app-private-key-secret-id" +GITHUB_TOKEN_ENV_NAME = "GITHUB_TOKEN" +GITHUB_APP_CLIENT_ID_ENV_NAME = "GITHUB_APP_CLIENT_ID" +GITHUB_APP_INSTALLATION_ID_ENV_NAME = "GITHUB_APP_INSTALLATION_ID" +GITHUB_APP_PRIVATE_KEY_ENV_NAME = "GITHUB_APP_PRIVATE_KEY" + + +class _ActionParamsInvalidError(Exception): + """Raised when the action parameters are invalid.""" + + +class FlaskCharm(paas_charm.flask.Charm): """Flask Charm service.""" def __init__(self, *args: typing.Any) -> None: @@ -24,6 +45,110 @@ def __init__(self, *args: typing.Any) -> None: args: passthrough to CharmBase. """ super().__init__(*args) + self.framework.observe( + self.on.redeliver_failed_webhooks_action, self._on_redeliver_failed_webhooks_action + ) + + def _on_redeliver_failed_webhooks_action(self, event: ops.charm.ActionEvent) -> None: + """Redeliver failed webhooks since a given time.""" + logger.info("Redelivering failed webhooks.") + container: ops.Container = self.unit.get_container("flask-app") + since_seconds = event.params[SINCE_PARAM_NAME] + github_path = event.params[GITHUB_PATH_PARAM_NAME] + webhook_id = event.params[WEBHOOK_ID_PARAM_NAME] + + try: + auth_env = self._get_github_auth_env(event) + except _ActionParamsInvalidError as exc: + event.fail(f"Invalid action parameters passed: {exc}") + return + try: + stdout, _ = container.exec( + [ + "/usr/bin/python3", + "/flask/app/webhook_redelivery.py", + "--since", + str(since_seconds), + "--github-path", + github_path, + "--webhook-id", + str(webhook_id), + ], + environment=auth_env, + ).wait_output() + logger.info("Got %s", stdout) + # only consider the last line as result + result = json.loads(stdout.rstrip().split("\n")[-1]) + event.set_results(result) + except ExecError as exc: + logger.warning("Webhook redelivery failed, script reported: %s", exc.stderr) + if exc.exit_code == SCRIPT_ARG_PARSE_ERROR_EXIT_CODE: + event.fail(f"Argument parsing failed. {exc.stderr}") + return + event.fail("Webhooks redelivery failed. Look at the juju logs for more information.") + + def _get_github_auth_env(self, event: ActionEvent) -> dict[str, str]: + """Get the GitHub auth environment variables from the action parameters. + + Args: + event: The action event. + + Returns: + The GitHub auth environment variables used by the script in the workload. + """ + github_token_secret_id = event.params.get(GITHUB_TOKEN_SECRET_ID_PARAM_NAME) + github_app_client_id = event.params.get(GITHUB_APP_CLIENT_ID_PARAM_NAME) + github_app_installation_id = event.params.get(GITHUB_APP_INSTALLATION_ID_PARAM_NAME) + github_app_private_key_secret_id = event.params.get( + GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME + ) + + github_token = ( + self._get_secret_value(github_token_secret_id, "token") + if github_token_secret_id + else None + ) + github_app_private_key = ( + self._get_secret_value(github_app_private_key_secret_id, "private-key") + if github_app_private_key_secret_id + else None + ) + + env_vars = { + GITHUB_TOKEN_ENV_NAME: github_token, + GITHUB_APP_CLIENT_ID_ENV_NAME: github_app_client_id, + GITHUB_APP_INSTALLATION_ID_ENV_NAME: ( + str(github_app_installation_id) if github_app_installation_id else None + ), + GITHUB_APP_PRIVATE_KEY_ENV_NAME: github_app_private_key, + } + return {k: v for k, v in env_vars.items() if v} + + def _get_secret_value(self, secret_id: str, key: str) -> str: + """Get the value of a secret. + + Args: + secret_id: The secret id. + key: The key of the secret value to extract. + + Returns: + The secret value. + + Raises: + _ActionParamsInvalidError: If the secret does not exist + or the key is not in the secret. + """ + try: + secret = self.model.get_secret(id=secret_id) + except ops.model.ModelError as exc: + raise _ActionParamsInvalidError(f"Could not access/find secret {secret_id}") from exc + secret_data = secret.get_content() + try: + return secret_data[key] + except KeyError as exc: + raise _ActionParamsInvalidError( + f"Secret {secret_id} does not contain a field called '{key}'." + ) from exc if __name__ == "__main__": diff --git a/charm/tox.ini b/charm/tox.ini new file mode 100644 index 0000000..10d33c0 --- /dev/null +++ b/charm/tox.ini @@ -0,0 +1,76 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +[tox] +skipsdist=True +skip_missing_interpreters = True +envlist = lint, static + +[vars] +src_path = {toxinidir}/src/ + +[testenv] +setenv = + PYTHONPATH = {toxinidir}:{toxinidir}/lib:{[vars]src_path} + PYTHONBREAKPOINT=ipdb.set_trace + PY_COLORS=1 +passenv = + PYTHONPATH + CHARM_BUILD_DIR + MODEL_SETTINGS + +[testenv:fmt] +description = srcly coding style standards to code +deps = + black + isort +commands = + isort {[vars]src_path} + black {[vars]src_path} + +[testenv:lint] +description = Check code against coding style standards +deps = + black + codespell + flake8<6.0.0 + flake8-builtins + flake8-copyright<6.0.0 + flake8-docstrings>=1.6.0 + flake8-docstrings-complete>=1.0.3 + flake8-test-docs>=1.0 + isort + mypy + pep8-naming + pydocstyle>=2.10 + pylint + pyproject-flake8<6.0.0 + pytest + pytest-asyncio + pytest-operator + requests + types-PyYAML + types-requests + -r{toxinidir}/requirements.txt +commands = + pydocstyle {[vars]src_path} + # uncomment the following line if this charm owns a lib + # codespell {[vars]lib_path} + codespell {toxinidir} --skip {toxinidir}/.git --skip {toxinidir}/.tox \ + --skip {toxinidir}/build --skip {toxinidir}/lib --skip {toxinidir}/venv \ + --skip {toxinidir}/.mypy_cache --skip {toxinidir}/icon.svg + # pflake8 wrapper supports config from pyproject.toml + pflake8 {[vars]src_path} --ignore=W503 + isort --check-only --diff {[vars]src_path} + black --check --diff {[vars]src_path} + mypy {[vars]src_path} + pylint {[vars]src_path} + + +[testenv:static] +description = Run static analysis tests +deps = + bandit[toml] + -r{toxinidir}/requirements.txt +commands = + bandit -c {toxinidir}/pyproject.toml -r {[vars]src_path} diff --git a/requirements.txt b/requirements.txt index 237353a..cc51bbd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,4 @@ pydantic==2.10.3 kombu==5.4.2 pymongo==4.10.1 PyYAML==6.0.* +pygithub==2.5.0 diff --git a/rockcraft.yaml b/rockcraft.yaml index 536a80b..9894ed1 100644 --- a/rockcraft.yaml +++ b/rockcraft.yaml @@ -34,3 +34,4 @@ parts: # Note: Prefix each entry with "flask/app/" followed by the local path. - flask/app/app.py - flask/app/webhook_router + - flask/app/webhook_redelivery.py diff --git a/src-docs/parse.py.md b/src-docs/parse.py.md index 551865f..b4f7d33 100644 --- a/src-docs/parse.py.md +++ b/src-docs/parse.py.md @@ -5,13 +5,10 @@ # module `parse.py` Module for parsing the webhook payload. -**Global Variables** ---------------- -- **WORKFLOW_JOB** --- - + ## function `webhook_to_job` diff --git a/src-docs/router.py.md b/src-docs/router.py.md index 75ee279..b06e251 100644 --- a/src-docs/router.py.md +++ b/src-docs/router.py.md @@ -5,9 +5,6 @@ # module `router.py` Module for routing webhooks to the appropriate message queue. -**Global Variables** ---------------- -- **WORKFLOW_JOB** --- diff --git a/tests/conftest.py b/tests/conftest.py index ae691e2..69269b9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,11 @@ CHARM_FILE_PARAM = "--charm-file" FLASK_APP_IMAGE_PARAM = "--github-runner-webhook-router-image" USE_EXISTING_APP_PARAM = "--use-existing-app" +GITHUB_TOKEN_PARAM = "--github-token" # nosec this is no hardcoded password +GITHUB_APP_CLIENT_ID_PARAM = "--github-app-client-id" +GITHUB_APP_INSTALLATION_ID_PARAM_NAME = "--github-app-installation-id" +GITHUB_APP_PRIVATE_KEY_PARAM_NAME = "--github-app-private-key" +WEBHOOK_TEST_REPOSITORY_PARAM = "--webhook-test-repository" def pytest_addoption(parser: Parser) -> None: @@ -23,3 +28,29 @@ def pytest_addoption(parser: Parser) -> None: action="store_true", help="Use an existing app instead of deploying a new one, useful for local testing", ) + parser.addoption( + GITHUB_TOKEN_PARAM, + action="store", + help="GitHub token used for testing github API interactions", + ) + parser.addoption( + GITHUB_APP_CLIENT_ID_PARAM, + action="store", + help="GitHub App ID used for testing github API interactions", + ) + parser.addoption( + GITHUB_APP_INSTALLATION_ID_PARAM_NAME, + action="store", + help="GitHub App installation ID used for testing github API interactions", + ) + parser.addoption( + GITHUB_APP_PRIVATE_KEY_PARAM_NAME, + action="store", + help="GitHub App private key used for testing github API interactions", + ) + parser.addoption( + WEBHOOK_TEST_REPOSITORY_PARAM, + action="store", + help="Name of the GitHub repository used to test webhook delivery", + default="canonical/github-runner-webhook-router", + ) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 4a2dfa1..ea7fb62 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -2,8 +2,9 @@ # See LICENSE file for licensing details. """Fixtures for the github-runner-webhook-router charm.""" - +import os import random +from collections import namedtuple from typing import Any import pytest @@ -12,7 +13,24 @@ from juju.model import Model from pytest_operator.plugin import OpsTest -from tests.conftest import CHARM_FILE_PARAM, FLASK_APP_IMAGE_PARAM +from tests.conftest import ( + CHARM_FILE_PARAM, + FLASK_APP_IMAGE_PARAM, + GITHUB_APP_CLIENT_ID_PARAM, + GITHUB_APP_INSTALLATION_ID_PARAM_NAME, + GITHUB_APP_PRIVATE_KEY_PARAM_NAME, + GITHUB_TOKEN_PARAM, + WEBHOOK_TEST_REPOSITORY_PARAM, +) + +GITHUB_TOKEN_ENV_VAR = "GITHUB_TOKEN" # nosec this is no hardcoded password +GITHUB_APP_INSTALLATION_ID_ENV_VAR = "GITHUB_APP_INSTALLATION_ID" +GITHUB_APP_PRIVATE_KEY_ENV_VAR = "GITHUB_APP_PRIVATE_KEY" +GITHUB_APP_CLIENT_ID_ENV_VAR = "GITHUB_APP_CLIENT_ID" + +GithubAuthenticationMethodParams = namedtuple( + "GithubAuthenticationMethodParams", ["client_id", "installation_id", "private_key", "token"] +) @pytest.fixture(name="use_existing_app", scope="module") @@ -35,6 +53,80 @@ def flask_app_image_fixture(pytestconfig: pytest.Config) -> str | None: return flask_app_image +@pytest.fixture(name="github_token", scope="module") +def github_token_fixture(pytestconfig: pytest.Config) -> str | None: + """Return the github token secret""" + github_token = pytestconfig.getoption(GITHUB_TOKEN_PARAM) or os.getenv(GITHUB_TOKEN_ENV_VAR) + return github_token + + +@pytest.fixture(name="github_app_client_id", scope="module") +def github_app_client_id_fixture(pytestconfig: pytest.Config) -> str | None: + """Return the github app id""" + github_app_client_id = pytestconfig.getoption(GITHUB_APP_CLIENT_ID_PARAM) or os.getenv( + GITHUB_APP_CLIENT_ID_ENV_VAR + ) + return github_app_client_id + + +@pytest.fixture(name="github_app_installation_id", scope="module") +def github_app_installation_id_fixture(pytestconfig: pytest.Config) -> int | None: + """Return the github app installation id""" + github_app_installation_id = pytestconfig.getoption( + GITHUB_APP_INSTALLATION_ID_PARAM_NAME + ) or os.getenv(GITHUB_APP_INSTALLATION_ID_ENV_VAR) + if github_app_installation_id is None: + return None + return int(github_app_installation_id) + + +@pytest.fixture(name="github_app_private_key", scope="module") +def github_app_private_key_fixture(pytestconfig: pytest.Config) -> str | None: + """Return the github app private key""" + github_app_private_key = pytestconfig.getoption( + GITHUB_APP_PRIVATE_KEY_PARAM_NAME + ) or os.getenv(GITHUB_APP_PRIVATE_KEY_ENV_VAR) + return github_app_private_key + + +@pytest.fixture( + name="github_auth", + scope="module", + params=[ + pytest.param(True, id="use github token"), + pytest.param(False, id="use github app auth"), + ], +) +def github_auth_fixture( + request: pytest.FixtureRequest, + github_token: str | None, + github_app_client_id: str | None, + github_app_installation_id: str | None, + github_app_private_key: str | None, +) -> GithubAuthenticationMethodParams: + """Return whether to use github app auth""" + if request.param: + assert github_token is not None, "Github token is required" + return GithubAuthenticationMethodParams( + client_id=None, installation_id=None, private_key=None, token=github_token + ) + if not (github_app_client_id or github_app_installation_id or github_app_private_key): + pytest.skip("Not all github app auth parameters provided/non-empty") + return GithubAuthenticationMethodParams( + client_id=github_app_client_id, + installation_id=github_app_installation_id, + private_key=github_app_private_key, + token=None, + ) + + +@pytest.fixture(name="test_repo", scope="module") +def test_repo_fixture(pytestconfig: pytest.Config) -> str | None: + """Return the github test repository""" + test_repo = pytestconfig.getoption(WEBHOOK_TEST_REPOSITORY_PARAM) + return test_repo + + @pytest.fixture(name="model", scope="module") def model_fixture(ops_test: OpsTest) -> Model: """Juju model used in the test.""" @@ -94,11 +186,10 @@ def deploy_config_fixture( } -@pytest_asyncio.fixture(name="app", scope="module") -async def app_fixture( +@pytest_asyncio.fixture(name="router", scope="module") +async def router_fixture( model: Model, deploy_config: dict[str, Any], - mongodb: Application, ) -> Application: """Deploy the application.""" app_name = deploy_config["app-name"] @@ -119,6 +210,5 @@ async def app_fixture( application_name=deploy_config["app-name"], config=deploy_config["config"], ) - await model.relate(f"{application.name}:mongodb", f"{mongodb.name}:database") - await model.wait_for_idle(apps=[app_name, mongodb.name], status="active") + await model.wait_for_idle(apps=[app_name], status="blocked") return application diff --git a/tests/integration/test_app.py b/tests/integration/test_app.py index f78fdb8..07d9f7e 100644 --- a/tests/integration/test_app.py +++ b/tests/integration/test_app.py @@ -9,9 +9,10 @@ import random import re import secrets -from typing import Optional +from typing import Any, Optional import pytest +import pytest_asyncio import requests from juju.application import Application from juju.model import Model @@ -28,6 +29,19 @@ PORT = 8000 +@pytest_asyncio.fixture(name="app", scope="module") +async def app_fixture( + router: Application, + mongodb: Application, + deploy_config: dict[str, Any], +) -> Application: + """Relate the router with mongodb and return the router application.""" + if not deploy_config["use-existing-app"]: + await router.model.relate(f"{router.name}:mongodb", f"{mongodb.name}:database") + await router.model.wait_for_idle(apps=[router.name, mongodb.name], status="active") + return router + + @pytest.mark.parametrize( "webhook_secret", [ @@ -265,7 +279,10 @@ async def _get_unit_ips(app: Application) -> tuple[str, ...]: a tuple containing unit ip addresses. """ status = await app.model.get_status() - return tuple(unit.address for unit in status.applications[app.name].units.values()) + app_status = status.applications[app.name] + assert app_status is not None, f"Application {app.name} not found in status" + # mypy does not recognize that app_status is of type ApplicationStatus + return tuple(unit.address for unit in app_status.units.values()) # type: ignore def _request(payload: dict, webhook_secret: Optional[str], base_url: str) -> requests.Response: @@ -397,7 +414,7 @@ async def _get_mongodb_uri_from_secrets(ops_test, model: Model) -> str | None: mongodb_uri = None juju_secrets = await model.list_secrets() - for secret in juju_secrets["results"]: + for secret in juju_secrets: if re.match(r"database\.(\d+)\.user\.secret", secret.label): _, show_secret, _ = await ops_test.juju( "show-secret", secret.uri, "--reveal", "--format", "json" diff --git a/tests/integration/test_webhook_redelivery.py b/tests/integration/test_webhook_redelivery.py new file mode 100644 index 0000000..b89dfd1 --- /dev/null +++ b/tests/integration/test_webhook_redelivery.py @@ -0,0 +1,315 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Integration tests for the webhook redelivery script.""" + +import secrets +from asyncio import sleep +from datetime import datetime, timezone +from typing import Callable, Iterator +from uuid import uuid4 + +import pytest +from github import Github +from github.Auth import Token +from github.Hook import Hook +from github.HookDelivery import HookDeliverySummary +from github.Repository import Repository +from github.Workflow import Workflow +from juju.action import Action +from juju.application import Application +from juju.unit import Unit + +from tests.integration.conftest import GithubAuthenticationMethodParams + +# some tests do not require real gh resources, so we use fake values for them in those tests +FAKE_HOOK_ID = 123 +FAKE_REPO = "org/repo" + +# this is no hardcoded password +GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME = "github-app-private-key-secret-id" # nosec +GITHUB_APP_CLIENT_ID_PARAM_NAME = "github-app-client-id" +GITHUB_APP_INSTALLATION_ID_PARAM_NAME = "github-app-installation-id" +GITHUB_TOKEN_SECRET_ID_PARAM_NAME = "github-token-secret-id" # nosec this is no hardcoded password + +TEST_WORKFLOW_DISPATCH_FILE = "webhook_redelivery_test.yaml" + + +@pytest.fixture(name="repo", scope="module") +def repo_fixture(github_token: str, test_repo: str) -> Repository: + """Create a repository object for the test repo.""" + github = Github(auth=Token(github_token)) + repo = github.get_repo(test_repo) + + return repo + + +@pytest.fixture(name="hook") +def hook_fixture(repo: Repository) -> Iterator["Hook"]: + """Create a webhook for the test repo. + + The hook gets deleted after the test. + """ + # we need a unique url to distinguish this webhook from others + # the ip is internal and the webhook delivery is expected to fail + unique_url = f"http://192.168.0.1:8080/{uuid4().hex}" + hook = repo.create_hook( + name="web", + events=["workflow_job"], + config={"url": unique_url, "content_type": "json", "insecure_ssl": "0"}, + ) + + yield hook + + hook.delete() + + +@pytest.fixture(name="test_workflow", scope="module") +def test_workflow_fixture(repo: Repository) -> Iterator[Workflow]: + """Create a workflow for the test repo.""" + start_time = datetime.now(timezone.utc) + workflow = repo.get_workflow(TEST_WORKFLOW_DISPATCH_FILE) + yield workflow + # cancel all runs, it's not necessary to stay in queue or be picked up by a runner + for run in workflow.get_runs(created=f">={start_time.isoformat()}"): + run.cancel() + + +async def test_webhook_redelivery( + router: Application, + github_auth: GithubAuthenticationMethodParams, + repo: Repository, + hook: Hook, + test_workflow: Workflow, +) -> None: + """ + arrange: a hook with a failed delivery + act: call the action to redeliver the webhook + assert: the failed delivery has been redelivered + """ + unit = router.units[0] + + action_parms = {"webhook-id": hook.id, "since": 600, "github-path": repo.full_name} + secret_id = await _create_secret_for_github_auth(router, github_auth) + if github_auth.token: + action_parms["github-token-secret-id"] = secret_id + else: + action_parms |= { + GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME: secret_id, + GITHUB_APP_CLIENT_ID_PARAM_NAME: github_auth.client_id, + GITHUB_APP_INSTALLATION_ID_PARAM_NAME: github_auth.installation_id, + } + # we need to dispatch a job in order to have a webhook delivery with event "workflow_job" + assert test_workflow.create_dispatch(ref="main") + + async def _wait_for_delivery_condition( + condition: Callable[[HookDeliverySummary], bool], condition_title: str + ) -> None: + """Wait to find a certain delivery with the condition.""" + for _ in range(10): + deliveries = repo.get_hook_deliveries(hook.id) + for delivery in deliveries: + if condition(delivery): + return + await sleep(1) + assert False, f"Did not receive a webhook who fits the condition '{condition_title}'" + + await _wait_for_delivery_condition( + lambda d: d.event == "workflow_job", "event is workflow_job" + ) + + action: Action = await unit.run_action("redeliver-failed-webhooks", **action_parms) + + await action.wait() + assert action.status == "completed", f"action failed with f{action.data['message']}" + assert ( + action.results.get("redelivered") == "1" + ), f"redelivered not matching in {action.results}" + await _wait_for_delivery_condition( + lambda d: d.event == "workflow_job" and bool(d.redelivery), + "delivery with event workflow_job has been redelivered", + ) + + +@pytest.mark.parametrize( + "github_app_client_id, github_app_installation_id, " + "github_app_private_key_secret, github_token_secret," + "expected_message", + [ + pytest.param( + "123", + 446, + "private", + "token", + "Github auth details are specified in two ways. " + "Please specify only one of github token or github app auth details.", + id="github app config and github token secret", + ), + pytest.param( + None, + None, + None, + None, + "Github auth details are not specified completely." + " Am missing github token or complete set of app auth parameters.", + id="no github app config or github token", + ), + pytest.param( + "eda", + 123, + None, + None, + "Github auth details are not specified completely." + " Am missing github token or complete set of app auth parameters.", + id="not all github app config provided", + ), + ], +) # we use a lot of arguments, but it seems not worth to introduce a capsulating object for this +async def test_action_github_auth_param_error( + # pylint: disable=too-many-arguments,too-many-positional-arguments + github_app_client_id: str | None, + github_app_installation_id: int | None, + github_app_private_key_secret: str | None, + github_token_secret: str | None, + expected_message: str, + router: Application, +): + """ + arrange: Given a mocked environment with invalid github auth configuration. + act: Call the action. + assert: The action fails with the expected message. + """ + unit: Unit = router.units[0] + + secret_data = [] + action_parms = {"webhook-id": FAKE_HOOK_ID, "since": 600, "github-path": FAKE_REPO} + if github_app_private_key_secret: + secret_data.append(f"private-key={github_app_private_key_secret}") + if github_token_secret: + secret_data.append(f"token={github_token_secret}") + if secret_data: + secret_id = await _create_secret(router, secret_data) + if github_app_private_key_secret: + action_parms[GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME] = secret_id + if github_token_secret: + action_parms[GITHUB_TOKEN_SECRET_ID_PARAM_NAME] = secret_id + if github_app_client_id: + action_parms[GITHUB_APP_CLIENT_ID_PARAM_NAME] = github_app_client_id + if github_app_installation_id: + action_parms[GITHUB_APP_INSTALLATION_ID_PARAM_NAME] = github_app_installation_id + + action: Action = await unit.run_action("redeliver-failed-webhooks", **action_parms) + await action.wait() + + assert action.status == "failed" + assert "Argument parsing failed" in (action_msg := action.data["message"]) + assert expected_message in action_msg + + +@pytest.mark.parametrize( + "use_app_auth", + [ + pytest.param( + True, + id="wrong field in github app secret", + ), + pytest.param( + False, + id="wrong field in github token secret", + ), + ], +) +async def test_secret_missing(use_app_auth: bool, router: Application) -> None: + """ + arrange: Given auth parameters with a secret which is missing. + act: Call the action. + assert: The action fails with the expected message. + """ + unit: Unit = router.units[0] + + action_parms = {"webhook-id": FAKE_HOOK_ID, "since": 600, "github-path": FAKE_REPO} + if use_app_auth: + action_parms |= { + GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME: secrets.token_hex(16), + GITHUB_APP_CLIENT_ID_PARAM_NAME: secrets.token_hex(16), + GITHUB_APP_INSTALLATION_ID_PARAM_NAME: 123, + } + else: + action_parms["github-token-secret-id"] = secrets.token_hex(16) + + action: Action = await unit.run_action("redeliver-failed-webhooks", **action_parms) + + await action.wait() + assert action.status == "failed" + assert "Invalid action parameters passed" in (action_msg := action.data["message"]) + assert "Could not access/find secret" in action_msg + + +@pytest.mark.parametrize( + "use_app_auth, expected_message", + [ + pytest.param( + True, + "does not contain a field called 'private-key'.", + id="wrong field in github app secret", + ), + pytest.param( + False, + "does not contain a field called 'token'.", + id="wrong field in github token secret", + ), + ], +) +async def test_secret_key_missing( + use_app_auth: bool, expected_message: str, router: Application +) -> None: + """ + arrange: Given auth parameters with a secret which is missing a key. + act: Call the action. + assert: The action fails with the expected message. + """ + unit = router.units[0] + + action_parms = {"webhook-id": FAKE_HOOK_ID, "since": 600, "github-path": FAKE_REPO} + secret_data = [ + f"private-key-invalid={secrets.token_hex(16)}", + f"token-invalid={secrets.token_hex(16)}", + ] + secret_id = await _create_secret(router, secret_data) + if use_app_auth: + action_parms |= { + GITHUB_APP_PRIVATE_KEY_SECRET_ID_PARAM_NAME: secret_id, + GITHUB_APP_CLIENT_ID_PARAM_NAME: secrets.token_hex(16), + GITHUB_APP_INSTALLATION_ID_PARAM_NAME: 123, + } + else: + action_parms["github-token-secret-id"] = secret_id + + action: Action = await unit.run_action("redeliver-failed-webhooks", **action_parms) + await action.wait() + + assert action.status == "failed" + assert "Invalid action parameters passed" in (action_msg := action.data["message"]) + assert expected_message in action_msg + + +async def _create_secret_for_github_auth( + app: Application, github_auth: GithubAuthenticationMethodParams +) -> str: + """Create a secret with appropriate key depending on the Github auth type.""" + if github_auth.token: + secret_data = [f"token={github_auth.token}"] + else: + secret_data = [f"private-key={github_auth.private_key}"] + + return await _create_secret(app, secret_data) + + +async def _create_secret(app: Application, secret_data: list[str]) -> str: + """Create a secret with the given data.""" + secret_name = f"secret-{uuid4().hex}" + secret = await app.model.add_secret(secret_name, secret_data) + secret_id = secret.split(":")[-1] + + await app.model.grant_secret(secret_name, app.name) + + return secret_id diff --git a/tests/unit/test_webhook_redelivery.py b/tests/unit/test_webhook_redelivery.py new file mode 100644 index 0000000..1555d30 --- /dev/null +++ b/tests/unit/test_webhook_redelivery.py @@ -0,0 +1,276 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for webhook redelivery script.""" +import secrets +from collections import namedtuple +from datetime import datetime, timedelta, timezone +from unittest.mock import MagicMock + +import github +import pytest +from github.HookDelivery import HookDeliverySummary + +from webhook_redelivery import ( + OK_STATUS, + RedeliveryError, + WebhookAddress, + _redeliver_failed_webhook_delivery_attempts, +) + +_Delivery = namedtuple("_Delivery", ["id", "status", "age"]) + + +@pytest.fixture( + name="webhook_address", + params=[ + pytest.param(True, id="repository webhook"), + pytest.param(False, id="organization webhook"), + ], +) +def webhook_address_fixture(request: pytest.FixtureRequest) -> WebhookAddress: + """Return a webhook address for a repository and for an organization.""" + return WebhookAddress( + github_org=secrets.token_hex(16), + github_repo=secrets.token_hex(16) if request.param else None, + id=1234, + ) + + +@pytest.mark.parametrize( + "deliveries,since_seconds,expected_redelivered", + [ + pytest.param([], 5, set(), id="empty"), + pytest.param([_Delivery(1, "failed", 4)], 5, {1}, id="one failed"), + pytest.param([_Delivery(1, "failed", 4)], 3, set(), id="one failed but too old"), + pytest.param([_Delivery(1, "failed", 1)], 0, {}, id="one failed zero since seconds"), + pytest.param([_Delivery(1, "failed", 0)], 0, {1}, id="one failed zero age"), + pytest.param( + [_Delivery(1, "failed", 0)], -1, {}, id="one failed zero age negative since seconds" + ), + pytest.param( + [_Delivery(1, "failed", 4), _Delivery(2, "failed", 4)], 5, {1, 2}, id="two failed" + ), + pytest.param([_Delivery(1, OK_STATUS, 4)], 5, set(), id="one OK"), + pytest.param( + [_Delivery(1, OK_STATUS, 3), _Delivery(2, OK_STATUS, 3), _Delivery(3, "failed", 4)], + 5, + {3}, + id="two OK one failed", + ), + pytest.param( + [_Delivery(1, OK_STATUS, 3), _Delivery(2, OK_STATUS, 3), _Delivery(3, "failed", 4)], + 2, + set(), + id="two OK one failed but too old", + ), + pytest.param( + [ + _Delivery(1, OK_STATUS, 3), + _Delivery(2, OK_STATUS, 3), + _Delivery(3, "failed", 4), + _Delivery(4, "failed", 3), + ], + 1, + {}, + id="two OK two failed and all too old", + ), + ], +) +def test_redeliver( + monkeypatch: pytest.MonkeyPatch, + deliveries: list[_Delivery], + since_seconds: int, + expected_redelivered: set[int], + webhook_address: WebhookAddress, +): + """ + arrange: A mocked github client and different combinations of deliveries. + act: Call the script. + assert: The expected redeliveries are made. + """ + github_client = MagicMock(spec=github.Github) + monkeypatch.setattr("webhook_redelivery.Github", MagicMock(return_value=github_client)) + now = datetime.now(tz=timezone.utc) + monkeypatch.setattr("webhook_redelivery.datetime", MagicMock(now=MagicMock(return_value=now))) + + get_hook_deliveries_mock = _get_get_deliveries_mock(github_client, webhook_address) + get_hook_deliveries_mock.return_value = [ + MagicMock( + spec=HookDeliverySummary, + id=d.id, + status=d.status, + action="queued", + event="workflow_job", + delivered_at=now - timedelta(seconds=d.age), + ) + for d in deliveries + ] + + github_token = secrets.token_hex(16) + redelivered = _redeliver_failed_webhook_delivery_attempts( + github_auth=github_token, webhook_address=webhook_address, since_seconds=since_seconds + ) + + assert redelivered == len(expected_redelivered) + + redeliver_mock = github_client.requester.requestJsonAndCheck + assert redeliver_mock.call_count == redelivered + for _id in expected_redelivered: + return redeliver_mock.assert_any_call( + "POST", _get_redeliver_mock_api_url(webhook_address, _id) + ) + + +@pytest.mark.parametrize( + "action,event", + [ + pytest.param("completed", "workflow_job", id="completed workflow_job"), + pytest.param("in_progress", "workflow_job", id="in_progress workflow_job"), + pytest.param("waiting", "workflow_job", id="waiting workflow_job"), + pytest.param("queued", "push", id="queued push"), + pytest.param("completed", "push", id="completed push"), + pytest.param("queued", "workflow_run", id="queued workflow_run"), + ], +) +def test_redelivery_ignores_non_queued_or_non_workflow_job( + monkeypatch: pytest.MonkeyPatch, + webhook_address: WebhookAddress, + action: str, + event: str, +): + """ + arrange: A mocked github client and different combinations of non-routable actions and events. + act: Call the script. + assert: No redeliveries are made. + """ + github_client = MagicMock(spec=github.Github) + monkeypatch.setattr("webhook_redelivery.Github", MagicMock(return_value=github_client)) + now = datetime.now(tz=timezone.utc) + monkeypatch.setattr("webhook_redelivery.datetime", MagicMock(now=MagicMock(return_value=now))) + + get_hook_deliveries_mock = _get_get_deliveries_mock(github_client, webhook_address) + get_hook_deliveries_mock.return_value = [ + MagicMock( + spec=HookDeliverySummary, + id=d.id, + status=d.status, + action=action, + event=event, + delivered_at=now - timedelta(seconds=d.age), + ) + for d in [_Delivery(i, action, 4) for i in range(3)] + ] + + github_token = secrets.token_hex(16) + redelivered = _redeliver_failed_webhook_delivery_attempts( + github_auth=github_token, webhook_address=webhook_address, since_seconds=5 + ) + + assert redelivered == 0 + + redeliver_mock = github_client.requester.requestJsonAndCheck + assert redeliver_mock.call_count == 0 + + +@pytest.mark.parametrize( + "github_exception,expected_msg", + [ + pytest.param( + github.BadCredentialsException(403), + "The github client returned a Bad Credential error", + id="BadCredentialsException", + ), + pytest.param( + github.RateLimitExceededException(403), + "The github client is returning a Rate Limit Exceeded error", + id="RateLimitExceededException", + ), + pytest.param( + github.GithubException(500), + "The github client encountered an error", + id="GithubException", + ), + ], +) +def test_redelivery_github_errors( + monkeypatch: pytest.MonkeyPatch, + github_exception: github.GithubException, + expected_msg: str, + webhook_address: WebhookAddress, +): + """ + arrange: A mocked github client and a github exception. + act: Call the script. + assert: The expected error is raised. + """ + github_client = MagicMock(spec=github.Github) + monkeypatch.setattr("webhook_redelivery.Github", MagicMock(return_value=github_client)) + + github_token = secrets.token_hex(16) + since_seconds = 5 + + get_hook_deliveries_mock = _get_get_deliveries_mock(github_client, webhook_address) + get_hook_deliveries_mock.side_effect = github_exception + + with pytest.raises(RedeliveryError) as exc_info: + _redeliver_failed_webhook_delivery_attempts( + github_auth=github_token, webhook_address=webhook_address, since_seconds=since_seconds + ) + assert expected_msg in str(exc_info.value) + + +def test_redelivery_api_insufficient_data( + monkeypatch: pytest.MonkeyPatch, + webhook_address: WebhookAddress, +): + """ + arrange: A mocked github client which returns not all required fields in an API response. + act: Call the script. + assert: An AssertionError is raised. + """ + github_client = MagicMock(spec=github.Github) + monkeypatch.setattr("webhook_redelivery.Github", MagicMock(return_value=github_client)) + + github_token = secrets.token_hex(16) + since_seconds = 5 + + get_hook_deliveries_mock = _get_get_deliveries_mock(github_client, webhook_address) + get_hook_deliveries_mock.return_value = [ + MagicMock( + spec=HookDeliverySummary, + id=1, + status="failed", + delivered_at=datetime.now(tz=timezone.utc), + action="ping", + event=None, # missing event + ) + ] + + with pytest.raises(AssertionError) as exc_info: + _redeliver_failed_webhook_delivery_attempts( + github_auth=github_token, webhook_address=webhook_address, since_seconds=since_seconds + ) + assert "is missing required fields: {'event'}" in str(exc_info.value) + + +def _get_get_deliveries_mock( + github_client_mock: MagicMock, webhook_address: WebhookAddress +) -> MagicMock: + """Return the mock for the get_hook_deliveries method.""" + return ( + github_client_mock.get_repo.return_value.get_hook_deliveries + if webhook_address.github_repo + else github_client_mock.get_organization.return_value.get_hook_deliveries + ) + + +def _get_redeliver_mock_api_url(webhook_address: WebhookAddress, delivery_id: int) -> str: + """Return the expected API URL for redelivering a webhook.""" + return ( + f"/repos/{webhook_address.github_org}/{webhook_address.github_repo}" + f"/hooks/{webhook_address.id}/deliveries/{delivery_id}/attempts" + if webhook_address.github_repo + else f"/orgs/{webhook_address.github_org}/hooks/{webhook_address.id}" + f"/deliveries/{delivery_id}/attempts" + ) diff --git a/tox.ini b/tox.ini index a82d880..87ed798 100644 --- a/tox.ini +++ b/tox.ini @@ -9,8 +9,8 @@ envlist = lint, unit, static, coverage-report [vars] app_path = {toxinidir}/webhook_router/ tst_path = {toxinidir}/tests/ -;lib_path = {toxinidir}/lib/charms/operator_name_with_underscores -all_path = {[vars]app_path} {[vars]tst_path} +redelivery_script_path = {toxinidir}/webhook_redelivery.py +all_path = {[vars]app_path} {[vars]tst_path} {[vars]redelivery_script_path} [testenv] setenv = @@ -76,7 +76,7 @@ deps = pytest -r{toxinidir}/requirements.txt commands = - coverage run --source={[vars]app_path} \ + coverage run --source={[vars]app_path},webhook_redelivery \ -m pytest --ignore={[vars]tst_path}integration -v --tb native -s {posargs} coverage report @@ -98,9 +98,10 @@ commands = bandit -c {toxinidir}/pyproject.toml -r {[vars]app_path} {[vars]tst_path} [testenv:integration] +passenv = GITHUB_TOKEN, GITHUB_APP_CLIENT_ID, GITHUB_APP_INSTALLATION_ID, GITHUB_APP_PRIVATE_KEY description = Run integration tests deps = - juju==3.1.* + juju==3.6.* pytest pytest-asyncio pytest-operator diff --git a/webhook_redelivery.py b/webhook_redelivery.py new file mode 100644 index 0000000..7af9611 --- /dev/null +++ b/webhook_redelivery.py @@ -0,0 +1,406 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Redeliver failed webhooks since a given time. + +Only webhooks with action type queued are redelivered (as the others are not routable). +""" +import argparse +import json +import logging +import os +import sys +from collections import namedtuple +from dataclasses import dataclass +from datetime import datetime, timedelta, timezone +from functools import wraps +from typing import Callable, Iterator, ParamSpec, TypeVar + +from github import BadCredentialsException, Github, GithubException, RateLimitExceededException +from github.Auth import AppAuth, AppInstallationAuth, Token +from pydantic import BaseModel + +from webhook_router.app import SUPPORTED_GITHUB_EVENT +from webhook_router.router import ROUTABLE_JOB_STATUS + +ARG_PARSE_ERROR_EXIT_CODE = 1 +REDELIVERY_ERROR_EXIT_CODE = 2 + +GITHUB_TOKEN_ENV_NAME = "GITHUB_TOKEN" +GITHUB_APP_CLIENT_ID_ENV_NAME = "GITHUB_APP_CLIENT_ID" +GITHUB_APP_INSTALLATION_ID_ENV_NAME = "GITHUB_APP_INSTALLATION_ID" +GITHUB_APP_PRIVATE_KEY_ENV_NAME = "GITHUB_APP_PRIVATE_KEY" + +OK_STATUS = "OK" + +P = ParamSpec("P") +R = TypeVar("R") + +logger = logging.getLogger(__name__) + + +class GithubAppAuthDetails(BaseModel): + """The details to authenticate with Github using a Github App. + + Attributes: + client_id: The Github App client ID. + installation_id: The installation ID of the Github App. + private_key: The private key to authenticate with Github. + """ + + client_id: str + installation_id: int + private_key: str + + +GithubToken = str +GithubAuthDetails = GithubAppAuthDetails | GithubToken + +_ParsedArgs = namedtuple("_ParsedArgs", ["since", "github_auth_details", "webhook_address"]) + + +@dataclass +class WebhookAddress: + """The address details to identify the webhook. + + Attributes: + github_org: Github organisation where the webhook is registered. + github_repo: Github repository, where the webhook is registered. Only applicable for + repository webhooks. + id: The identifier of the webhook. + """ + + github_org: str + github_repo: str | None + id: int + + +@dataclass +class _WebhookDeliveryAttempt: + """The details of a webhook delivery attempt. + + Attributes: + id: The identifier of the delivery. + status: The status of the delivery. + delivered_at: The time the delivery was made. + action: The action type of the delivery. + See https://docs.github.com/en/webhooks/webhook-events-and-payloads#workflow_job for + possible values for the workflow_job event. May be null for other events (e.g. ping). + event: The event type of the delivery. + """ + + id: int + status: str + delivered_at: datetime + action: str | None + event: str + + +class RedeliveryError(Exception): + """Raised when an error occurs during redelivery.""" + + +class ArgParseError(Exception): + """Raised when an error occurs during argument parsing.""" + + +def main() -> None: # pragma: no cover this is checked by integration tests + """Run the module as script.""" + args = _arg_parsing() + + redelivery_count = _redeliver_failed_webhook_delivery_attempts( + github_auth=args.github_auth_details, + webhook_address=args.webhook_address, + since_seconds=args.since, + ) + + print(json.dumps({"redelivered": redelivery_count})) + + +def _arg_parsing() -> _ParsedArgs: # pragma: no cover this is checked by integration tests + """Parse the command line arguments. + + Raises: + ArgParseError: If the arguments are invalid. + + Returns: + The parsed arguments. + """ + parser = argparse.ArgumentParser( + description=f"{__doc__}. The script returns the amount of redelivered webhooks in JSON" + " format. The script assumes github authentication details to be given via environment" + " variables. Depending on the authentication method, the script expects the environment" + f" variable {GITHUB_TOKEN_ENV_NAME} to be passed for token based authentication, or" + f" {GITHUB_APP_CLIENT_ID_ENV_NAME}, {GITHUB_APP_INSTALLATION_ID_ENV_NAME}," + f" {GITHUB_APP_PRIVATE_KEY_ENV_NAME} for GitHub App authentication." + ) + parser.add_argument( + "--since", + type=int, + help="The amount of seconds to look back for failed deliveries.", + required=True, + ) + parser.add_argument( + "--github-path", + type=str, + help=( + "The path of the organisation or repository where the webhooks are registered. Should" + "be in the format of or /." + ), + required=True, + ) + parser.add_argument( + "--webhook-id", + type=int, + help="The identifier of the webhook for which delivery attempts are being checked.", + required=True, + ) + args = parser.parse_args() + + github_app_client_id = os.getenv(GITHUB_APP_CLIENT_ID_ENV_NAME) + github_app_installation_id = os.getenv(GITHUB_APP_INSTALLATION_ID_ENV_NAME) + github_app_private_key = os.getenv(GITHUB_APP_PRIVATE_KEY_ENV_NAME) + github_token = os.getenv(GITHUB_TOKEN_ENV_NAME) + + github_auth_details: GithubAuthDetails + got_str = ( + f" Got {GITHUB_APP_CLIENT_ID_ENV_NAME} = {github_app_client_id}," + f" {GITHUB_APP_INSTALLATION_ID_ENV_NAME} = {github_app_installation_id}," + f" {GITHUB_APP_PRIVATE_KEY_ENV_NAME} = {'***' if github_app_private_key else None}," + f" {GITHUB_TOKEN_ENV_NAME} = {'***' if github_app_private_key else None}", + ) + if github_token and ( + github_app_client_id or github_app_installation_id or github_app_private_key + ): + raise ArgParseError( + "Github auth details are specified in two ways. " + "Please specify only one of github token or github app auth details." + f"{got_str}" + ) + if github_token: + github_auth_details = github_token + elif github_app_client_id and github_app_installation_id and github_app_private_key: + try: + github_auth_details = GithubAppAuthDetails( + client_id=github_app_client_id, + installation_id=int(github_app_installation_id), + private_key=github_app_private_key, + ) + except ValueError as exc: + raise ArgParseError(f"Failed to parse github auth details: {exc}") from exc + else: + raise ArgParseError( + "Github auth details are not specified completely. " + "Am missing github token or complete set of app auth parameters." + f"{got_str}" + ) + webhook_address = WebhookAddress( + github_org=args.github_path.split("/")[0], + github_repo=args.github_path.split("/")[1] if "/" in args.github_path else None, + id=args.webhook_id, + ) + + return _ParsedArgs( + since=args.since, github_auth_details=github_auth_details, webhook_address=webhook_address + ) + + +def _github_api_exc_decorator(func: Callable[P, R]) -> Callable[P, R]: + """Decorator to handle GitHub API exceptions.""" + + @wraps(func) + def _wrapper(*posargs: P.args, **kwargs: P.kwargs) -> R: + """Wrap the function to handle Github API exceptions. + + Catch Github API exceptions and raise an appropriate RedeliveryError instead. + + + Raises: + RedeliveryError: If an error occurs during redelivery. + + Returns: + The result of the origin function when no github error occurs. + """ + try: + return func(*posargs, **kwargs) + except BadCredentialsException as exc: + logging.error("Github client credentials error: %s", exc, exc_info=exc) + raise RedeliveryError( + "The github client returned a Bad Credential error, " + "please ensure credentials are set and have proper access rights." + ) from exc + except RateLimitExceededException as exc: + logging.error("Github rate limit exceeded error: %s", exc, exc_info=exc) + raise RedeliveryError( + "The github client is returning a Rate Limit Exceeded error, " + "please wait before retrying." + ) from exc + except GithubException as exc: + logging.error("Github API error: %s", exc, exc_info=exc) + raise RedeliveryError( + "The github client encountered an error. Please have a look at the logs." + ) from exc + + return _wrapper + + +@_github_api_exc_decorator +def _redeliver_failed_webhook_delivery_attempts( + github_auth: GithubAuthDetails, webhook_address: WebhookAddress, since_seconds: int +) -> int: + """Redeliver failed webhook deliveries since a certain number of seconds ago. + + Args: + github_auth: The GitHub authentication details used to interact with the Github API. + webhook_address: The data to identify the webhook. + since_seconds: The amount of seconds to look back for failed deliveries. + + Returns: + The number of failed webhook deliveries redelivered. + + """ + github = _get_github_client(github_auth) + + deliveries = _iter_delivery_attempts(github_client=github, webhook_address=webhook_address) + since_datetime = datetime.now(tz=timezone.utc) - timedelta(seconds=since_seconds) + failed_deliveries = _filter_for_failed_attempts_since( + deliveries=deliveries, since_datetime=since_datetime + ) + redelivered_count = _redeliver_attempts( + deliveries=failed_deliveries, github_client=github, webhook_address=webhook_address + ) + + return redelivered_count + + +# Github App authentication is not tested in unit tests, but in integration tests. +def _get_github_client(github_auth: GithubAuthDetails) -> Github: # pragma: no cover + """Get a Github client. + + Args: + github_auth: The Github authentication details. + + Returns: + The Github client. + """ + if isinstance(github_auth, GithubToken): + return Github(auth=Token(github_auth)) + + app_auth = AppAuth(app_id=github_auth.client_id, private_key=github_auth.private_key) + app_installation_auth = AppInstallationAuth( + app_auth=app_auth, installation_id=github_auth.installation_id + ) + return Github(auth=app_installation_auth) + + +def _iter_delivery_attempts( + github_client: Github, webhook_address: WebhookAddress +) -> Iterator[_WebhookDeliveryAttempt]: + """Iterate over webhook delivery attempts. + + Args: + github_client: The GitHub client used to interact with the Github API. + webhook_address: The data to identify the webhook. + """ + webhook_origin = ( + github_client.get_repo(f"{webhook_address.github_org}/{webhook_address.github_repo}") + if webhook_address.github_repo + else github_client.get_organization(webhook_address.github_org) + ) + deliveries = webhook_origin.get_hook_deliveries(webhook_address.id) + for delivery in deliveries: + # we check that the API is really returning the expected fields with non-null vals + # as pygithub is not doing this validation for us + required_fields = {"id", "status", "delivered_at", "event"} + none_fields = { + field for field in required_fields if getattr(delivery, field, None) is None + } + if none_fields: + raise AssertionError( + f"The webhook delivery {delivery.raw_data} is missing required fields:" + f" {none_fields}" + ) + yield _WebhookDeliveryAttempt( + id=delivery.id, # type: ignore + status=delivery.status, # type: ignore + delivered_at=delivery.delivered_at, # type: ignore + action=delivery.action, # type: ignore + event=delivery.event, # type: ignore + ) + + +def _filter_for_failed_attempts_since( + deliveries: Iterator[_WebhookDeliveryAttempt], since_datetime: datetime +) -> Iterator[_WebhookDeliveryAttempt]: + """Filter webhook delivery attempts for failed deliveries since a given time. + + Args: + deliveries: The webhook delivery attempts. + since_datetime: The time to look back for failed deliveries. + """ + for delivery in deliveries: + if delivery.delivered_at < since_datetime: + break + + if ( + delivery.status != OK_STATUS + and delivery.action == ROUTABLE_JOB_STATUS + and delivery.event == SUPPORTED_GITHUB_EVENT + ): + yield delivery + + +def _redeliver_attempts( + deliveries: Iterator[_WebhookDeliveryAttempt], + github_client: Github, + webhook_address: WebhookAddress, +) -> int: + """Redeliver failed webhook deliveries since a given time. + + Args: + deliveries: The webhook delivery attempts. + github_client: The GitHub client used to interact with the Github API. + webhook_address: The data to identify the webhook. + + Returns: + The number of failed webhook deliveries redelivered. + """ + deliver_count = 0 + for delivery in deliveries: + _redeliver_attempt( + github_client=github_client, webhook_address=webhook_address, delivery_id=delivery.id + ) + deliver_count += 1 + return deliver_count + + +def _redeliver_attempt( + github_client: Github, webhook_address: WebhookAddress, delivery_id: int +) -> None: + """Redeliver a webhook delivery. + + Args: + github_client: The GitHub client used to interact with the Github API. + webhook_address: The data to identify the webhook. + delivery_id: The identifier of the webhook delivery to redeliver. + """ + # pygithub doesn't support the endpoint so we have to use the requester directly to perform + # a raw request: https://pygithub.readthedocs.io/en/stable/utilities.html#raw-requests + path_base = ( + f"/repos/{webhook_address.github_org}/{webhook_address.github_repo}" + if webhook_address.github_repo + else f"/orgs/{webhook_address.github_org}" + ) + url = f"{path_base}/hooks/{webhook_address.id}/deliveries/{delivery_id}/attempts" + github_client.requester.requestJsonAndCheck("POST", url) + + +if __name__ == "__main__": # pragma: no cover this is checked by integration tests + try: + main() + except ArgParseError as exc: + print(f"{exc}", file=sys.stderr) + sys.exit(ARG_PARSE_ERROR_EXIT_CODE) + except RedeliveryError as exc: + logger.exception("Webhook redelivery failed: %s", exc) + sys.exit(REDELIVERY_ERROR_EXIT_CODE) diff --git a/webhook_router/parse.py b/webhook_router/parse.py index 316265d..e108424 100644 --- a/webhook_router/parse.py +++ b/webhook_router/parse.py @@ -8,9 +8,6 @@ from pydantic import BaseModel, HttpUrl -WORKFLOW_JOB = "workflow_job" - - ValidationResult = namedtuple("ValidationResult", ["is_valid", "msg"]) Labels = set[str] diff --git a/webhook_router/router.py b/webhook_router/router.py index 0f030d5..349dfd5 100644 --- a/webhook_router/router.py +++ b/webhook_router/router.py @@ -11,7 +11,7 @@ logger = logging.getLogger(__name__) -WORKFLOW_JOB = "workflow_job" +ROUTABLE_JOB_STATUS = JobStatus.QUEUED Flavor = str Label = str LabelCombinationIdentifier = tuple[Label, ...] @@ -98,7 +98,7 @@ def forward(job: Job, routing_table: RoutingTable) -> None: Raises: RouterError: If the job cannot be forwarded. """ - if job.status != JobStatus.QUEUED: + if job.status != ROUTABLE_JOB_STATUS: logger.debug("Received job with status %s. Ignoring.", job.status) return