From fe4def75dc0a316789cec068a574713a2b2af92f Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 12 Jul 2022 09:25:05 +0400 Subject: Add GitHub Artifact API View Adds an API route to fetch GitHub build artifacts through a GitHub app. Signed-off-by: Hassan Abouelela --- .gitignore | 3 + poetry.lock | 67 +++++- pydis_site/apps/api/github_utils.py | 183 ++++++++++++++++ pydis_site/apps/api/tests/test_github_utils.py | 287 +++++++++++++++++++++++++ pydis_site/apps/api/urls.py | 9 +- pydis_site/apps/api/views.py | 53 +++++ pydis_site/settings.py | 10 +- pyproject.toml | 1 + 8 files changed, 609 insertions(+), 4 deletions(-) create mode 100644 pydis_site/apps/api/github_utils.py create mode 100644 pydis_site/apps/api/tests/test_github_utils.py diff --git a/.gitignore b/.gitignore index 45073da5..4fc4417d 100644 --- a/.gitignore +++ b/.gitignore @@ -132,3 +132,6 @@ log.* # Mac/OSX .DS_Store + +# Private keys +*.pem diff --git a/poetry.lock b/poetry.lock index f6576fba..1bee4397 100644 --- a/poetry.lock +++ b/poetry.lock @@ -67,6 +67,17 @@ category = "main" optional = false python-versions = ">=3.6" +[[package]] +name = "cffi" +version = "1.15.1" +description = "Foreign Function Interface for Python calling C code." +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +pycparser = "*" + [[package]] name = "cfgv" version = "3.3.1" @@ -121,6 +132,25 @@ requests = ">=1.0.0" [package.extras] yaml = ["PyYAML (>=3.10)"] +[[package]] +name = "cryptography" +version = "37.0.4" +description = "cryptography is a package which provides cryptographic recipes and primitives to Python developers." +category = "main" +optional = false +python-versions = ">=3.6" + +[package.dependencies] +cffi = ">=1.12" + +[package.extras] +docs = ["sphinx (>=1.6.5,!=1.8.0,!=3.1.0,!=3.1.1)", "sphinx-rtd-theme"] +docstest = ["pyenchant (>=1.6.11)", "twine (>=1.12.0)", "sphinxcontrib-spelling (>=4.0.1)"] +pep8test = ["black", "flake8", "flake8-import-order", "pep8-naming"] +sdist = ["setuptools_rust (>=0.11.4)"] +ssh = ["bcrypt (>=3.1.5)"] +test = ["pytest (>=6.2.0)", "pytest-benchmark", "pytest-cov", "pytest-subtests", "pytest-xdist", "pretend", "iso8601", "pytz", "hypothesis (>=1.11.4,!=3.79.2)"] + [[package]] name = "distlib" version = "0.3.4" @@ -607,6 +637,14 @@ category = "dev" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" +[[package]] +name = "pycparser" +version = "2.21" +description = "C parser in Python" +category = "main" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" + [[package]] name = "pydocstyle" version = "6.1.1" @@ -637,6 +675,23 @@ category = "dev" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" +[[package]] +name = "pyjwt" +version = "2.4.0" +description = "JSON Web Token implementation in Python" +category = "main" +optional = false +python-versions = ">=3.6" + +[package.dependencies] +cryptography = {version = ">=3.3.1", optional = true, markers = "extra == \"crypto\""} + +[package.extras] +crypto = ["cryptography (>=3.3.1)"] +dev = ["sphinx", "sphinx-rtd-theme", "zope.interface", "cryptography (>=3.3.1)", "pytest (>=6.0.0,<7.0.0)", "coverage[toml] (==5.0.4)", "mypy", "pre-commit"] +docs = ["sphinx", "sphinx-rtd-theme", "zope.interface"] +tests = ["pytest (>=6.0.0,<7.0.0)", "coverage[toml] (==5.0.4)"] + [[package]] name = "python-dotenv" version = "0.17.1" @@ -876,7 +931,7 @@ testing = ["pytest (>=6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest- [metadata] lock-version = "1.1" python-versions = "3.9.*" -content-hash = "e71d10c3d478c5d99e842f4c449a093caa1d4b2d255eb0dfb19843c5265d4aca" +content-hash = "c656c07f40d32ee7d30c19a7084b40e1e851209a362a3fe882aa03c2fd286454" [metadata.files] anyio = [ @@ -896,6 +951,7 @@ certifi = [ {file = "certifi-2022.6.15-py3-none-any.whl", hash = "sha256:fe86415d55e84719d75f8b69414f6438ac3547d2078ab91b67e779ef69378412"}, {file = "certifi-2022.6.15.tar.gz", hash = "sha256:84c85a9078b11105f04f3036a9482ae10e4621616db313fe045dd24743a0820d"}, ] +cffi = [] cfgv = [ {file = "cfgv-3.3.1-py2.py3-none-any.whl", hash = "sha256:c6a0883f3917a037485059700b9e75da2464e6c27051014ad85ba6aaa5884426"}, {file = "cfgv-3.3.1.tar.gz", hash = "sha256:f5a830efb9ce7a445376bb66ec94c638a9787422f96264c98edc6bdeed8ab736"}, @@ -963,6 +1019,7 @@ coveralls = [ {file = "coveralls-2.2.0-py2.py3-none-any.whl", hash = "sha256:2301a19500b06649d2ec4f2858f9c69638d7699a4c63027c5d53daba666147cc"}, {file = "coveralls-2.2.0.tar.gz", hash = "sha256:b990ba1f7bc4288e63340be0433698c1efe8217f78c689d254c2540af3d38617"}, ] +cryptography = [] distlib = [ {file = "distlib-0.3.4-py2.py3-none-any.whl", hash = "sha256:6564fe0a8f51e734df6333d08b8b94d4ea8ee6b99b5ed50613f731fd4089f34b"}, {file = "distlib-0.3.4.zip", hash = "sha256:e4b58818180336dc9c529bfb9a0b58728ffc09ad92027a3f30b7cd91e3458579"}, @@ -1157,6 +1214,10 @@ pycodestyle = [ {file = "pycodestyle-2.7.0-py2.py3-none-any.whl", hash = "sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068"}, {file = "pycodestyle-2.7.0.tar.gz", hash = "sha256:c389c1d06bf7904078ca03399a4816f974a1d590090fecea0c63ec26ebaf1cef"}, ] +pycparser = [ + {file = "pycparser-2.21-py2.py3-none-any.whl", hash = "sha256:8ee45429555515e1f6b185e78100aea234072576aa43ab53aefcae078162fca9"}, + {file = "pycparser-2.21.tar.gz", hash = "sha256:e644fdec12f7872f86c58ff790da456218b10f863970249516d60a5eaca77206"}, +] pydocstyle = [ {file = "pydocstyle-6.1.1-py3-none-any.whl", hash = "sha256:6987826d6775056839940041beef5c08cc7e3d71d63149b48e36727f70144dc4"}, {file = "pydocstyle-6.1.1.tar.gz", hash = "sha256:1d41b7c459ba0ee6c345f2eb9ae827cab14a7533a88c5c6f7e94923f72df92dc"}, @@ -1166,6 +1227,10 @@ pyflakes = [ {file = "pyflakes-2.3.1-py2.py3-none-any.whl", hash = "sha256:7893783d01b8a89811dd72d7dfd4d84ff098e5eed95cfa8905b22bbffe52efc3"}, {file = "pyflakes-2.3.1.tar.gz", hash = "sha256:f5bc8ecabc05bb9d291eb5203d6810b49040f6ff446a756326104746cc00c1db"}, ] +pyjwt = [ + {file = "PyJWT-2.4.0-py3-none-any.whl", hash = "sha256:72d1d253f32dbd4f5c88eaf1fdc62f3a19f676ccbadb9dbc5d07e951b2b26daf"}, + {file = "PyJWT-2.4.0.tar.gz", hash = "sha256:d42908208c699b3b973cbeb01a969ba6a96c821eefb1c5bfe4c390c01d67abba"}, +] python-dotenv = [ {file = "python-dotenv-0.17.1.tar.gz", hash = "sha256:b1ae5e9643d5ed987fc57cc2583021e38db531946518130777734f9589b3141f"}, {file = "python_dotenv-0.17.1-py2.py3-none-any.whl", hash = "sha256:00aa34e92d992e9f8383730816359647f358f4a3be1ba45e5a5cefd27ee91544"}, diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py new file mode 100644 index 00000000..70dccdff --- /dev/null +++ b/pydis_site/apps/api/github_utils.py @@ -0,0 +1,183 @@ +"""Utilities for working with the GitHub API.""" + +import asyncio +import datetime +import math + +import httpx +import jwt +from asgiref.sync import async_to_sync + +from pydis_site import settings + +MAX_POLLS = 20 +"""The maximum number of attempts at fetching a workflow run.""" + + +class ArtifactProcessingError(Exception): + """Base exception for other errors related to processing a GitHub artifact.""" + + status: int + + +class UnauthorizedError(ArtifactProcessingError): + """The application does not have permission to access the requested repo.""" + + status = 401 + + +class NotFoundError(ArtifactProcessingError): + """The requested resource could not be found.""" + + status = 404 + + +class ActionFailedError(ArtifactProcessingError): + """The requested workflow did not conclude successfully.""" + + status = 400 + + +class RunTimeoutError(ArtifactProcessingError): + """The requested workflow run was not ready in time.""" + + status = 408 + + +def generate_token() -> str: + """ + Generate a JWT token to access the GitHub API. + + The token is valid for roughly 10 minutes after generation, before the API starts + returning 401s. + + Refer to: + https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#authenticating-as-a-github-app + """ + now = datetime.datetime.now() + return jwt.encode( + { + "iat": math.floor((now - datetime.timedelta(seconds=60)).timestamp()), # Issued at + "exp": math.floor((now + datetime.timedelta(minutes=9)).timestamp()), # Expires at + "iss": settings.GITHUB_OAUTH_APP_ID, + }, + settings.GITHUB_OAUTH_KEY, + algorithm="RS256" + ) + + +async def authorize(owner: str, repo: str) -> httpx.AsyncClient: + """ + Get an access token for the requested repository. + + The process is roughly: + - GET app/installations to get a list of all app installations + - POST to get a token to access the given app + - GET installation/repositories and check if the requested one is part of those + """ + client = httpx.AsyncClient( + base_url=settings.GITHUB_API, + headers={"Authorization": f"bearer {generate_token()}"}, + timeout=settings.TIMEOUT_PERIOD, + ) + + try: + # Get a list of app installations we have access to + apps = await client.get("app/installations") + apps.raise_for_status() + + for app in apps.json(): + # Look for an installation with the right owner + if app["account"]["login"] != owner: + continue + + # Get the repositories of the specified owner + app_token = await client.post(app["access_tokens_url"]) + app_token.raise_for_status() + client.headers["Authorization"] = f"bearer {app_token.json()['token']}" + + repos = await client.get("installation/repositories") + repos.raise_for_status() + + # Search for the request repository + for accessible_repo in repos.json()["repositories"]: + if accessible_repo["name"] == repo: + # We've found the correct repository, and it's accessible with the current auth + return client + + raise NotFoundError( + "Could not find the requested repository. Make sure the application can access it." + ) + + except BaseException as e: + # Close the client if we encountered an unexpected exception + await client.aclose() + raise e + + +async def wait_for_run(client: httpx.AsyncClient, run: dict) -> str: + """Wait for the provided `run` to finish, and return the URL to its artifacts.""" + polls = 0 + while polls <= MAX_POLLS: + if run["status"] != "completed": + # The action is still processing, wait a bit longer + polls += 1 + await asyncio.sleep(10) + + elif run["conclusion"] != "success": + # The action failed, or did not run + raise ActionFailedError(f"The requested workflow ended with: {run['conclusion']}") + + else: + # The desired action was found, and it ended successfully + return run["artifacts_url"] + + run = await client.get(run["url"]) + run.raise_for_status() + run = run.json() + + raise RunTimeoutError("The requested workflow was not ready in time.") + + +@async_to_sync +async def get_artifact( + owner: str, repo: str, sha: str, action_name: str, artifact_name: str +) -> str: + """Get a download URL for a build artifact.""" + client = await authorize(owner, repo) + + try: + # Get the workflow runs for this repository + runs = await client.get(f"/repos/{owner}/{repo}/actions/runs", params={"per_page": 100}) + runs.raise_for_status() + runs = runs.json() + + # Filter the runs for the one associated with the given SHA + for run in runs["workflow_runs"]: + if run["name"] == action_name and sha == run["head_sha"]: + break + else: + raise NotFoundError( + "Could not find a run matching the provided settings in the previous hundred runs." + ) + + # Wait for the workflow to finish + url = await wait_for_run(client, run) + + # Filter the artifacts, and return the download URL + artifacts = await client.get(url) + artifacts.raise_for_status() + + for artifact in artifacts.json()["artifacts"]: + if artifact["name"] == artifact_name: + data = await client.get(artifact["archive_download_url"]) + if data.status_code == 302: + return str(data.next_request.url) + + # The following line is left untested since it should in theory be impossible + data.raise_for_status() # pragma: no cover + + raise NotFoundError("Could not find an artifact matching the provided name.") + + finally: + await client.aclose() diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py new file mode 100644 index 00000000..dc17d609 --- /dev/null +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -0,0 +1,287 @@ +import asyncio +import datetime +import random +import unittest +from unittest import mock + +import django.test +import httpx +import jwt +import rest_framework.response +import rest_framework.test +from django.urls import reverse + +from .. import github_utils + + +def patched_raise_for_status(response: httpx.Response): + """Fake implementation of raise_for_status which does not need a request to be set.""" + if response.status_code // 100 != 2: # pragma: no cover + raise httpx.HTTPStatusError( + f"Non 2xx response code: {response.status_code}", + request=getattr(response, "_request", httpx.Request("GET", "")), + response=response + ) + + +class GeneralUtilityTests(unittest.TestCase): + """Test the utility methods which do not fit in another class.""" + + def test_token_generation(self): + """Test that the a valid JWT token is generated.""" + def encode(payload: dict, _: str, algorithm: str, *args, **kwargs) -> str: + """ + Intercept the encode method. + + It is performed with an algorithm which does not require a PEM key, as it may + not be available in testing environments. + """ + self.assertEqual("RS256", algorithm, "The GitHub App JWT must be signed using RS256.") + return original_encode( + payload, "secret-encoding-key", algorithm="HS256", *args, **kwargs + ) + + original_encode = jwt.encode + with mock.patch("jwt.encode", new=encode): + token = github_utils.generate_token() + decoded = jwt.decode(token, "secret-encoding-key", algorithms=["HS256"]) + + delta = datetime.timedelta(minutes=10) + self.assertAlmostEqual(decoded["exp"] - decoded["iat"], delta.total_seconds()) + self.assertLess(decoded["exp"], (datetime.datetime.now() + delta).timestamp()) + + +@mock.patch("httpx.AsyncClient", autospec=True) +@mock.patch("asyncio.sleep", new=mock.AsyncMock(return_value=asyncio.Future)) +@mock.patch("httpx.Response.raise_for_status", new=patched_raise_for_status) +class WaitForTests(unittest.IsolatedAsyncioTestCase): + """Tests the wait_for utility.""" + + async def test_wait_for_successful_run(self, client_mock: mock.Mock): + """Test that the wait_for method handles successfully runs.""" + final_url = "some_url" + str(random.randint(0, 10)) + + client_mock.get.side_effect = responses = [ + httpx.Response(200, json={"status": "queued", "url": ""}), + httpx.Response(200, json={"status": "pending", "url": ""}), + httpx.Response(200, json={ + "status": "completed", + "conclusion": "success", + "url": "", + "artifacts_url": final_url + }) + ] + + result = await github_utils.wait_for_run(client_mock, responses[0].json()) + self.assertEqual(final_url, result) + + async def test_wait_for_failed_run(self, client_mock: mock.Mock): + """Test that the wait_for method handles failed runs.""" + client_mock.get.return_value = httpx.Response(200, json={ + "status": "completed", + "conclusion": "failed", + }) + + with self.assertRaises(github_utils.ActionFailedError): + await github_utils.wait_for_run(client_mock, {"status": "pending", "url": ""}) + + async def test_wait_for_timeout(self, client_mock: mock.Mock): + """Test that the wait_for method quits after a few attempts.""" + client_mock.get.side_effect = responses = [ + httpx.Response(200, json={"status": "pending", "url": ""}) + ] * (github_utils.MAX_POLLS + 5) + + with self.assertRaises(github_utils.RunTimeoutError): + await github_utils.wait_for_run(client_mock, responses[0].json()) + + +async def get_response_authorize( + _: httpx.AsyncClient, request: httpx.Request, **__ +) -> httpx.Response: + """ + Helper method for the authorize tests. + + Requests are intercepted before being sent out, and the appropriate responses are returned. + """ + path = request.url.path + auth = request.headers.get("Authorization") + + if request.method == "GET": + if path == "/app/installations": + if auth == "bearer JWT initial token": + return httpx.Response(200, request=request, json=[{ + "account": {"login": "VALID_OWNER"}, + "access_tokens_url": "https://example.com/ACCESS_TOKEN_URL" + }]) + else: + return httpx.Response( + 401, json={"error": "auth app/installations"}, request=request + ) + + elif path == "/installation/repositories": + if auth == "bearer app access token": + return httpx.Response(200, request=request, json={ + "repositories": [{ + "name": "VALID_REPO" + }] + }) + else: # pragma: no cover + return httpx.Response( + 401, json={"error": "auth installation/repositories"}, request=request + ) + + elif request.method == "POST": + if path == "/ACCESS_TOKEN_URL": + if auth == "bearer JWT initial token": + return httpx.Response(200, request=request, json={"token": "app access token"}) + else: # pragma: no cover + return httpx.Response(401, json={"error": "auth access_token"}, request=request) + + # Reaching this point means something has gone wrong + return httpx.Response(500, request=request) # pragma: no cover + + +@mock.patch("httpx.AsyncClient.send", new=get_response_authorize) +@mock.patch.object(github_utils, "generate_token", new=mock.Mock(return_value="JWT initial token")) +class AuthorizeTests(unittest.IsolatedAsyncioTestCase): + """Test the authorize utility.""" + + async def test_invalid_apps_auth(self): + """Test that an exception is raised if authorization was attempted with an invalid token.""" + with mock.patch.object(github_utils, "generate_token", return_value="Invalid token"): + with self.assertRaises(httpx.HTTPStatusError) as error: + await github_utils.authorize("VALID_OWNER", "VALID_REPO") + + exception: httpx.HTTPStatusError = error.exception + self.assertEqual(401, exception.response.status_code) + self.assertEqual("auth app/installations", exception.response.json()["error"]) + + async def test_missing_repo(self): + """Test that an exception is raised when the selected owner or repo are not available.""" + with self.assertRaises(github_utils.NotFoundError): + await github_utils.authorize("INVALID_OWNER", "VALID_REPO") + with self.assertRaises(github_utils.NotFoundError): + await github_utils.authorize("VALID_OWNER", "INVALID_REPO") + + async def test_valid_authorization(self): + """Test that an accessible repository can be accessed.""" + client = await github_utils.authorize("VALID_OWNER", "VALID_REPO") + self.assertEqual("bearer app access token", client.headers.get("Authorization")) + + +async def get_response_get_artifact(request: httpx.Request, **_) -> httpx.Response: + """ + Helper method for the get_artifact tests. + + Requests are intercepted before being sent out, and the appropriate responses are returned. + """ + path = request.url.path + + if "force_error" in path: + return httpx.Response(404, request=request) + + if request.method == "GET": + if path == "/repos/owner/repo/actions/runs": + return httpx.Response(200, request=request, json={"workflow_runs": [{ + "name": "action_name", + "head_sha": "action_sha" + }]}) + elif path == "/artifact_url": + return httpx.Response(200, request=request, json={"artifacts": [{ + "name": "artifact_name", + "archive_download_url": "artifact_download_url" + }]}) + elif path == "/artifact_download_url": + response = httpx.Response(302, request=request) + response.next_request = httpx.Request("GET", httpx.URL("https://final_download.url")) + return response + + # Reaching this point means something has gone wrong + return httpx.Response(500, request=request) # pragma: no cover + + +class ArtifactFetcherTests(unittest.IsolatedAsyncioTestCase): + """Test the get_artifact utility.""" + + def setUp(self) -> None: + self.call_args = ["owner", "repo", "action_sha", "action_name", "artifact_name"] + self.client = httpx.AsyncClient(base_url="https://example.com") + + self.patchers = [ + mock.patch.object(self.client, "send", new=get_response_get_artifact), + mock.patch.object(github_utils, "authorize", return_value=self.client), + mock.patch.object(github_utils, "wait_for_run", return_value="artifact_url"), + ] + + for patcher in self.patchers: + patcher.start() + + def tearDown(self) -> None: + for patcher in self.patchers: + patcher.stop() + + def test_client_closed_on_errors(self): + """Test that the client is terminated even if an error occurs at some point.""" + self.call_args[0] = "force_error" + with self.assertRaises(httpx.HTTPStatusError): + github_utils.get_artifact(*self.call_args) + self.assertTrue(self.client.is_closed) + + def test_missing(self): + """Test that an exception is raised if the requested artifact was not found.""" + cases = ( + "invalid sha", + "invalid action name", + "invalid artifact name", + ) + for i, name in enumerate(cases, 2): + with self.subTest(f"Test {name} raises an error"): + new_args = self.call_args.copy() + new_args[i] = name + + with self.assertRaises(github_utils.NotFoundError): + github_utils.get_artifact(*new_args) + + def test_valid(self): + """Test that the correct download URL is returned for valid requests.""" + url = github_utils.get_artifact(*self.call_args) + self.assertEqual("https://final_download.url", url) + self.assertTrue(self.client.is_closed) + + +@mock.patch.object(github_utils, "get_artifact") +class GitHubArtifactViewTests(django.test.TestCase): + """Test the GitHub artifact fetch API view.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + + cls.kwargs = { + "owner": "test_owner", + "repo": "test_repo", + "sha": "test_sha", + "action_name": "test_action", + "artifact_name": "test_artifact", + } + cls.url = reverse("api:github-artifacts", kwargs=cls.kwargs) + + async def test_successful(self, artifact_mock: mock.Mock): + """Test a proper response is returned with proper input.""" + artifact_mock.return_value = "final download url" + result = self.client.get(self.url) + + self.assertIsInstance(result, rest_framework.response.Response) + self.assertEqual({"url": artifact_mock.return_value}, result.data) + + async def test_failed_fetch(self, artifact_mock: mock.Mock): + """Test that a proper error is returned when the request fails.""" + artifact_mock.side_effect = github_utils.NotFoundError("Test error message") + result = self.client.get(self.url) + + self.assertIsInstance(result, rest_framework.response.Response) + self.assertEqual({ + "error_type": github_utils.NotFoundError.__name__, + "error": "Test error message", + "requested_resource": "/".join(self.kwargs.values()) + }, result.data) diff --git a/pydis_site/apps/api/urls.py b/pydis_site/apps/api/urls.py index 1e564b29..2757f176 100644 --- a/pydis_site/apps/api/urls.py +++ b/pydis_site/apps/api/urls.py @@ -1,7 +1,7 @@ from django.urls import include, path from rest_framework.routers import DefaultRouter -from .views import HealthcheckView, RulesView +from .views import GitHubArtifactsView, HealthcheckView, RulesView from .viewsets import ( AocAccountLinkViewSet, AocCompletionistBlockViewSet, @@ -86,5 +86,10 @@ urlpatterns = ( # from django_hosts.resolvers import reverse path('bot/', include((bot_router.urls, 'api'), namespace='bot')), path('healthcheck', HealthcheckView.as_view(), name='healthcheck'), - path('rules', RulesView.as_view(), name='rules') + path('rules', RulesView.as_view(), name='rules'), + path( + 'github/artifact/////', + GitHubArtifactsView.as_view(), + name="github-artifacts" + ), ) diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index 816463f6..ad2d948e 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -1,7 +1,10 @@ from rest_framework.exceptions import ParseError +from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView +from . import github_utils + class HealthcheckView(APIView): """ @@ -152,3 +155,53 @@ class RulesView(APIView): "Do not offer or ask for paid work of any kind." ), ]) + + +class GitHubArtifactsView(APIView): + """ + Provides utilities for interacting with the GitHub API and obtaining action artifacts. + + ## Routes + ### GET /github/artifacts + Returns a download URL for the artifact requested. + + { + 'url': 'https://pipelines.actions.githubusercontent.com/...' + } + + ### Exceptions + In case of an error, the following body will be returned: + + { + "error_type": "", + "error": "", + "requested_resource": "///" + } + + ## Authentication + Does not require any authentication nor permissions. + """ + + authentication_classes = () + permission_classes = () + + def get( + self, + request: Request, + *, + owner: str, + repo: str, + sha: str, + action_name: str, + artifact_name: str + ) -> Response: + """Return a download URL for the requested artifact.""" + try: + url = github_utils.get_artifact(owner, repo, sha, action_name, artifact_name) + return Response({"url": url}) + except github_utils.ArtifactProcessingError as e: + return Response({ + "error_type": e.__class__.__name__, + "error": str(e), + "requested_resource": f"{owner}/{repo}/{sha}/{action_name}/{artifact_name}" + }, status=e.status) diff --git a/pydis_site/settings.py b/pydis_site/settings.py index 03c16f4b..f382b052 100644 --- a/pydis_site/settings.py +++ b/pydis_site/settings.py @@ -21,7 +21,6 @@ import environ import sentry_sdk from sentry_sdk.integrations.django import DjangoIntegration - env = environ.Env( DEBUG=(bool, False), SITE_DSN=(str, ""), @@ -30,10 +29,19 @@ env = environ.Env( GIT_SHA=(str, 'development'), TIMEOUT_PERIOD=(int, 5), GITHUB_TOKEN=(str, None), + GITHUB_OAUTH_APP_ID=(str, None), + GITHUB_OAUTH_KEY=(str, None), ) GIT_SHA = env("GIT_SHA") +GITHUB_API = "https://api.github.com" GITHUB_TOKEN = env("GITHUB_TOKEN") +GITHUB_OAUTH_APP_ID = env("GITHUB_OAUTH_APP_ID") +GITHUB_OAUTH_KEY = env("GITHUB_OAUTH_KEY") + +if GITHUB_OAUTH_KEY and (oauth_file := Path(GITHUB_OAUTH_KEY)).is_file(): + # Allow the OAuth key to be loaded from a file + GITHUB_OAUTH_KEY = oauth_file.read_text(encoding="utf-8") sentry_sdk.init( dsn=env('SITE_DSN'), diff --git a/pyproject.toml b/pyproject.toml index 467fc8bc..1c24d308 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,7 @@ markdown = "~=3.3.4" python-frontmatter = "~=1.0" django-prometheus = "~=2.1" django-distill = "~=2.9.0" +PyJWT = {version = "~=2.4.0", extras = ["crypto"]} [tool.poetry.dev-dependencies] coverage = "~=5.0" -- cgit v1.2.3 From 0404e0040a0a6fc4f4520bfdc14b800390800382 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 12 Jul 2022 09:52:06 +0400 Subject: Update Netlify Build Script Update the netlify build script to use the artifacts API from the site. Signed-off-by: Hassan Abouelela --- static-builds/README.md | 24 +++++--- static-builds/netlify_build.py | 125 +++++++++-------------------------------- 2 files changed, 45 insertions(+), 104 deletions(-) diff --git a/static-builds/README.md b/static-builds/README.md index 9b86ed08..a3c7962b 100644 --- a/static-builds/README.md +++ b/static-builds/README.md @@ -27,16 +27,29 @@ Alternatively, you can use the [Dockerfile](/Dockerfile) and extract the build. Both output their builds to a `build/` directory. ### Deploying To Netlify -To deploy to netlify, link your site GitHub repository to a netlify site, and use the following settings: +To deploy to netlify, link your site GitHub repository to a netlify site, and use the settings below. +The netlify build script uses the site API to fetch and download the artifact, using a GitHub app that +can access the repo. The app must have the `actions` and `artifacts` scopes enabled. +### Netlify Settings Build Command: -`python -m pip install httpx==0.19.0 && python static-builds/netlify_build.py` +`python -m pip install httpx==0.23.0 && python static-builds/netlify_build.py` Publish Directory: `build` -Environment Variables: -- PYTHON_VERSION: 3.8 +**Environment Variables** + +| Name | Value | Description | +|----------------|--------------------------------|-------------------------------------------------------------------------------------------| +| PYTHON_VERSION | 3.8 | The python version. Supported options are defined by netlify [here][netlify build image]. | +| API_URL | https://pythondiscord.com/ | The link to the API, which will be used to fetch the build artifacts. | +| ACTION_NAME | Build & Publish Static Preview | The name of the workflow which will be used to find the artifact. | +| ARTIFACT_NAME | static-build | The name of the artifact to download. | + + +[netlify build image]: https://github.com/netlify/build-image/tree/focal + Note that at this time, if you are deploying to netlify yourself, you won't have access to the @@ -45,6 +58,3 @@ You can either update the pack to one which will work on your domain, or you'll > Warning: If you are modifying the [build script](./netlify_build.py), make sure it is compatible with Python 3.8. - -Note: The build script uses [nightly.link](https://github.com/oprypin/nightly.link) -to fetch the artifact with no authentication. diff --git a/static-builds/netlify_build.py b/static-builds/netlify_build.py index 4e1e6106..13cd0279 100644 --- a/static-builds/netlify_build.py +++ b/static-builds/netlify_build.py @@ -4,106 +4,42 @@ # This script performs all the actions required to build and deploy our project on netlify # It depends on the following packages, which are set in the netlify UI: -# httpx == 0.19.0 +# httpx == 0.23.0 +import json import os -import time -import typing import zipfile from pathlib import Path from urllib import parse import httpx -API_URL = "https://api.github.com" -NIGHTLY_URL = "https://nightly.link" -OWNER, REPO = parse.urlparse(os.getenv("REPOSITORY_URL")).path.lstrip("/").split("/")[0:2] - - -def get_build_artifact() -> typing.Tuple[int, str]: - """ - Search for a build artifact, and return the result. - - The return is a tuple of the check suite ID, and the URL to the artifacts. - """ - print("Fetching build URL.") - - if os.getenv("PULL_REQUEST").lower() == "true": - print(f"Fetching data for PR #{os.getenv('REVIEW_ID')}") - - pull_url = f"{API_URL}/repos/{OWNER}/{REPO}/pulls/{os.getenv('REVIEW_ID')}" - pull_request = httpx.get(pull_url) - pull_request.raise_for_status() - - commit_sha = pull_request.json()["head"]["sha"] - - workflows_params = parse.urlencode({ - "event": "pull_request", - "per_page": 100 - }) - - else: - commit_sha = os.getenv("COMMIT_REF") - - workflows_params = parse.urlencode({ - "event": "push", - "per_page": 100 - }) - - print(f"Fetching action data for commit {commit_sha}") - - workflows = httpx.get(f"{API_URL}/repos/{OWNER}/{REPO}/actions/runs?{workflows_params}") - workflows.raise_for_status() - - for run in workflows.json()["workflow_runs"]: - if run["name"] == "Build & Publish Static Preview" and commit_sha == run["head_sha"]: - print(f"Found action for this commit: {run['id']}\n{run['html_url']}") - break - else: - raise Exception("Could not find the workflow run for this event.") - - polls = 0 - while polls <= 20: - if run["status"] != "completed": - print("Action isn't ready, sleeping for 10 seconds.") - polls += 1 - time.sleep(10) - - elif run["conclusion"] != "success": - print("Aborting build due to a failure in a previous CI step.") - exit(0) - - else: - print(f"Found artifact URL:\n{run['artifacts_url']}") - return run["check_suite_id"], run["artifacts_url"] - - _run = httpx.get(run["url"]) - _run.raise_for_status() - run = _run.json() - - raise Exception("Polled for the artifact workflow, but it was not ready in time.") - - -def download_artifact(suite_id: int, url: str) -> None: - """Download a build artifact from `url`, and unzip the content.""" - print("Fetching artifact data.") - - artifacts = httpx.get(url) - artifacts.raise_for_status() - artifacts = artifacts.json() - - if artifacts["total_count"] == "0": - raise Exception(f"No artifacts were found for this build, aborting.\n{url}") - - for artifact in artifacts["artifacts"]: - if artifact["name"] == "static-build": - print("Found artifact with build.") - break - else: - raise Exception("Could not find an artifact with the expected name.") - - artifact_url = f"{NIGHTLY_URL}/{OWNER}/{REPO}/suites/{suite_id}/artifacts/{artifact['id']}" - zipped_content = httpx.get(artifact_url) +if __name__ == "__main__": + owner, repo = parse.urlparse(os.getenv("REPOSITORY_URL")).path.lstrip("/").split("/")[0:2] + + download_url = "/".join([ + os.getenv("API_URL").rstrip("/"), + "api/github/artifact", + owner, + repo, + os.getenv("COMMIT_REF"), + parse.quote(os.getenv("ACTION_NAME")), + os.getenv("ARTIFACT_NAME"), + ]) + print(f"Fetching download URL from {download_url}") + response = httpx.get(download_url, follow_redirects=True) + + if response.status_code != 200: + try: + print(response.json()) + except json.JSONDecodeError: + pass + + response.raise_for_status() + + url = response.json()["url"] + print(f"Downloading build from {url}") + zipped_content = httpx.get(url, follow_redirects=True) zipped_content.raise_for_status() zip_file = Path("temp.zip") @@ -115,8 +51,3 @@ def download_artifact(suite_id: int, url: str) -> None: zip_file.unlink(missing_ok=True) print("Wrote artifact content to target directory.") - - -if __name__ == "__main__": - print("Build started") - download_artifact(*get_build_artifact()) -- cgit v1.2.3 From 26a3c19b53883015e8ba87db2a668c3eece2ce20 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 12 Jul 2022 14:45:22 +0400 Subject: Make Awaiting Workflow Run A User Responsibility Moves the responsibility of re-requesting a workflow run from the API to the user. This makes the requests much shorter-lived, and allows the client to control how they want to handle sleeping and retrying. This also has the benefit of removing the only real piece of async code, so now the view is completely sync once again. Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/github_utils.py | 79 +++++------ pydis_site/apps/api/tests/test_github_utils.py | 174 ++++++++++++------------- static-builds/netlify_build.py | 9 +- 3 files changed, 131 insertions(+), 131 deletions(-) diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py index 70dccdff..707b36e5 100644 --- a/pydis_site/apps/api/github_utils.py +++ b/pydis_site/apps/api/github_utils.py @@ -1,17 +1,17 @@ """Utilities for working with the GitHub API.""" -import asyncio import datetime import math import httpx import jwt -from asgiref.sync import async_to_sync from pydis_site import settings -MAX_POLLS = 20 -"""The maximum number of attempts at fetching a workflow run.""" +MAX_RUN_TIME = datetime.timedelta(minutes=3) +"""The maximum time allowed before an action is declared timed out.""" +ISO_FORMAT_STRING = "%Y-%m-%dT%H:%M:%SZ" +"""The datetime string format GitHub uses.""" class ArtifactProcessingError(Exception): @@ -44,6 +44,12 @@ class RunTimeoutError(ArtifactProcessingError): status = 408 +class RunPendingError(ArtifactProcessingError): + """The requested workflow run is still pending, try again later.""" + + status = 202 + + def generate_token() -> str: """ Generate a JWT token to access the GitHub API. @@ -66,7 +72,7 @@ def generate_token() -> str: ) -async def authorize(owner: str, repo: str) -> httpx.AsyncClient: +def authorize(owner: str, repo: str) -> httpx.Client: """ Get an access token for the requested repository. @@ -75,7 +81,7 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient: - POST to get a token to access the given app - GET installation/repositories and check if the requested one is part of those """ - client = httpx.AsyncClient( + client = httpx.Client( base_url=settings.GITHUB_API, headers={"Authorization": f"bearer {generate_token()}"}, timeout=settings.TIMEOUT_PERIOD, @@ -83,7 +89,7 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient: try: # Get a list of app installations we have access to - apps = await client.get("app/installations") + apps = client.get("app/installations") apps.raise_for_status() for app in apps.json(): @@ -92,11 +98,11 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient: continue # Get the repositories of the specified owner - app_token = await client.post(app["access_tokens_url"]) + app_token = client.post(app["access_tokens_url"]) app_token.raise_for_status() client.headers["Authorization"] = f"bearer {app_token.json()['token']}" - repos = await client.get("installation/repositories") + repos = client.get("installation/repositories") repos.raise_for_status() # Search for the request repository @@ -111,44 +117,39 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient: except BaseException as e: # Close the client if we encountered an unexpected exception - await client.aclose() + client.close() raise e -async def wait_for_run(client: httpx.AsyncClient, run: dict) -> str: - """Wait for the provided `run` to finish, and return the URL to its artifacts.""" - polls = 0 - while polls <= MAX_POLLS: - if run["status"] != "completed": - # The action is still processing, wait a bit longer - polls += 1 - await asyncio.sleep(10) - - elif run["conclusion"] != "success": - # The action failed, or did not run - raise ActionFailedError(f"The requested workflow ended with: {run['conclusion']}") +def check_run_status(run: dict) -> str: + """Check if the provided run has been completed, otherwise raise an exception.""" + created_at = datetime.datetime.strptime(run["created_at"], ISO_FORMAT_STRING) + run_time = datetime.datetime.now() - created_at + if run["status"] != "completed": + if run_time <= MAX_RUN_TIME: + raise RunPendingError( + f"The requested run is still pending. It was created " + f"{run_time.seconds // 60}:{run_time.seconds % 60 :>02} minutes ago." + ) else: - # The desired action was found, and it ended successfully - return run["artifacts_url"] + raise RunTimeoutError("The requested workflow was not ready in time.") - run = await client.get(run["url"]) - run.raise_for_status() - run = run.json() + if run["conclusion"] != "success": + # The action failed, or did not run + raise ActionFailedError(f"The requested workflow ended with: {run['conclusion']}") - raise RunTimeoutError("The requested workflow was not ready in time.") + # The requested action is ready + return run["artifacts_url"] -@async_to_sync -async def get_artifact( - owner: str, repo: str, sha: str, action_name: str, artifact_name: str -) -> str: +def get_artifact(owner: str, repo: str, sha: str, action_name: str, artifact_name: str) -> str: """Get a download URL for a build artifact.""" - client = await authorize(owner, repo) + client = authorize(owner, repo) try: # Get the workflow runs for this repository - runs = await client.get(f"/repos/{owner}/{repo}/actions/runs", params={"per_page": 100}) + runs = client.get(f"/repos/{owner}/{repo}/actions/runs", params={"per_page": 100}) runs.raise_for_status() runs = runs.json() @@ -161,16 +162,16 @@ async def get_artifact( "Could not find a run matching the provided settings in the previous hundred runs." ) - # Wait for the workflow to finish - url = await wait_for_run(client, run) + # Check the workflow status + url = check_run_status(run) # Filter the artifacts, and return the download URL - artifacts = await client.get(url) + artifacts = client.get(url) artifacts.raise_for_status() for artifact in artifacts.json()["artifacts"]: if artifact["name"] == artifact_name: - data = await client.get(artifact["archive_download_url"]) + data = client.get(artifact["archive_download_url"]) if data.status_code == 302: return str(data.next_request.url) @@ -180,4 +181,4 @@ async def get_artifact( raise NotFoundError("Could not find an artifact matching the provided name.") finally: - await client.aclose() + client.close() diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py index dc17d609..78f2979d 100644 --- a/pydis_site/apps/api/tests/test_github_utils.py +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -1,6 +1,4 @@ -import asyncio import datetime -import random import unittest from unittest import mock @@ -14,16 +12,6 @@ from django.urls import reverse from .. import github_utils -def patched_raise_for_status(response: httpx.Response): - """Fake implementation of raise_for_status which does not need a request to be set.""" - if response.status_code // 100 != 2: # pragma: no cover - raise httpx.HTTPStatusError( - f"Non 2xx response code: {response.status_code}", - request=getattr(response, "_request", httpx.Request("GET", "")), - response=response - ) - - class GeneralUtilityTests(unittest.TestCase): """Test the utility methods which do not fit in another class.""" @@ -51,53 +39,50 @@ class GeneralUtilityTests(unittest.TestCase): self.assertLess(decoded["exp"], (datetime.datetime.now() + delta).timestamp()) -@mock.patch("httpx.AsyncClient", autospec=True) -@mock.patch("asyncio.sleep", new=mock.AsyncMock(return_value=asyncio.Future)) -@mock.patch("httpx.Response.raise_for_status", new=patched_raise_for_status) -class WaitForTests(unittest.IsolatedAsyncioTestCase): - """Tests the wait_for utility.""" - - async def test_wait_for_successful_run(self, client_mock: mock.Mock): - """Test that the wait_for method handles successfully runs.""" - final_url = "some_url" + str(random.randint(0, 10)) - - client_mock.get.side_effect = responses = [ - httpx.Response(200, json={"status": "queued", "url": ""}), - httpx.Response(200, json={"status": "pending", "url": ""}), - httpx.Response(200, json={ - "status": "completed", - "conclusion": "success", - "url": "", - "artifacts_url": final_url - }) - ] +class WaitForTests(unittest.TestCase): + """Tests the check_run_status utility.""" - result = await github_utils.wait_for_run(client_mock, responses[0].json()) - self.assertEqual(final_url, result) + def test_completed_run(self): + final_url = "some_url_string_1234" - async def test_wait_for_failed_run(self, client_mock: mock.Mock): - """Test that the wait_for method handles failed runs.""" - client_mock.get.return_value = httpx.Response(200, json={ + result = github_utils.check_run_status({ "status": "completed", - "conclusion": "failed", + "conclusion": "success", + "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + "artifacts_url": final_url, }) + self.assertEqual(final_url, result) - with self.assertRaises(github_utils.ActionFailedError): - await github_utils.wait_for_run(client_mock, {"status": "pending", "url": ""}) + def test_pending_run(self): + """Test that a pending run raises the proper exception.""" + with self.assertRaises(github_utils.RunPendingError): + github_utils.check_run_status({ + "status": "pending", + "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + }) - async def test_wait_for_timeout(self, client_mock: mock.Mock): - """Test that the wait_for method quits after a few attempts.""" - client_mock.get.side_effect = responses = [ - httpx.Response(200, json={"status": "pending", "url": ""}) - ] * (github_utils.MAX_POLLS + 5) + def test_timeout_error(self): + """Test that a timeout is declared after a certain duration.""" + # Set the creation time to well before the MAX_RUN_TIME + # to guarantee the right conclusion + created = ( + datetime.datetime.now() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10) + ).strftime(github_utils.ISO_FORMAT_STRING) with self.assertRaises(github_utils.RunTimeoutError): - await github_utils.wait_for_run(client_mock, responses[0].json()) + github_utils.check_run_status({"status": "pending", "created_at": created}) + + def test_failed_run(self): + """Test that a failed run raises the proper exception.""" + with self.assertRaises(github_utils.ActionFailedError): + github_utils.check_run_status({ + "status": "completed", + "conclusion": "failed", + "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + }) -async def get_response_authorize( - _: httpx.AsyncClient, request: httpx.Request, **__ -) -> httpx.Response: +def get_response_authorize(_: httpx.Client, request: httpx.Request, **__) -> httpx.Response: """ Helper method for the authorize tests. @@ -141,76 +126,83 @@ async def get_response_authorize( return httpx.Response(500, request=request) # pragma: no cover -@mock.patch("httpx.AsyncClient.send", new=get_response_authorize) +@mock.patch("httpx.Client.send", new=get_response_authorize) @mock.patch.object(github_utils, "generate_token", new=mock.Mock(return_value="JWT initial token")) -class AuthorizeTests(unittest.IsolatedAsyncioTestCase): +class AuthorizeTests(unittest.TestCase): """Test the authorize utility.""" - async def test_invalid_apps_auth(self): + def test_invalid_apps_auth(self): """Test that an exception is raised if authorization was attempted with an invalid token.""" with mock.patch.object(github_utils, "generate_token", return_value="Invalid token"): with self.assertRaises(httpx.HTTPStatusError) as error: - await github_utils.authorize("VALID_OWNER", "VALID_REPO") + github_utils.authorize("VALID_OWNER", "VALID_REPO") exception: httpx.HTTPStatusError = error.exception self.assertEqual(401, exception.response.status_code) self.assertEqual("auth app/installations", exception.response.json()["error"]) - async def test_missing_repo(self): + def test_missing_repo(self): """Test that an exception is raised when the selected owner or repo are not available.""" with self.assertRaises(github_utils.NotFoundError): - await github_utils.authorize("INVALID_OWNER", "VALID_REPO") + github_utils.authorize("INVALID_OWNER", "VALID_REPO") with self.assertRaises(github_utils.NotFoundError): - await github_utils.authorize("VALID_OWNER", "INVALID_REPO") + github_utils.authorize("VALID_OWNER", "INVALID_REPO") - async def test_valid_authorization(self): + def test_valid_authorization(self): """Test that an accessible repository can be accessed.""" - client = await github_utils.authorize("VALID_OWNER", "VALID_REPO") + client = github_utils.authorize("VALID_OWNER", "VALID_REPO") self.assertEqual("bearer app access token", client.headers.get("Authorization")) -async def get_response_get_artifact(request: httpx.Request, **_) -> httpx.Response: - """ - Helper method for the get_artifact tests. +class ArtifactFetcherTests(unittest.TestCase): + """Test the get_artifact utility.""" - Requests are intercepted before being sent out, and the appropriate responses are returned. - """ - path = request.url.path + @staticmethod + def get_response_get_artifact(request: httpx.Request, **_) -> httpx.Response: + """ + Helper method for the get_artifact tests. - if "force_error" in path: - return httpx.Response(404, request=request) + Requests are intercepted before being sent out, and the appropriate responses are returned. + """ + path = request.url.path - if request.method == "GET": - if path == "/repos/owner/repo/actions/runs": - return httpx.Response(200, request=request, json={"workflow_runs": [{ - "name": "action_name", - "head_sha": "action_sha" - }]}) - elif path == "/artifact_url": - return httpx.Response(200, request=request, json={"artifacts": [{ - "name": "artifact_name", - "archive_download_url": "artifact_download_url" - }]}) - elif path == "/artifact_download_url": - response = httpx.Response(302, request=request) - response.next_request = httpx.Request("GET", httpx.URL("https://final_download.url")) - return response - - # Reaching this point means something has gone wrong - return httpx.Response(500, request=request) # pragma: no cover + if "force_error" in path: + return httpx.Response(404, request=request) + if request.method == "GET": + if path == "/repos/owner/repo/actions/runs": + return httpx.Response( + 200, request=request, json={"workflow_runs": [{ + "name": "action_name", + "head_sha": "action_sha" + }]} + ) + elif path == "/artifact_url": + return httpx.Response( + 200, request=request, json={"artifacts": [{ + "name": "artifact_name", + "archive_download_url": "artifact_download_url" + }]} + ) + elif path == "/artifact_download_url": + response = httpx.Response(302, request=request) + response.next_request = httpx.Request( + "GET", + httpx.URL("https://final_download.url") + ) + return response -class ArtifactFetcherTests(unittest.IsolatedAsyncioTestCase): - """Test the get_artifact utility.""" + # Reaching this point means something has gone wrong + return httpx.Response(500, request=request) # pragma: no cover def setUp(self) -> None: self.call_args = ["owner", "repo", "action_sha", "action_name", "artifact_name"] - self.client = httpx.AsyncClient(base_url="https://example.com") + self.client = httpx.Client(base_url="https://example.com") self.patchers = [ - mock.patch.object(self.client, "send", new=get_response_get_artifact), + mock.patch.object(self.client, "send", new=self.get_response_get_artifact), mock.patch.object(github_utils, "authorize", return_value=self.client), - mock.patch.object(github_utils, "wait_for_run", return_value="artifact_url"), + mock.patch.object(github_utils, "check_run_status", return_value="artifact_url"), ] for patcher in self.patchers: @@ -266,7 +258,7 @@ class GitHubArtifactViewTests(django.test.TestCase): } cls.url = reverse("api:github-artifacts", kwargs=cls.kwargs) - async def test_successful(self, artifact_mock: mock.Mock): + def test_successful(self, artifact_mock: mock.Mock): """Test a proper response is returned with proper input.""" artifact_mock.return_value = "final download url" result = self.client.get(self.url) @@ -274,7 +266,7 @@ class GitHubArtifactViewTests(django.test.TestCase): self.assertIsInstance(result, rest_framework.response.Response) self.assertEqual({"url": artifact_mock.return_value}, result.data) - async def test_failed_fetch(self, artifact_mock: mock.Mock): + def test_failed_fetch(self, artifact_mock: mock.Mock): """Test that a proper error is returned when the request fails.""" artifact_mock.side_effect = github_utils.NotFoundError("Test error message") result = self.client.get(self.url) diff --git a/static-builds/netlify_build.py b/static-builds/netlify_build.py index 13cd0279..a473bd91 100644 --- a/static-builds/netlify_build.py +++ b/static-builds/netlify_build.py @@ -8,6 +8,7 @@ import json import os +import time import zipfile from pathlib import Path from urllib import parse @@ -29,7 +30,7 @@ if __name__ == "__main__": print(f"Fetching download URL from {download_url}") response = httpx.get(download_url, follow_redirects=True) - if response.status_code != 200: + if response.status_code // 100 != 2: try: print(response.json()) except json.JSONDecodeError: @@ -37,6 +38,12 @@ if __name__ == "__main__": response.raise_for_status() + # The workflow is still pending, retry in a bit + while response.status_code == 202: + print(f"{response.json()['error']}. Retrying in 10 seconds.") + time.sleep(10) + response = httpx.get(download_url, follow_redirects=True) + url = response.json()["url"] print(f"Downloading build from {url}") zipped_content = httpx.get(url, follow_redirects=True) -- cgit v1.2.3 From 124c84b9e4f8195485b6ff9b3896cc87e640e02b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Thu, 14 Jul 2022 05:57:05 +0400 Subject: Clean Up Artifact Tests Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/tests/test_github_utils.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py index 78f2979d..a9eab9a5 100644 --- a/pydis_site/apps/api/tests/test_github_utils.py +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -21,7 +21,7 @@ class GeneralUtilityTests(unittest.TestCase): """ Intercept the encode method. - It is performed with an algorithm which does not require a PEM key, as it may + The result is encoded with an algorithm which does not require a PEM key, as it may not be available in testing environments. """ self.assertEqual("RS256", algorithm, "The GitHub App JWT must be signed using RS256.") @@ -39,10 +39,11 @@ class GeneralUtilityTests(unittest.TestCase): self.assertLess(decoded["exp"], (datetime.datetime.now() + delta).timestamp()) -class WaitForTests(unittest.TestCase): +class CheckRunTests(unittest.TestCase): """Tests the check_run_status utility.""" def test_completed_run(self): + """Test that an already completed run returns the correct URL.""" final_url = "some_url_string_1234" result = github_utils.check_run_status({ @@ -245,20 +246,17 @@ class ArtifactFetcherTests(unittest.TestCase): class GitHubArtifactViewTests(django.test.TestCase): """Test the GitHub artifact fetch API view.""" - @classmethod - def setUpClass(cls): - super().setUpClass() - - cls.kwargs = { + def setUp(self): + self.kwargs = { "owner": "test_owner", "repo": "test_repo", "sha": "test_sha", "action_name": "test_action", "artifact_name": "test_artifact", } - cls.url = reverse("api:github-artifacts", kwargs=cls.kwargs) + self.url = reverse("api:github-artifacts", kwargs=self.kwargs) - def test_successful(self, artifact_mock: mock.Mock): + def test_correct_artifact(self, artifact_mock: mock.Mock): """Test a proper response is returned with proper input.""" artifact_mock.return_value = "final download url" result = self.client.get(self.url) -- cgit v1.2.3 From 37001bca59c1d3d5fc8a8dadffda00d55fc9e0b6 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 24 Jul 2022 08:32:43 +0200 Subject: Use Dataclass For Workflow Run Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/github_utils.py | 41 ++++++++++++++++----- pydis_site/apps/api/tests/test_github_utils.py | 50 +++++++++++++++----------- 2 files changed, 62 insertions(+), 29 deletions(-) diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py index 707b36e5..c4ace6b7 100644 --- a/pydis_site/apps/api/github_utils.py +++ b/pydis_site/apps/api/github_utils.py @@ -1,7 +1,8 @@ """Utilities for working with the GitHub API.""" - +import dataclasses import datetime import math +import typing import httpx import jwt @@ -50,6 +51,29 @@ class RunPendingError(ArtifactProcessingError): status = 202 +@dataclasses.dataclass(frozen=True) +class WorkflowRun: + """ + A workflow run from the GitHub API. + + https://docs.github.com/en/rest/actions/workflow-runs#get-a-workflow-run + """ + + name: str + head_sha: str + created_at: str + status: str + conclusion: str + artifacts_url: str + + @classmethod + def from_raw(cls, data: dict[str, typing.Any]): + """Create an instance using the raw data from the API, discarding unused fields.""" + return cls(**{ + key.name: data[key.name] for key in dataclasses.fields(cls) + }) + + def generate_token() -> str: """ Generate a JWT token to access the GitHub API. @@ -121,12 +145,12 @@ def authorize(owner: str, repo: str) -> httpx.Client: raise e -def check_run_status(run: dict) -> str: +def check_run_status(run: WorkflowRun) -> str: """Check if the provided run has been completed, otherwise raise an exception.""" - created_at = datetime.datetime.strptime(run["created_at"], ISO_FORMAT_STRING) + created_at = datetime.datetime.strptime(run.created_at, ISO_FORMAT_STRING) run_time = datetime.datetime.now() - created_at - if run["status"] != "completed": + if run.status != "completed": if run_time <= MAX_RUN_TIME: raise RunPendingError( f"The requested run is still pending. It was created " @@ -135,12 +159,12 @@ def check_run_status(run: dict) -> str: else: raise RunTimeoutError("The requested workflow was not ready in time.") - if run["conclusion"] != "success": + if run.conclusion != "success": # The action failed, or did not run - raise ActionFailedError(f"The requested workflow ended with: {run['conclusion']}") + raise ActionFailedError(f"The requested workflow ended with: {run.conclusion}") # The requested action is ready - return run["artifacts_url"] + return run.artifacts_url def get_artifact(owner: str, repo: str, sha: str, action_name: str, artifact_name: str) -> str: @@ -155,7 +179,8 @@ def get_artifact(owner: str, repo: str, sha: str, action_name: str, artifact_nam # Filter the runs for the one associated with the given SHA for run in runs["workflow_runs"]: - if run["name"] == action_name and sha == run["head_sha"]: + run = WorkflowRun.from_raw(run) + if run.name == action_name and sha == run.head_sha: break else: raise NotFoundError( diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py index a9eab9a5..f5e072a9 100644 --- a/pydis_site/apps/api/tests/test_github_utils.py +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -1,4 +1,6 @@ +import dataclasses import datetime +import typing import unittest from unittest import mock @@ -42,45 +44,46 @@ class GeneralUtilityTests(unittest.TestCase): class CheckRunTests(unittest.TestCase): """Tests the check_run_status utility.""" + run_kwargs: typing.Mapping = { + "name": "run_name", + "head_sha": "sha", + "status": "completed", + "conclusion": "success", + "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + "artifacts_url": "url", + } + def test_completed_run(self): """Test that an already completed run returns the correct URL.""" final_url = "some_url_string_1234" - result = github_utils.check_run_status({ - "status": "completed", - "conclusion": "success", - "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), - "artifacts_url": final_url, - }) + kwargs = dict(self.run_kwargs, artifacts_url=final_url) + result = github_utils.check_run_status(github_utils.WorkflowRun(**kwargs)) self.assertEqual(final_url, result) def test_pending_run(self): """Test that a pending run raises the proper exception.""" + kwargs = dict(self.run_kwargs, status="pending") with self.assertRaises(github_utils.RunPendingError): - github_utils.check_run_status({ - "status": "pending", - "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), - }) + github_utils.check_run_status(github_utils.WorkflowRun(**kwargs)) def test_timeout_error(self): """Test that a timeout is declared after a certain duration.""" + kwargs = dict(self.run_kwargs, status="pending") # Set the creation time to well before the MAX_RUN_TIME # to guarantee the right conclusion - created = ( + kwargs["created_at"] = ( datetime.datetime.now() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10) ).strftime(github_utils.ISO_FORMAT_STRING) with self.assertRaises(github_utils.RunTimeoutError): - github_utils.check_run_status({"status": "pending", "created_at": created}) + github_utils.check_run_status(github_utils.WorkflowRun(**kwargs)) def test_failed_run(self): """Test that a failed run raises the proper exception.""" + kwargs = dict(self.run_kwargs, conclusion="failed") with self.assertRaises(github_utils.ActionFailedError): - github_utils.check_run_status({ - "status": "completed", - "conclusion": "failed", - "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), - }) + github_utils.check_run_status(github_utils.WorkflowRun(**kwargs)) def get_response_authorize(_: httpx.Client, request: httpx.Request, **__) -> httpx.Response: @@ -172,11 +175,16 @@ class ArtifactFetcherTests(unittest.TestCase): if request.method == "GET": if path == "/repos/owner/repo/actions/runs": + run = github_utils.WorkflowRun( + name="action_name", + head_sha="action_sha", + created_at=datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + status="completed", + conclusion="success", + artifacts_url="artifacts_url" + ) return httpx.Response( - 200, request=request, json={"workflow_runs": [{ - "name": "action_name", - "head_sha": "action_sha" - }]} + 200, request=request, json={"workflow_runs": [dataclasses.asdict(run)]} ) elif path == "/artifact_url": return httpx.Response( -- cgit v1.2.3 From f16d3b1b1d14cdf0de1e56ae2bc466152e930f34 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 24 Jul 2022 10:06:47 +0200 Subject: Use UTC Time For GitHub API When reading the created_at time from the GitHub API, it'll be a naive date string with UTC time, so we use that instead of the system's time. Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/github_utils.py | 2 +- pydis_site/apps/api/tests/test_github_utils.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py index c4ace6b7..7d26b147 100644 --- a/pydis_site/apps/api/github_utils.py +++ b/pydis_site/apps/api/github_utils.py @@ -148,7 +148,7 @@ def authorize(owner: str, repo: str) -> httpx.Client: def check_run_status(run: WorkflowRun) -> str: """Check if the provided run has been completed, otherwise raise an exception.""" created_at = datetime.datetime.strptime(run.created_at, ISO_FORMAT_STRING) - run_time = datetime.datetime.now() - created_at + run_time = datetime.datetime.utcnow() - created_at if run.status != "completed": if run_time <= MAX_RUN_TIME: diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py index f5e072a9..f642f689 100644 --- a/pydis_site/apps/api/tests/test_github_utils.py +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -49,7 +49,7 @@ class CheckRunTests(unittest.TestCase): "head_sha": "sha", "status": "completed", "conclusion": "success", - "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + "created_at": datetime.datetime.utcnow().strftime(github_utils.ISO_FORMAT_STRING), "artifacts_url": "url", } @@ -73,7 +73,7 @@ class CheckRunTests(unittest.TestCase): # Set the creation time to well before the MAX_RUN_TIME # to guarantee the right conclusion kwargs["created_at"] = ( - datetime.datetime.now() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10) + datetime.datetime.utcnow() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10) ).strftime(github_utils.ISO_FORMAT_STRING) with self.assertRaises(github_utils.RunTimeoutError): -- cgit v1.2.3 From 460ccffe266373febcd1676d609d65f03de5a967 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Fri, 12 Aug 2022 16:32:59 +0200 Subject: Rename GitHub App Environment Variables Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/github_utils.py | 4 ++-- pydis_site/settings.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py index 7d26b147..ad24165d 100644 --- a/pydis_site/apps/api/github_utils.py +++ b/pydis_site/apps/api/github_utils.py @@ -89,9 +89,9 @@ def generate_token() -> str: { "iat": math.floor((now - datetime.timedelta(seconds=60)).timestamp()), # Issued at "exp": math.floor((now + datetime.timedelta(minutes=9)).timestamp()), # Expires at - "iss": settings.GITHUB_OAUTH_APP_ID, + "iss": settings.GITHUB_APP_ID, }, - settings.GITHUB_OAUTH_KEY, + settings.GITHUB_APP_KEY, algorithm="RS256" ) diff --git a/pydis_site/settings.py b/pydis_site/settings.py index f382b052..bbf1d3aa 100644 --- a/pydis_site/settings.py +++ b/pydis_site/settings.py @@ -29,19 +29,19 @@ env = environ.Env( GIT_SHA=(str, 'development'), TIMEOUT_PERIOD=(int, 5), GITHUB_TOKEN=(str, None), - GITHUB_OAUTH_APP_ID=(str, None), - GITHUB_OAUTH_KEY=(str, None), + GITHUB_APP_ID=(str, None), + GITHUB_APP_KEY=(str, None), ) GIT_SHA = env("GIT_SHA") GITHUB_API = "https://api.github.com" GITHUB_TOKEN = env("GITHUB_TOKEN") -GITHUB_OAUTH_APP_ID = env("GITHUB_OAUTH_APP_ID") -GITHUB_OAUTH_KEY = env("GITHUB_OAUTH_KEY") +GITHUB_APP_ID = env("GITHUB_APP_ID") +GITHUB_APP_KEY = env("GITHUB_APP_KEY") -if GITHUB_OAUTH_KEY and (oauth_file := Path(GITHUB_OAUTH_KEY)).is_file(): +if GITHUB_APP_KEY and (key_file := Path(GITHUB_APP_KEY)).is_file(): # Allow the OAuth key to be loaded from a file - GITHUB_OAUTH_KEY = oauth_file.read_text(encoding="utf-8") + GITHUB_APP_KEY = key_file.read_text(encoding="utf-8") sentry_sdk.init( dsn=env('SITE_DSN'), -- cgit v1.2.3