From 5418a99e88eb0568d6633c7dbca89e12e151a27a Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Thu, 10 Aug 2023 18:54:41 +0000 Subject: [PATCH 01/11] add github dependency --- .devcontainer/devcontainer.json | 3 +- runner_manager/dependencies.py | 17 ++++++++++ runner_manager/models/settings.py | 48 +++++++++++++++++++++++++-- tests/unit/models/test_settings.py | 52 ++++++++++++++++++++++++++++-- 4 files changed, 115 insertions(+), 5 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 36705888..12fa687f 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -34,7 +34,8 @@ "github.vscode-github-actions", "ms-kubernetes-tools.vscode-kubernetes-tools", "ms-python.python", - "ms-python.vscode-pylance" + "ms-python.vscode-pylance", + "GitHub.copilot-chat" ], "settings": { "python": { diff --git a/runner_manager/dependencies.py b/runner_manager/dependencies.py index 73a3fefb..cdf2b0de 100644 --- a/runner_manager/dependencies.py +++ b/runner_manager/dependencies.py @@ -1,9 +1,12 @@ from functools import lru_cache +import httpx +from githubkit.config import Config from redis import Redis from redis_om import get_redis_connection from rq import Queue +from runner_manager.clients.github import GitHub from runner_manager.models.settings import Settings @@ -17,5 +20,19 @@ def get_redis() -> Redis: return get_redis_connection(url=get_settings().redis_om_url, decode_responses=True) +@lru_cache def get_queue() -> Queue: return Queue(connection=get_redis()) + + +@lru_cache() +def get_github() -> GitHub: + settings: Settings = get_settings() + config: Config = Config( + base_url=httpx.URL(str(settings.github_base_url)), + follow_redirects=True, + accept="*/*", + user_agent="runner-manager", + timeout=httpx.Timeout(30.0), + ) + return GitHub(settings.github_auth_strategy(), config=config) diff --git a/runner_manager/models/settings.py b/runner_manager/models/settings.py index 23118add..74ab16d8 100644 --- a/runner_manager/models/settings.py +++ b/runner_manager/models/settings.py @@ -3,7 +3,8 @@ from typing import Any, Dict, Optional import yaml -from pydantic import AnyHttpUrl, BaseSettings, RedisDsn, SecretStr +from githubkit import AppInstallationAuthStrategy, TokenAuthStrategy +from pydantic import AnyHttpUrl, BaseSettings, ConfigError, RedisDsn, SecretStr class ConfigFile(BaseSettings): @@ -32,9 +33,52 @@ class LogLevel(str, Enum): class Settings(BaseSettings): name: Optional[str] = "runner-manager" redis_om_url: Optional[RedisDsn] = None - github_base_url: Optional[AnyHttpUrl] = None + github_base_url: AnyHttpUrl = AnyHttpUrl("api.github.com", scheme="https") github_webhook_secret: Optional[SecretStr] = None log_level: LogLevel = LogLevel.INFO + github_token: Optional[SecretStr] = None + github_app_id: int | str = 0 + github_private_key: SecretStr = SecretStr("") + github_installation_id: int = 0 + github_client_id: Optional[str] = None + github_client_secret: SecretStr = SecretStr("") + + @property + def app_install(self) -> bool: + """ + Returns True if the github auth strategy should be an app install. + + To consider an app install user should define: + + - github_app_id + - github_private_key + - github_installation_id + """ + if ( + self.github_app_id + and self.github_private_key + and self.github_installation_id + ): + return True + return False + + def github_auth_strategy(self) -> AppInstallationAuthStrategy | TokenAuthStrategy: + """ + Returns the appropriate auth strategy for the current configuration. + """ + # prefer AppInstallationAuthStrategy over TokenAuthStrategy + if self.app_install: + return AppInstallationAuthStrategy( + app_id=self.github_app_id, + installation_id=self.github_installation_id, + private_key=self.github_private_key.get_secret_value(), + client_id=self.github_client_id, + client_secret=self.github_client_secret.get_secret_value(), + ) + elif self.github_token: + return TokenAuthStrategy(token=self.github_token.get_secret_value()) + else: + raise ConfigError("No github auth strategy configured") class Config: smart_union = True diff --git a/tests/unit/models/test_settings.py b/tests/unit/models/test_settings.py index 94f49a03..1c7d1a5e 100644 --- a/tests/unit/models/test_settings.py +++ b/tests/unit/models/test_settings.py @@ -3,6 +3,10 @@ import pytest import yaml +from githubkit import AppInstallationAuthStrategy, TokenAuthStrategy +from hypothesis import assume, given +from hypothesis import strategies as st +from pydantic import ConfigError from pytest import fixture from runner_manager.dependencies import get_settings @@ -19,11 +23,11 @@ def yaml_data(): @fixture(scope="function") -def config_file(yaml_data): +def config_file(yaml_data, monkeypatch): # create a yaml file with some data with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: yaml.dump(yaml_data, f) - os.environ["CONFIG_FILE"] = f.name + monkeypatch.setenv("CONFIG_FILE", f.name) config = ConfigFile() return config.config_file @@ -70,3 +74,47 @@ def test_get_settings(config_file): get_settings() with pytest.raises(FileNotFoundError): Settings() + + +@given( + st.builds( + Settings, + github_app_id=st.integers(), + github_installation_id=st.integers(), + github_private_key=st.text(), + github_token=st.text(), + ) +) +def test_app_install(stsettings): + assume(stsettings.github_app_id) + assume(stsettings.github_installation_id) + assume(stsettings.github_private_key) + assert stsettings.app_install is True + assert isinstance(stsettings.github_auth_strategy(), AppInstallationAuthStrategy) + + +@given( + st.builds( + Settings, + github_app_id=st.just(0), + github_installation_id=st.just(0), + github_private_key=st.just(""), + ) +) +def test_token_auth_strategy(stsettings): + assume(stsettings.github_token) + assert isinstance(stsettings.github_auth_strategy(), TokenAuthStrategy) + + +@given( + st.builds( + Settings, + github_token=st.none(), + github_app_id=st.just(0), + github_installation_id=st.just(0), + github_private_key=st.just(""), + ) +) +def test_config_error(stsettings): + with pytest.raises(ConfigError): + stsettings.github_auth_strategy() From bb53962484a73d2959518d5ffbdd9108910d5f1a Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Thu, 10 Aug 2023 20:34:35 +0000 Subject: [PATCH 02/11] add missing file --- tests/api/test_dependency.py | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/api/test_dependency.py diff --git a/tests/api/test_dependency.py b/tests/api/test_dependency.py new file mode 100644 index 00000000..0caaa891 --- /dev/null +++ b/tests/api/test_dependency.py @@ -0,0 +1,42 @@ +from functools import lru_cache +from typing import List + +from fastapi import APIRouter, Depends +from githubkit import Response +from githubkit.rest import OrgsOrgActionsRunnersGetResponse200, Runner +from pytest import fixture + +from runner_manager.clients.github import GitHub +from runner_manager.dependencies import get_github, get_settings +from runner_manager.models.settings import Settings + +router = APIRouter() + + +@router.get("/runners") +def list_runners(github: GitHub = Depends(get_github)) -> List[Runner]: + resp: Response[ + List[OrgsOrgActionsRunnersGetResponse200] + ] = github.rest.actions.list_self_hosted_runners_for_org(org="octo-org") + + return resp.parsed_data.runners + + +@lru_cache() +def settings(): + return Settings( + github_token="test", + github_base_url="http://localhost:4010", + ) + + +@fixture() +def github_app(fastapp): + fastapp.dependency_overrides[get_settings] = settings + return fastapp + + +def test_github_dependency(client, github_app, monkeypatch): + github_app.include_router(router) + runners: List[Runner] = client.get("/runners").json() + assert len(runners) >= 1 From 06bd90317094530102f07021b0ae1d039538042d Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Fri, 11 Aug 2023 22:11:41 +0000 Subject: [PATCH 03/11] update --- tests/unit/models/test_settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/models/test_settings.py b/tests/unit/models/test_settings.py index 38152be8..e4dc714b 100644 --- a/tests/unit/models/test_settings.py +++ b/tests/unit/models/test_settings.py @@ -120,6 +120,7 @@ def test_config_error(stsettings): with pytest.raises(ConfigError): stsettings.github_auth_strategy() + def test_settings_runner_group(runner_group: RunnerGroup): settings = Settings(runner_groups=[runner_group]) assert settings.runner_groups == [runner_group] From a7160081520d54f30244d8a3fd54b98531683ccb Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Fri, 11 Aug 2023 22:13:53 +0000 Subject: [PATCH 04/11] rename file --- tests/api/{test_dependency.py => test_dependencies.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/api/{test_dependency.py => test_dependencies.py} (94%) diff --git a/tests/api/test_dependency.py b/tests/api/test_dependencies.py similarity index 94% rename from tests/api/test_dependency.py rename to tests/api/test_dependencies.py index 0caaa891..36634003 100644 --- a/tests/api/test_dependency.py +++ b/tests/api/test_dependencies.py @@ -36,7 +36,7 @@ def github_app(fastapp): return fastapp -def test_github_dependency(client, github_app, monkeypatch): +def test_github_dependency(client, github_app): github_app.include_router(router) runners: List[Runner] = client.get("/runners").json() assert len(runners) >= 1 From fb82b2251c27028b98e4e69bda38ba366022ee11 Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Fri, 11 Aug 2023 22:51:37 +0000 Subject: [PATCH 05/11] test github fix --- tests/api/conftest.py | 12 ++++++++++++ tests/api/test_dependencies.py | 21 +++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/tests/api/conftest.py b/tests/api/conftest.py index fa9828df..80d8df16 100644 --- a/tests/api/conftest.py +++ b/tests/api/conftest.py @@ -1,9 +1,12 @@ from datetime import timedelta +from functools import lru_cache import pytest from fastapi.testclient import TestClient from hypothesis import HealthCheck, settings +from runner_manager import Settings +from runner_manager.dependencies import get_settings from runner_manager.main import app settings.register_profile( @@ -16,10 +19,19 @@ settings.load_profile("api") +@lru_cache() +def api_settings(): + return Settings( + github_token="test", + github_base_url="http://localhost:4010", + ) + + @pytest.fixture(scope="function") def fastapp(): fastapp = app fastapp.dependency_overrides = {} + fastapp.dependency_overrides[get_settings] = api_settings return fastapp diff --git a/tests/api/test_dependencies.py b/tests/api/test_dependencies.py index 36634003..bb80bbfa 100644 --- a/tests/api/test_dependencies.py +++ b/tests/api/test_dependencies.py @@ -2,12 +2,14 @@ from typing import List from fastapi import APIRouter, Depends +from fastapi.testclient import TestClient from githubkit import Response from githubkit.rest import OrgsOrgActionsRunnersGetResponse200, Runner from pytest import fixture from runner_manager.clients.github import GitHub from runner_manager.dependencies import get_github, get_settings +from runner_manager.main import app from runner_manager.models.settings import Settings router = APIRouter() @@ -22,21 +24,8 @@ def list_runners(github: GitHub = Depends(get_github)) -> List[Runner]: return resp.parsed_data.runners -@lru_cache() -def settings(): - return Settings( - github_token="test", - github_base_url="http://localhost:4010", - ) - - -@fixture() -def github_app(fastapp): - fastapp.dependency_overrides[get_settings] = settings - return fastapp - - -def test_github_dependency(client, github_app): - github_app.include_router(router) +def test_github_dependency(client, fastapp): + fastapp.include_router(router, prefix="/api") + # fastapp.dependency_overrides[get_settings] = settings runners: List[Runner] = client.get("/runners").json() assert len(runners) >= 1 From f6d44071321c8e77a29c8a302c85a67382a1c3ef Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Fri, 11 Aug 2023 22:53:45 +0000 Subject: [PATCH 06/11] use path with no redirect --- manifests/base/runner-manager/main/deployment.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/base/runner-manager/main/deployment.yaml b/manifests/base/runner-manager/main/deployment.yaml index 0eaa2a66..786e5e4d 100644 --- a/manifests/base/runner-manager/main/deployment.yaml +++ b/manifests/base/runner-manager/main/deployment.yaml @@ -29,7 +29,7 @@ spec: livenessProbe: failureThreshold: 3 httpGet: - path: /_health + path: /_health/ port: http initialDelaySeconds: 5 periodSeconds: 10 From 53097d4d069186efa520bf9e5b920f7211c8ddd1 Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Fri, 11 Aug 2023 22:55:32 +0000 Subject: [PATCH 07/11] lint --- tests/api/test_dependencies.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/api/test_dependencies.py b/tests/api/test_dependencies.py index bb80bbfa..03503ae7 100644 --- a/tests/api/test_dependencies.py +++ b/tests/api/test_dependencies.py @@ -1,16 +1,11 @@ -from functools import lru_cache from typing import List from fastapi import APIRouter, Depends -from fastapi.testclient import TestClient from githubkit import Response from githubkit.rest import OrgsOrgActionsRunnersGetResponse200, Runner -from pytest import fixture from runner_manager.clients.github import GitHub -from runner_manager.dependencies import get_github, get_settings -from runner_manager.main import app -from runner_manager.models.settings import Settings +from runner_manager.dependencies import get_github router = APIRouter() From a80dbdded22d92180c78d006986beb4a81325bc5 Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Sat, 12 Aug 2023 02:26:11 +0000 Subject: [PATCH 08/11] stop assuming --- tests/unit/models/test_settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/models/test_settings.py b/tests/unit/models/test_settings.py index e4dc714b..2e6d99c3 100644 --- a/tests/unit/models/test_settings.py +++ b/tests/unit/models/test_settings.py @@ -103,8 +103,8 @@ def test_app_install(stsettings): ) ) def test_token_auth_strategy(stsettings): - assume(stsettings.github_token) - assert isinstance(stsettings.github_auth_strategy(), TokenAuthStrategy) + if stsettings.github_token: + assert isinstance(stsettings.github_auth_strategy(), TokenAuthStrategy) @given( From a4595f839e82a82b98a496669304e3a4dfae6510 Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Sat, 12 Aug 2023 02:29:20 +0000 Subject: [PATCH 09/11] describe resources on failure --- .github/workflows/test-deployment.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test-deployment.yaml b/.github/workflows/test-deployment.yaml index d2f0aca1..58856259 100644 --- a/.github/workflows/test-deployment.yaml +++ b/.github/workflows/test-deployment.yaml @@ -34,6 +34,9 @@ jobs: - name: get all resources if: failure() run: kubectl get all + - name: describe all resources + if: failure() + run: kubectl describe all - name: get logs if: failure() run: | From 1cb99d8ee4ca81da794a73801d19cf905c474d6a Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Sat, 12 Aug 2023 07:48:52 +0000 Subject: [PATCH 10/11] default allowed hosts --- runner_manager/models/settings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/runner_manager/models/settings.py b/runner_manager/models/settings.py index 0bf2594f..1ba45822 100644 --- a/runner_manager/models/settings.py +++ b/runner_manager/models/settings.py @@ -38,7 +38,9 @@ class Settings(BaseSettings): api_key: Optional[SecretStr] = None allowed_hosts: Optional[Sequence[str]] = [ "localhost", + "127.0.0.1", "testserver", + "10.*", ] log_level: LogLevel = LogLevel.INFO runner_groups: List[BaseRunnerGroup] = [] From fa2c9e4837334b5e0972cedd8a02114ea5788d7a Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Mon, 14 Aug 2023 09:16:25 -0700 Subject: [PATCH 11/11] Update runner_manager/models/settings.py --- runner_manager/models/settings.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/runner_manager/models/settings.py b/runner_manager/models/settings.py index 1ba45822..59e4959d 100644 --- a/runner_manager/models/settings.py +++ b/runner_manager/models/settings.py @@ -37,10 +37,7 @@ class Settings(BaseSettings): redis_om_url: Optional[RedisDsn] = None api_key: Optional[SecretStr] = None allowed_hosts: Optional[Sequence[str]] = [ - "localhost", - "127.0.0.1", - "testserver", - "10.*", + "*", ] log_level: LogLevel = LogLevel.INFO runner_groups: List[BaseRunnerGroup] = []