From f79f7bc8fa91427988a35dbeaa009ed9e51fab69 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Tue, 31 Mar 2026 15:29:08 +0200 Subject: [PATCH 01/13] feat(github): add GitHub App authentication to charm layer Add charm config, state handling, secret-changed event, file permissions hardening, docs, and tests for GitHub App auth. --- .../config/vocabularies/local/accept.txt | 1 + charmcraft.yaml | 36 ++++-- docs/changelog.md | 4 + docs/explanation/security.md | 11 +- docs/how-to/change-token.md | 35 +++++- docs/reference/charm-architecture.md | 4 +- docs/reference/token-scopes.md | 70 ++++++++--- src/charm.py | 41 +++++++ src/charm_state.py | 113 ++++++++++++++++-- src/factories.py | 4 +- src/manager_service.py | 6 +- tests/integration/conftest.py | 6 - tests/unit/conftest.py | 3 + tests/unit/factories.py | 6 + tests/unit/test_charm.py | 110 ++++++++++++++--- tests/unit/test_charm_state.py | 111 +++++++++++++++++ tests/unit/test_factories.py | 50 ++++++++ tests/unit/test_manager_service.py | 11 ++ 18 files changed, 552 insertions(+), 70 deletions(-) diff --git a/.vale/styles/config/vocabularies/local/accept.txt b/.vale/styles/config/vocabularies/local/accept.txt index 3ae679c14e..459d236f56 100644 --- a/.vale/styles/config/vocabularies/local/accept.txt +++ b/.vale/styles/config/vocabularies/local/accept.txt @@ -1,5 +1,6 @@ aproxy Aproxy +GitHub App backoff denylist enqueued diff --git a/charmcraft.yaml b/charmcraft.yaml index 5f15bfaebe..952287d9f3 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -10,9 +10,9 @@ description: | A [Juju](https://juju.is/) [charm](https://juju.is/docs/olm/charmed-operators) managing [self-hosted runners for GitHub Actions](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners). Each unit of this charm will start a configurable number of virtual machines to host - self-hosted runners. Each runner performs only one job, after which it unregisters from GitHub to - ensure that each job runs in a clean environment. The charm will periodically check the number of - runners and spawn or destroy them as necessary to maintain the configured number of runners. Both + self-hosted runners. Each runner performs only one job, after which it unregisters from GitHub to + ensure that each job runs in a clean environment. The charm will periodically check the number of + runners and spawn or destroy them as necessary to maintain the configured number of runners. Both the reconciliation interval and the number of runners to maintain are configurable. links: @@ -77,8 +77,8 @@ config: This is useful when the charm is deployed in a network that requires a proxy to access the internet. Note that you should carefully choose values for the 'aproxy-exclude-addresses' and - 'aproxy-redirect-ports' so that the network traffic from the runner to the HTTP proxy is not - captured by aproxy. The simplest way to achieve this is to add the IP address of the HTTP proxy + 'aproxy-redirect-ports' so that the network traffic from the runner to the HTTP proxy is not + captured by aproxy. The simplest way to achieve this is to add the IP address of the HTTP proxy to 'aproxy-exclude-addresses' or exclude the HTTP proxy port from 'aproxy-redirect-ports'. aproxy-exclude-addresses: type: string @@ -102,8 +102,8 @@ config: default: "" description: >- Additional comma separated labels to attach to self-hosted runners. By default, the labels - "self-hosted", architecture (i.e. "x64", "arm64"), os (i.e. "linux"), os-flavor (i.e. - "jammy") are set. Any labels provided via this configuration will be appended to the default + "self-hosted", architecture (i.e. "x64", "arm64"), os (i.e. "linux"), os-flavor (i.e. + "jammy") are set. Any labels provided via this configuration will be appended to the default values. path: type: string @@ -141,6 +141,24 @@ config: 'repo' scope for repository runners and 'repo' + 'admin:org' scope for organization runners. For fine grained token scopes, see https://charmhub.io/github-runner/docs/how-to-change-token. + github-app-client-id: + type: string + default: "" + description: >- + GitHub App Client ID used instead of `token` for GitHub API authentication. + This is the Client ID shown on the GitHub App settings page (e.g. "Iv23liXXXXXX"). + The legacy numeric App ID is also accepted. + github-app-installation-id: + type: int + default: 0 + description: >- + GitHub App installation ID used instead of `token` for GitHub API authentication. + github-app-private-key-secret-id: + type: string + default: "" + description: >- + Juju secret ID containing the PEM-encoded GitHub App private key under the `private-key` + field. Use this together with `github-app-client-id` and `github-app-installation-id`. virtual-machines: type: int default: 0 @@ -179,8 +197,8 @@ config: pre-job-script: type: string description: >- - Optional script (needs shebang) to execute in the pre-job phase of a spawned runner VM. - This can e.g. be useful for specific infrastructure related configurations + Optional script (needs shebang) to execute in the pre-job phase of a spawned runner VM. + This can e.g. be useful for specific infrastructure related configurations (e.g. usage of certain proxies or custom routes). Note that the user executing the script is the ubuntu user (which has sudo rights). Example script: diff --git a/docs/changelog.md b/docs/changelog.md index fb6061fc40..ae1d81afc6 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,10 @@ This changelog documents user-relevant changes to the GitHub runner charm. +## 2026-03-31 + +- Add GitHub App authentication support using Juju secrets for the private key, alongside the existing PAT-based authentication. + ## 2026-03-30 - Fixed stale `systemd` service files left behind when a unit is removed from a co-located machine. The service is now disabled, the service file removed, and the unit data cleaned up on stop. diff --git a/docs/explanation/security.md b/docs/explanation/security.md index 7dd1849beb..9a40e8adb5 100644 --- a/docs/explanation/security.md +++ b/docs/explanation/security.md @@ -45,16 +45,19 @@ When `allow-external-contributor` is set to `false`, external contributors can s This approach ensures that all code from external contributors is reviewed by trusted users before execution on self-hosted runners. -## Permission for GitHub app or personal access token +## Permissions for GitHub App or personal access token -The charm interacts with GitHub via RESTful API. This requires a [personal access token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens). +The charm interacts with GitHub using the RESTful API. This requires either a [GitHub App](https://docs.github.com/en/apps) or a [personal access token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens). -It is generally recommended to grant the minimal permissions necessary for security reasons. Use a [fine-grained token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens#creating-a-fine-grained-personal-access-token) to control the scope of permission. See [token scopes](https://charmhub.io/github-runner/docs/reference-token-scopes) for more information. +GitHub App authentication is preferred as it provides fine-grained permissions and does not tie access to a personal user account. The App's private key is stored securely in a Juju secret. + +When using a personal access token, use a [fine-grained token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens#creating-a-fine-grained-personal-access-token) to control the scope of permission. See [authentication and token scopes](https://charmhub.io/github-runner/docs/reference-token-scopes) for more information. ### Good practices -- Use a fine-grained personal access token. +- Prefer GitHub App authentication over personal access tokens. - Give the minimal permission required. +- When using a personal access token, use a fine-grained token. ## OpenStack project management diff --git a/docs/how-to/change-token.md b/docs/how-to/change-token.md index 08b9e669ef..2497213f15 100644 --- a/docs/how-to/change-token.md +++ b/docs/how-to/change-token.md @@ -1,8 +1,35 @@ -# How to change GitHub personal access token +# How to change GitHub authentication -This charm supports changing the [GitHub personal access token (PAT)](https://github.com/settings/tokens) used. +The charm supports two authentication methods: a GitHub App or a personal access token (PAT). +See [Authentication and token scopes](https://charmhub.io/github-runner/docs/reference-token-scopes) for required permissions. -## Changing the token +## GitHub App authentication + +Create a [GitHub App](https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/registering-a-github-app) with the required permissions and install it on the target organization or repository. + +Store the App's PEM-encoded private key in a Juju secret: + +```shell +juju add-secret github-app-key private-key="$(cat /path/to/private-key.pem)" +``` + +Note the secret ID from the output (e.g. `secret:abc123def`), then grant it to the charm and configure the App credentials: + +```shell +juju grant-secret github-app-key +juju config \ + github-app-client-id= \ + github-app-installation-id= \ + github-app-private-key-secret-id= +``` + +To rotate the private key, update the Juju secret: + +```shell +juju update-secret github-app-key private-key="$(cat /path/to/new-private-key.pem)" +``` + +## Personal access token authentication Create a new [GitHub Personal Access Token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens). @@ -16,4 +43,4 @@ By using [`juju config`](https://juju.is/docs/juju/juju-config) to change the [c ```shell juju config token= -``` \ No newline at end of file +``` diff --git a/docs/reference/charm-architecture.md b/docs/reference/charm-architecture.md index ccf35a48ff..fa4f8a4896 100644 --- a/docs/reference/charm-architecture.md +++ b/docs/reference/charm-architecture.md @@ -90,7 +90,7 @@ would also be directed to the HTTP(s) proxy, unlike when using aproxy. ## GitHub API usage -The charm requires a GitHub personal access token for the [`token` configuration](https://charmhub.io/github-runner/configure#token). This token is used for: +The charm requires GitHub API credentials — either a [GitHub App](https://charmhub.io/github-runner/docs/reference-token-scopes) or a [personal access token](https://charmhub.io/github-runner/configure#token). These credentials are used for: - Requesting self-hosted runner registration tokens - Requesting a list of runner applications @@ -98,7 +98,7 @@ The charm requires a GitHub personal access token for the [`token` configuration - Deleting self-hosted runners Note that the GitHub API uses a [rate-limiting mechanism](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28). When this is reached, the charm may not be able to perform the necessary operations and may go into -BlockedStatus. The charm will automatically recover from this state once the rate limit is reset, but using a different token with a higher rate limit may be a better solution depending on your deployment requirements. +BlockedStatus. The charm will automatically recover from this state once the rate limit is reset. ## External contributor access control diff --git a/docs/reference/token-scopes.md b/docs/reference/token-scopes.md index 3d15f03c1f..90a3361c3e 100644 --- a/docs/reference/token-scopes.md +++ b/docs/reference/token-scopes.md @@ -1,19 +1,55 @@ -# Token scopes +# Authentication and token scopes -In order to use the GitHub runner charm, a personal access token with the necessary permissions -is required. +The GitHub runner charm supports two authentication methods for interacting with the GitHub API: +a [GitHub App](https://docs.github.com/en/apps) or a +[personal access token (PAT)](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens). -## Fine grained access token scopes +## GitHub App authentication + +GitHub App authentication is the recommended approach. It provides fine-grained permissions +and does not tie access to a personal user account. + +To configure the charm with a GitHub App, set the following charm configuration options: + +- `github-app-client-id`: The App's Client ID (shown on the GitHub App settings page). +- `github-app-installation-id`: The installation ID for the target organization or repository. +- `github-app-private-key-secret-id`: A Juju secret ID containing the App's PEM-encoded private + key under the `private-key` field. + +### Required GitHub App permissions + +#### Organizational runners + +Organization permissions: + +- Self-hosted runners: read & write + +Repository permissions: + +- Actions: read (required if COS integration is enabled and private repositories exist) +- Administration: read + +#### Repository runners + +Repository permissions: + +- Actions: read (required if COS integration is enabled and the repository is private) +- Administration: read & write +- Metadata: read + +## Personal access token authentication + +### Fine grained access token scopes **Note**: In addition to having a token with the necessary permissions, the user who owns the -token also must have admin access to the organisation or repository. +token also must have admin access to the organization or repository. -### Organizational runners +#### Organizational runners The following are the permissions scopes required for the GitHub runners when registering as an -organisational runner. +organizational runner. -Organisation: +Organization: - Self-hosted runners: read & write @@ -22,29 +58,29 @@ Repository: - Actions: read (required if COS integration is enabled and private repositories exist) - Administration: read -### Repository runners +#### Repository runners -The following are the permissions scopes required for the GitHub runners when registering as an +The following are the permissions scopes required for the GitHub runners when registering as a repository runner. - Actions: read (required if COS integration is enabled and the repository is private) - Administration: read & write - Metadata: read -## Personal access token scopes +### Classic personal access token scopes -Depending on whether the charm is used for GitHub organisations or repositories, the following scopes -should be selected when creating a personal access token. +Depending on whether the charm is used for GitHub organizations or repositories, the following +scopes should be selected when creating a personal access token. -### Organizational runners +#### Organizational runners -To use this charm for GitHub organisations, the following scopes should be selected: +To use this charm for GitHub organizations, the following scopes should be selected: - `repo` - `admin:org` -### Repository runners +#### Repository runners To use this charm for GitHub repositories, the following scopes should be selected: -- `repo` \ No newline at end of file +- `repo` diff --git a/src/charm.py b/src/charm.py index 8d84eb1165..5fcb95df99 100755 --- a/src/charm.py +++ b/src/charm.py @@ -34,6 +34,7 @@ ConfigChangedEvent, EventBase, InstallEvent, + SecretChangedEvent, StartEvent, StopEvent, UpdateStatusEvent, @@ -47,6 +48,9 @@ from charm_state import ( ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, DEBUG_SSH_INTEGRATION_NAME, + GITHUB_APP_CLIENT_ID_CONFIG_NAME, + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, PATH_CONFIG_NAME, @@ -201,6 +205,15 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self._stored.set_default( path=self.config[PATH_CONFIG_NAME], # for detecting changes token=self.config[TOKEN_CONFIG_NAME], # for detecting changes + github_app_client_id=self.config[ + GITHUB_APP_CLIENT_ID_CONFIG_NAME + ], # for detecting changes + github_app_installation_id=self.config[ + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME + ], # for detecting changes + github_app_private_key_secret_id=self.config[ + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME + ], # for detecting changes labels=self.config[LABELS_CONFIG_NAME], # for detecting changes allow_external_contributor=self.config[ ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME @@ -233,6 +246,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self.framework.observe( self.on[PLANNER_INTEGRATION_NAME].relation_broken, self._on_planner_relation_broken ) + self.framework.observe(self.on.secret_changed, self._on_secret_changed) self.framework.observe(self.on.check_runners_action, self._on_check_runners_action) self.framework.observe(self.on.flush_runners_action, self._on_flush_runners_action) self.framework.observe(self.on.update_status, self._on_update_status) @@ -339,6 +353,25 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: if self.config[TOKEN_CONFIG_NAME] != self._stored.token: self._stored.token = self.config[TOKEN_CONFIG_NAME] flush_runners = True + if self.config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] != self._stored.github_app_client_id: + self._stored.github_app_client_id = self.config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] + flush_runners = True + if ( + self.config[GITHUB_APP_INSTALLATION_ID_CONFIG_NAME] + != self._stored.github_app_installation_id + ): + self._stored.github_app_installation_id = self.config[ + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME + ] + flush_runners = True + if ( + self.config[GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME] + != self._stored.github_app_private_key_secret_id + ): + self._stored.github_app_private_key_secret_id = self.config[ + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME + ] + flush_runners = True if self.config[PATH_CONFIG_NAME] != self._stored.path: self._stored.path = self.config[PATH_CONFIG_NAME] flush_runners = True @@ -363,6 +396,14 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: self._manager_client.flush_runner() self.unit.status = ActiveStatus() + @catch_charm_errors + def _on_secret_changed(self, _: SecretChangedEvent) -> None: + """Handle secret content rotation (e.g. GitHub App private key).""" + logger.info("Secret changed, reconciling") + state = self._setup_state() + self._reconcile(state) + self._manager_client.flush_runner() + @catch_action_errors def _on_check_runners_action(self, event: ActionEvent) -> None: """Handle the action of checking of runner state. diff --git a/src/charm_state.py b/src/charm_state.py index f0e55faed4..9e1c1faebb 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -3,6 +3,9 @@ """State of the Charm.""" +# The charm state module intentionally centralizes config/state translation logic. +# pylint: disable=too-many-lines + import dataclasses import ipaddress import json @@ -14,7 +17,13 @@ import yaml from github_runner_manager.configuration import ProxyConfig, SSHDebugConnection -from github_runner_manager.configuration.github import GitHubPath, parse_github_path +from github_runner_manager.configuration.github import ( + GitHubAppAuth, + GitHubAuth, + GitHubPath, + GitHubTokenAuth, + parse_github_path, +) from ops import CharmBase from ops.model import SecretNotFoundError from pydantic import BaseModel, ValidationError, create_model_from_typeddict, validator @@ -34,6 +43,11 @@ DOCKERHUB_MIRROR_CONFIG_NAME = "dockerhub-mirror" FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME = "flavor-label-combinations" GROUP_CONFIG_NAME = "group" +GITHUB_APP_CLIENT_ID_CONFIG_NAME = "github-app-client-id" +GITHUB_APP_INSTALLATION_ID_CONFIG_NAME = "github-app-installation-id" +GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME = ( # nosec: not a password + "github-app-private-key-secret-id" +) LABELS_CONFIG_NAME = "labels" MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME = "max-total-virtual-machines" MANAGER_SSH_PROXY_COMMAND_CONFIG_NAME = "manager-ssh-proxy-command" @@ -124,14 +138,21 @@ class GithubConfig: Attributes: token: The Github API access token (PAT). + app_client_id: The GitHub App Client ID (or legacy numeric App ID). + installation_id: The GitHub App installation ID. + private_key: The GitHub App private key PEM. + auth: The github-runner-manager authentication model derived from these fields. path: The Github org/repo path. """ - token: str + token: str | None + app_client_id: str | None + installation_id: int | None + private_key: str | None path: GitHubPath @classmethod - def from_charm(cls, charm: CharmBase) -> "GithubConfig | None": + def from_charm(cls, charm: CharmBase) -> "GithubConfig": # noqa: C901 """Get github related charm configuration values from charm. Args: @@ -146,7 +167,14 @@ def from_charm(cls, charm: CharmBase) -> "GithubConfig | None": runner_group = cast(str, charm.config.get(GROUP_CONFIG_NAME, "default")) path_str = cast(str, charm.config.get(PATH_CONFIG_NAME, "")) - token = cast(str, charm.config.get(TOKEN_CONFIG_NAME)) + token = cast(str, charm.config.get(TOKEN_CONFIG_NAME)) or None + app_client_id = cast(str, charm.config.get(GITHUB_APP_CLIENT_ID_CONFIG_NAME, "")) or None + installation_id = ( + cast(int, charm.config.get(GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, 0)) or None + ) + private_key_secret_id = ( + cast(str, charm.config.get(GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, "")) or None + ) if not path_str: raise CharmConfigInvalidError(f"Missing {PATH_CONFIG_NAME} configuration") @@ -156,10 +184,55 @@ def from_charm(cls, charm: CharmBase) -> "GithubConfig | None": except ValueError as e: raise CharmConfigInvalidError(str(e)) from e - if not token: - raise CharmConfigInvalidError(f"Missing {TOKEN_CONFIG_NAME} configuration") + app_fields = (app_client_id, installation_id, private_key_secret_id) + app_fields_set = sum(field is not None for field in app_fields) + + if token and app_fields_set: + raise CharmConfigInvalidError( + "Configure either token or GitHub App authentication, not both" + ) + if not token and not app_fields_set: + raise CharmConfigInvalidError( + f"Missing {TOKEN_CONFIG_NAME} or GitHub App authentication configuration" + ) + if not token and app_fields_set != 3: + raise CharmConfigInvalidError( + "GitHub App authentication requires github-app-client-id, " + "github-app-installation-id and github-app-private-key-secret-id" + ) - return cls(token=cast(str, token), path=path) + private_key = None + if private_key_secret_id: + try: + private_key_secret = charm.model.get_secret(id=private_key_secret_id) + private_key = private_key_secret.get_content().get("private-key") + except SecretNotFoundError as exc: + raise CharmConfigInvalidError( + f"GitHub App private key secret {private_key_secret_id} not found" + ) from exc + if not private_key: + raise CharmConfigInvalidError( + f"GitHub App private key secret {private_key_secret_id} is missing private-key" + ) + + return cls( + token=token, + app_client_id=app_client_id, + installation_id=installation_id, + private_key=private_key, + path=path, + ) + + @property + def auth(self) -> GitHubAuth: + """Build the application GitHub auth configuration.""" + if self.token is not None: + return GitHubTokenAuth(token=self.token) + return GitHubAppAuth( + app_client_id=cast(str, self.app_client_id), + installation_id=cast(int, self.installation_id), + private_key=cast(str, self.private_key), + ) class CharmConfigInvalidError(Exception): @@ -175,6 +248,7 @@ def __init__(self, msg: str): Args: msg: Explanation of the error. """ + super().__init__(msg) self.msg = msg @@ -226,6 +300,10 @@ class CharmConfig(BaseModel): name. reconcile_interval: Time between each reconciliation of runners in minutes. token: GitHub personal access token for GitHub API. + app_client_id: GitHub App Client ID for GitHub API. + installation_id: GitHub App installation ID for GitHub API. + private_key: GitHub App private key PEM for GitHub API. + auth: The github-runner-manager authentication model derived from the credential fields. manager_proxy_command: ProxyCommand for the SSH connection from the manager to the runner. use_aproxy: Whether to use aproxy in the runner. aproxy_exclude_addresses: a list of addresses to exclude from the aproxy proxy. @@ -241,6 +319,9 @@ class CharmConfig(BaseModel): path: GitHubPath | None reconcile_interval: int token: str | None + app_client_id: str | None + installation_id: int | None + private_key: str | None manager_proxy_command: str | None use_aproxy: bool aproxy_exclude_addresses: list[str] = [] @@ -248,6 +329,17 @@ class CharmConfig(BaseModel): custom_pre_job_script: str | None runner_manager_log_level: LogLevel + @property + def auth(self) -> GitHubAuth: + """Build the GitHub auth configuration from the stored credentials.""" + return GithubConfig( + token=self.token, + app_client_id=self.app_client_id, + installation_id=self.installation_id, + private_key=self.private_key, + path=self.path, + ).auth + @classmethod def _parse_dockerhub_mirror(cls, charm: CharmBase) -> str | None: """Parse and validate dockerhub mirror URL. @@ -490,9 +582,12 @@ def from_charm(cls, charm: CharmBase) -> "CharmConfig": dockerhub_mirror=dockerhub_mirror, # type: ignore labels=labels, openstack_clouds_yaml=openstack_clouds_yaml, - path=github_config.path if github_config else None, + path=github_config.path, reconcile_interval=reconcile_interval, - token=github_config.token if github_config else None, + token=github_config.token, + app_client_id=github_config.app_client_id, + installation_id=github_config.installation_id, + private_key=github_config.private_key, manager_proxy_command=manager_proxy_command, use_aproxy=use_aproxy, # mypy doesn't know about the validator diff --git a/src/factories.py b/src/factories.py index ae04d3b567..ae34da9686 100644 --- a/src/factories.py +++ b/src/factories.py @@ -13,7 +13,7 @@ RunnerConfiguration, SupportServiceConfig, ) -from github_runner_manager.configuration.github import GitHubConfiguration, GitHubTokenAuth +from github_runner_manager.configuration.github import GitHubConfiguration from github_runner_manager.openstack_cloud.configuration import ( OpenStackConfiguration, OpenStackCredentials, @@ -40,7 +40,7 @@ def create_application_configuration( extra_labels = list(state.charm_config.labels) github_configuration = ( GitHubConfiguration( - auth=GitHubTokenAuth(token=state.charm_config.token), + auth=state.charm_config.auth, path=state.charm_config.path, ) if state.charm_config.path diff --git a/src/manager_service.py b/src/manager_service.py index a5e2ddbd42..d8c8160d72 100644 --- a/src/manager_service.py +++ b/src/manager_service.py @@ -6,6 +6,7 @@ import fcntl import json import logging +import os import shutil import socket import textwrap @@ -359,8 +360,11 @@ def _setup_config_file(config: ApplicationConfiguration, unit_name: str) -> Path unit_dir = GITHUB_RUNNER_MANAGER_SERVICE_DIR / _normalized_unit(unit_name) unit_dir.mkdir(parents=True, exist_ok=True) path = unit_dir / "config.yaml" - with open(path, "w+", encoding="utf-8") as file: + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + os.fchmod(fd, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as file: yaml_safe_dump(config_dict, file) + shutil.chown(path, user=constants.RUNNER_MANAGER_USER, group=constants.RUNNER_MANAGER_GROUP) return path diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 134b389ed0..c2c0e76fbe 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -26,7 +26,6 @@ from github.Auth import Token from github.Branch import Branch from github.Repository import Repository -from github_runner_manager.github_client import GithubClient from juju.application import Application from juju.client._definitions import FullStatus, UnitStatus from juju.model import Model @@ -466,11 +465,6 @@ async def model(ops_test: OpsTest, proxy_config: ProxyConfig) -> Model: return ops_test.model -@pytest.fixture(scope="module") -def runner_manager_github_client(github_config: GitHubConfig) -> GithubClient: - return GithubClient(token=github_config.token) - - @pytest_asyncio.fixture(scope="module") async def app_no_runner( model: Model, diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 3dca261d80..3aaf3f1c8b 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -157,6 +157,9 @@ def complete_charm_state_fixture(): path=GitHubOrg(org="canonical", group="group"), reconcile_interval=5, token="githubtoken", + app_client_id=None, + installation_id=None, + private_key=None, manager_proxy_command="ssh -W %h:%p example.com", use_aproxy=True, runner_manager_log_level="INFO", diff --git a/tests/unit/factories.py b/tests/unit/factories.py index c5b965aeb3..e58c11cefb 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -24,6 +24,9 @@ DEBUG_SSH_INTEGRATION_NAME, DOCKERHUB_MIRROR_CONFIG_NAME, FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME, + GITHUB_APP_CLIENT_ID_CONFIG_NAME, + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, GROUP_CONFIG_NAME, LABELS_CONFIG_NAME, MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME, @@ -132,6 +135,9 @@ class Meta: DOCKERHUB_MIRROR_CONFIG_NAME: "", FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME: "", GROUP_CONFIG_NAME: "default", + GITHUB_APP_CLIENT_ID_CONFIG_NAME: "", + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME: 0, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME: "", LABELS_CONFIG_NAME: "", MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME: 0, OPENSTACK_CLOUDS_YAML_CONFIG_NAME: yaml.safe_dump( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index b50f9d5636..2d77a4f199 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -28,6 +28,9 @@ ) from charm_state import ( FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME, + GITHUB_APP_CLIENT_ID_CONFIG_NAME, + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, OPENSTACK_CLOUDS_YAML_CONFIG_NAME, @@ -251,21 +254,41 @@ def test_on_config_changed_failure(harness: Harness): @pytest.mark.parametrize( - "config_option", + "config_option, initial_value, updated_value", [ - pytest.param(PATH_CONFIG_NAME, id="Path"), - pytest.param(TOKEN_CONFIG_NAME, id="Token"), - pytest.param(LABELS_CONFIG_NAME, id="Labels"), + pytest.param(PATH_CONFIG_NAME, "initial-path", "updated-path", id="Path"), + pytest.param(TOKEN_CONFIG_NAME, "initial-token", "updated-token", id="Token"), + pytest.param( + GITHUB_APP_CLIENT_ID_CONFIG_NAME, "Iv23liOld", "Iv23liNew", id="GitHub App Client ID" + ), + pytest.param( + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + 67890, + 67891, + id="GitHub App Installation ID", + ), + pytest.param( + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, + "secret:abc123", + "secret:def456", + id="GitHub App Private Key Secret ID", + ), + pytest.param(LABELS_CONFIG_NAME, "label-a", "label-b", id="Labels"), ], ) -def test__on_config_changed_flush(monkeypatch: pytest.MonkeyPatch, config_option: str): +def test__on_config_changed_flush( + monkeypatch: pytest.MonkeyPatch, + config_option: str, + initial_value, + updated_value, +): """ arrange: given a charm with OpenStack instance type and a certain config option value. act: update the config option. assert: runner flush is called. """ harness = Harness(GithubRunnerCharm) - harness.update_config({config_option: secrets.token_hex(16)}) + harness.update_config({config_option: initial_value}) harness.begin() state_mock = MagicMock() monkeypatch.setattr("charm.manager_service", MagicMock()) @@ -273,28 +296,40 @@ def test__on_config_changed_flush(monkeypatch: pytest.MonkeyPatch, config_option harness.charm._setup_state = MagicMock(return_value=state_mock) harness.charm._check_image_ready = MagicMock(return_value=True) - harness.update_config({config_option: secrets.token_hex(16)}) + harness.update_config({config_option: updated_value}) harness.charm._manager_client.flush_runner.assert_called_once() @pytest.mark.parametrize( - "config_option", + "config_option, config_value", [ - pytest.param(PATH_CONFIG_NAME, id="Path"), - pytest.param(TOKEN_CONFIG_NAME, id="Token"), - pytest.param(LABELS_CONFIG_NAME, id="Labels"), + pytest.param(PATH_CONFIG_NAME, secrets.token_hex(16), id="Path"), + pytest.param(TOKEN_CONFIG_NAME, secrets.token_hex(16), id="Token"), + pytest.param(GITHUB_APP_CLIENT_ID_CONFIG_NAME, "Iv23liExample", id="GitHub App Client ID"), + pytest.param( + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + 67890, + id="GitHub App Installation ID", + ), + pytest.param( + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, + f"secret:{secrets.token_hex(8)}", + id="GitHub App Private Key Secret ID", + ), + pytest.param(LABELS_CONFIG_NAME, secrets.token_hex(16), id="Labels"), ], ) -def test__on_config_changed_no_flush(monkeypatch: pytest.MonkeyPatch, config_option: str): +def test__on_config_changed_no_flush( + monkeypatch: pytest.MonkeyPatch, config_option: str, config_value +): """ arrange: given a charm with OpenStack instance type and a certain config option value. act: update the config option to be the same as before. - assert: runner flush is called. + assert: runner flush is not called. """ - config_option_val = secrets.token_hex(16) harness = Harness(GithubRunnerCharm) - harness.update_config({config_option: config_option_val}) + harness.update_config({config_option: config_value}) harness.begin() state_mock = MagicMock() monkeypatch.setattr("charm.manager_service", MagicMock()) @@ -302,11 +337,54 @@ def test__on_config_changed_no_flush(monkeypatch: pytest.MonkeyPatch, config_opt harness.charm._setup_state = MagicMock(return_value=state_mock) harness.charm._check_image_ready = MagicMock(return_value=True) - harness.update_config({config_option: config_option_val}) + harness.update_config({config_option: config_value}) harness.charm._manager_client.flush_runner.assert_not_called() +def test_on_secret_changed_flushes_runners(monkeypatch: pytest.MonkeyPatch): + """ + arrange: Charm configured with a GitHub App private key secret. + act: Update the secret content (triggers secret-changed event). + assert: Runners are flushed and service is reconciled. + """ + harness = Harness(GithubRunnerCharm) + secret_id = harness.add_user_secret(content={"private-key": "old-pem"}) + harness.update_config({GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME: secret_id}) + harness.begin() + state_mock = MagicMock() + monkeypatch.setattr("charm.manager_service", MagicMock()) + harness.charm._manager_client = MagicMock(spec=GitHubRunnerManagerClient) + harness.charm._setup_state = MagicMock(return_value=state_mock) + harness.charm._check_image_ready = MagicMock(return_value=True) + + harness.set_secret_content(secret_id, {"private-key": "new-pem"}) + + harness.charm._manager_client.flush_runner.assert_called_once() + + +def test_on_secret_changed_reconciles_on_any_secret(monkeypatch: pytest.MonkeyPatch): + """ + arrange: Charm configured with a GitHub App private key secret. + act: Update an unrelated secret's content. + assert: Runners are still flushed (holistic reconcile on any secret change). + """ + harness = Harness(GithubRunnerCharm) + secret_id = harness.add_user_secret(content={"private-key": "pem-data"}) + unrelated_secret_id = harness.add_user_secret(content={"some-key": "some-value"}) + harness.update_config({GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME: secret_id}) + harness.begin() + state_mock = MagicMock() + monkeypatch.setattr("charm.manager_service", MagicMock()) + harness.charm._manager_client = MagicMock(spec=GitHubRunnerManagerClient) + harness.charm._setup_state = MagicMock(return_value=state_mock) + harness.charm._check_image_ready = MagicMock(return_value=True) + + harness.set_secret_content(unrelated_secret_id, {"some-key": "new-value"}) + + harness.charm._manager_client.flush_runner.assert_called_once() + + def test_on_stop_busy_flush_and_cleanup_service(harness: Harness, monkeypatch: pytest.MonkeyPatch): """ arrange: Set up charm with Openstack mode and runner scaler mock. diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index 5beee9a0a6..6e17080f35 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -21,6 +21,9 @@ DEBUG_SSH_INTEGRATION_NAME, DOCKERHUB_MIRROR_CONFIG_NAME, FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME, + GITHUB_APP_CLIENT_ID_CONFIG_NAME, + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, GROUP_CONFIG_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, @@ -92,6 +95,114 @@ def test_github_config_from_charm_invalid_token(): GithubConfig.from_charm(mock_charm) +def test_github_config_from_charm_token_only(): + """ + arrange: Charm configured with PAT only. + act: Call from_charm. + assert: GithubConfig contains PAT fields only. + """ + mock_charm = MockGithubRunnerCharmFactory() + + result = GithubConfig.from_charm(mock_charm) + + assert result.token == mock_charm.config[TOKEN_CONFIG_NAME] + assert result.app_client_id is None + assert result.installation_id is None + assert result.private_key is None + + +def test_github_config_from_charm_app_only(): + """ + arrange: Charm configured with GitHub App auth only. + act: Call from_charm. + assert: GithubConfig resolves the private key secret content. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "" + mock_charm.config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] = "Iv23liExample" + mock_charm.config[GITHUB_APP_INSTALLATION_ID_CONFIG_NAME] = 456 + mock_charm.config[GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME] = "secret:abc123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {"private-key": "private-key-pem"} + mock_charm.model.get_secret.return_value = secret_mock + + result = GithubConfig.from_charm(mock_charm) + + assert result.token is None + assert result.app_client_id == "Iv23liExample" + assert result.installation_id == 456 + assert result.private_key == "private-key-pem" + mock_charm.model.get_secret.assert_called_once_with(id="secret:abc123") + + +@pytest.mark.parametrize( + "config_updates", + [ + pytest.param( + { + GITHUB_APP_CLIENT_ID_CONFIG_NAME: "Iv23liExample", + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME: 456, + }, + id="missing-private-key-secret-id", + ), + pytest.param( + { + GITHUB_APP_CLIENT_ID_CONFIG_NAME: "Iv23liExample", + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME: "secret:abc123", + }, + id="missing-installation-id", + ), + pytest.param( + { + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME: 456, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME: "secret:abc123", + }, + id="missing-app-id", + ), + ], +) +def test_github_config_from_charm_partial_app_fields(config_updates: dict[str, object]): + """ + arrange: Charm configured with only part of the GitHub App fields. + act: Call from_charm. + assert: CharmConfigInvalidError is raised. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "" + mock_charm.config.update(config_updates) + + with pytest.raises(CharmConfigInvalidError): + GithubConfig.from_charm(mock_charm) + + +def test_github_config_from_charm_rejects_both_pat_and_app_auth(): + """ + arrange: Charm configured with both PAT and GitHub App auth. + act: Call from_charm. + assert: CharmConfigInvalidError is raised. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] = "Iv23liExample" + mock_charm.config[GITHUB_APP_INSTALLATION_ID_CONFIG_NAME] = 456 + mock_charm.config[GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME] = "secret:abc123" + + with pytest.raises(CharmConfigInvalidError): + GithubConfig.from_charm(mock_charm) + + +def test_github_config_from_charm_rejects_missing_auth(): + """ + arrange: Charm configured with neither PAT nor GitHub App auth. + act: Call from_charm. + assert: CharmConfigInvalidError is raised. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "" + + with pytest.raises(CharmConfigInvalidError): + GithubConfig.from_charm(mock_charm) + + @pytest.mark.parametrize( "path_str, runner_group, expected_type, expected_attrs", [ diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index 0298e6ede9..3b3cbc1548 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import dataclasses +import pytest from github_runner_manager.configuration.base import ( ApplicationConfiguration, Flavor, @@ -13,6 +14,7 @@ SupportServiceConfig, ) from github_runner_manager.configuration.github import ( + GitHubAppAuth, GitHubConfiguration, GitHubOrg, GitHubTokenAuth, @@ -145,3 +147,51 @@ def test_create_application_configuration_with_planner( assert str(app_configuration.planner_url) == "http://planner.example.com" assert app_configuration.planner_token == "planner-token-value" + + +@pytest.mark.parametrize( + "charm_config_updates, expected_auth", + [ + pytest.param( + { + "token": "githubtoken", + "app_client_id": None, + "installation_id": None, + "private_key": None, + }, + GitHubTokenAuth(token="githubtoken"), + id="token-auth", + ), + pytest.param( + { + "token": None, + "app_client_id": "Iv23liExample", + "installation_id": 456, + "private_key": "private-key", + }, + GitHubAppAuth( + app_client_id="Iv23liExample", installation_id=456, private_key="private-key" + ), + id="app-auth", + ), + ], +) +def test_create_application_configuration_with_github_auth( + complete_charm_state: charm_state.CharmState, + charm_config_updates: dict, + expected_auth, +): + """ + arrange: Prepare CharmState with either PAT or GitHub App auth. + act: Call create_application_configuration. + assert: The resulting GitHubConfiguration contains the right auth model. + """ + state = dataclasses.replace( + complete_charm_state, + charm_config=complete_charm_state.charm_config.copy(update=charm_config_updates), + ) + + app_configuration = factories.create_application_configuration(state, "app_name", "unit_name") + + assert app_configuration.github_config + assert app_configuration.github_config.auth == expected_auth diff --git a/tests/unit/test_manager_service.py b/tests/unit/test_manager_service.py index dacfc23c61..a18ee54409 100644 --- a/tests/unit/test_manager_service.py +++ b/tests/unit/test_manager_service.py @@ -72,11 +72,20 @@ def mock_execute_command_fixture(monkeypatch): return mock_execute_command +@pytest.fixture(name="mock_chown") +def mock_chown_fixture(monkeypatch): + """Mock shutil.chown so tests pass without the runner-manager OS user.""" + mock_chown = MagicMock() + monkeypatch.setattr("manager_service.shutil.chown", mock_chown) + return mock_chown + + def test_setup_started( complete_charm_state: CharmState, patched_paths: PatchedPaths, mock_systemd: MagicMock, mock_execute_command: MagicMock, + mock_chown: MagicMock, monkeypatch, ): """ @@ -119,6 +128,7 @@ def test_setup_no_started( complete_charm_state: CharmState, mock_systemd: MagicMock, mock_execute_command: MagicMock, + mock_chown: MagicMock, ): """ arrange: Mock the service to be not running. @@ -136,6 +146,7 @@ def test_setup_systemd_error( complete_charm_state: CharmState, mock_systemd: MagicMock, mock_execute_command: MagicMock, + mock_chown: MagicMock, ): """ arrange: Mock the systemd to raise error. From dea2ff6e2ffc831e0f2a3011e307de2dbb9af029 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 2 Apr 2026 13:57:50 +0200 Subject: [PATCH 02/13] feat(tests): use GitHub App auth for charm e2e integration test The e2e test now uses GitHub App authentication when credentials are provided, falling back to PAT. This exercises the full charm wiring (Juju secret resolution, config propagation, runner registration) with App auth end-to-end. The app-level GitHub App auth integration test is removed as it is subsumed by the charm-level test. --- .github/workflows/e2e_test.yaml | 8 +- .../workflows/test_github_runner_manager.yaml | 5 -- CONTRIBUTING.md | 9 ++- github-runner-manager/tests/conftest.py | 18 ----- .../tests/integration/test_github_app_auth.py | 79 ------------------- tests/conftest.py | 18 +++++ tests/integration/conftest.py | 14 +++- tests/integration/helpers/common.py | 15 +++- tests/integration/test_e2e.py | 29 +++++++ 9 files changed, 84 insertions(+), 111 deletions(-) delete mode 100644 github-runner-manager/tests/integration/test_github_app_auth.py diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml index 2a0ac5a2ad..d1b49dad12 100644 --- a/.github/workflows/e2e_test.yaml +++ b/.github/workflows/e2e_test.yaml @@ -19,8 +19,9 @@ jobs: provider: lxd test-tox-env: integration-juju3.6 modules: '["test_e2e"]' - # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ - # mapping. See CONTRIBUTING.md for more details. + # INTEGRATION_TOKEN, OS_PASSWORD, GITHUB_APP_INSTALLATION_ID (base slot), + # GITHUB_APP_PRIVATE_KEY (slot 11) are passed through + # INTEGRATION_TEST_SECRET_ENV_VALUE_ mapping. See CONTRIBUTING.md for more details. extra-arguments: | -m=openstack \ --log-format="%(asctime)s %(levelname)s %(message)s" \ @@ -32,7 +33,8 @@ jobs: --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ - --dockerhub-mirror="${{ vars.INTEGRATION_TEST_DOCKERHUB_MIRROR }}" + --dockerhub-mirror="${{ vars.INTEGRATION_TEST_DOCKERHUB_MIRROR }}" \ + --github-app-client-id="${{ vars.INTEGRATION_TEST_GITHUB_APP_CLIENT_ID }}" self-hosted-runner: true self-hosted-runner-label: pfe-ci diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 2a6f1239d0..cdbecc563f 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -22,7 +22,6 @@ jobs: - test_debug_ssh - test_metrics - test_planner_runner - - test_github_app_auth steps: - name: Checkout code uses: actions/checkout@v6.0.2 @@ -51,10 +50,6 @@ jobs: ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_8 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_8 }} ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_9 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_9 }} ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_10 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_10 }} - # GITHUB_APP_CLIENT_ID, GITHUB_APP_INSTALLATION_ID, GITHUB_APP_PRIVATE_KEY - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_11 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_11 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_12 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_12 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_13 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_13 }} run: | tox -e integration -- -v --tb=native -s \ tests/integration/${{ matrix.test-module }}.py \ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff172dd856..195645d0c0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,11 +49,12 @@ that can be used for linting and formatting code when you're preparing contribut To prefer explicit setting arguments passing, use `extra-arguments` of the [following reusable workflow](https://github.com/canonical/operator-workflows/blob/main/.github/workflows/integration_test_run.yaml) to pass in non-sensitive values. -For sensitive values (`INTEGRATION_TOKEN`: `--token`, `OS_PASSWORD`: `--openstack-password`), map -them through `INTEGRATION_TEST_SECRET_ENV_NAME_` -environment variable settings under [repository](https://github.com/canonical/github-runner-operator) -> settings > variables > actions. +For sensitive values (`INTEGRATION_TOKEN`, `OS_PASSWORD`, `GITHUB_APP_INSTALLATION_ID`, +`GITHUB_APP_PRIVATE_KEY`, etc.), map them through `INTEGRATION_TEST_SECRET_ENV_NAME_` / +`INTEGRATION_TEST_SECRET_ENV_VALUE_` environment variable settings under +[repository](https://github.com/canonical/github-runner-operator) > settings > variables > actions. This is to prevent GitHub from leaking secrets when passing them over the CLI calls (unresolved). +See the workflow files for the current slot assignments. ### Building the charm diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index 483618229f..d2d6095acf 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -137,21 +137,3 @@ def pytest_addoption(parser): help="Directory to store debug logs.", default=os.getenv("DEBUG_LOG_DIR", "/tmp/github-runner-manager-test-logs"), ) - parser.addoption( - "--github-app-client-id", - action="store", - help="GitHub App Client ID for App authentication integration tests.", - default=os.getenv("GITHUB_APP_CLIENT_ID"), - ) - parser.addoption( - "--github-app-installation-id", - action="store", - help="GitHub App installation ID for App authentication integration tests.", - default=os.getenv("GITHUB_APP_INSTALLATION_ID"), - ) - parser.addoption( - "--github-app-private-key", - action="store", - help="GitHub App PEM-encoded private key for App authentication integration tests.", - default=os.getenv("GITHUB_APP_PRIVATE_KEY"), - ) diff --git a/github-runner-manager/tests/integration/test_github_app_auth.py b/github-runner-manager/tests/integration/test_github_app_auth.py deleted file mode 100644 index 2877f8dab1..0000000000 --- a/github-runner-manager/tests/integration/test_github_app_auth.py +++ /dev/null @@ -1,79 +0,0 @@ -# Copyright 2026 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Integration tests for GitHub App authentication.""" - -import secrets - -import pytest - -from github_runner_manager.configuration.github import ( - GitHubAppAuth, - GitHubPath, - parse_github_path, -) -from github_runner_manager.github_client import GithubClient -from github_runner_manager.manager.models import InstanceID - - -@pytest.fixture(autouse=True, scope="module") -def openstack_cleanup(): - """Override the autouse openstack_cleanup fixture — this module has no OpenStack dependency.""" - yield - - -@pytest.fixture(scope="module") -def github_app_auth(pytestconfig: pytest.Config) -> GitHubAppAuth: - """Get GitHub App auth configuration, skip if credentials not provided.""" - app_client_id = pytestconfig.getoption("--github-app-client-id") - installation_id = pytestconfig.getoption("--github-app-installation-id") - private_key = pytestconfig.getoption("--github-app-private-key") - - if not all([app_client_id, installation_id, private_key]): - pytest.skip("GitHub App credentials not provided") - - return GitHubAppAuth( - app_client_id=app_client_id, - installation_id=int(installation_id), - private_key=private_key, - ) - - -@pytest.fixture(scope="module") -def github_app_client(github_app_auth: GitHubAppAuth) -> GithubClient: - """Create a GithubClient using GitHub App authentication.""" - return GithubClient(auth=github_app_auth) - - -@pytest.fixture(scope="module") -def github_path(pytestconfig: pytest.Config) -> GitHubPath: - """Get the GitHub path from test configuration.""" - path_str = pytestconfig.getoption("--github-repository") - if not path_str: - pytest.skip("GitHub repository path not provided") - return parse_github_path(path_str, runner_group="default") - - -def test_get_jit_token_with_github_app_auth( - github_app_client: GithubClient, github_path: GitHubPath -) -> None: - """ - arrange: GithubClient created with GitHubAppAuth credentials. - act: Request a JIT config token to register a runner. - assert: Token is returned and runner is created, then clean up. - """ - prefix = "test-app-auth" - instance_id = InstanceID(prefix=prefix, suffix=secrets.token_hex(6)) - - runner = None - try: - jit_token, runner = github_app_client.get_runner_registration_jittoken( - github_path, instance_id=instance_id, labels=[prefix] - ) - - assert jit_token, "JIT config token should be non-empty" - assert runner.id > 0 - assert runner.identity.instance_id == instance_id - finally: - if runner is not None: - github_app_client.delete_runner(github_path, runner.id) diff --git a/tests/conftest.py b/tests/conftest.py index 9643176962..70e77a3aa5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -166,3 +166,21 @@ def pytest_addoption(parser: Parser): help="The DockerHub mirror URL to use for testing.", default=None, ) + parser.addoption( + "--github-app-client-id", + action="store", + help="The GitHub App Client ID for GitHub App authentication testing.", + default=os.environ.get("GITHUB_APP_CLIENT_ID"), + ) + parser.addoption( + "--github-app-installation-id", + action="store", + help="The GitHub App installation ID for GitHub App authentication testing.", + default=os.environ.get("GITHUB_APP_INSTALLATION_ID"), + ) + parser.addoption( + "--github-app-private-key", + action="store", + help="The GitHub App PEM-encoded private key for GitHub App authentication testing.", + default=os.environ.get("GITHUB_APP_PRIVATE_KEY"), + ) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c2c0e76fbe..0246b3dd7b 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -67,12 +67,24 @@ class GitHubConfig: """GitHub configuration for tests. Attributes: - token: GitHub personal access token. + token: GitHub personal access token (always required for test harness API calls). path: GitHub repository path in / or / format. + app_client_id: GitHub App Client ID (optional, for App auth testing). + installation_id: GitHub App installation ID (optional, for App auth testing). + private_key: GitHub App PEM-encoded private key (optional, for App auth testing). + has_app_auth: Whether GitHub App authentication credentials are configured. """ token: str path: str + app_client_id: str | None = None + installation_id: int | None = None + private_key: str | None = None + + @property + def has_app_auth(self) -> bool: + """Whether GitHub App authentication credentials are configured.""" + return all((self.app_client_id, self.installation_id, self.private_key)) @dataclass diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 5ff52ca98c..5e3a9371e2 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -29,6 +29,9 @@ from charm import UPGRADE_MSG from charm_state import ( BASE_VIRTUAL_MACHINES_CONFIG_NAME, + GITHUB_APP_CLIENT_ID_CONFIG_NAME, + GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, + GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, PATH_CONFIG_NAME, RECONCILE_INTERVAL_CONFIG_NAME, TEST_MODE_CONFIG_NAME, @@ -152,12 +155,22 @@ async def deploy_github_runner_charm( default_config = { PATH_CONFIG_NAME: github_config.path, - TOKEN_CONFIG_NAME: github_config.token, BASE_VIRTUAL_MACHINES_CONFIG_NAME: 0, TEST_MODE_CONFIG_NAME: "insecure", RECONCILE_INTERVAL_CONFIG_NAME: reconcile_interval, } + if github_config.has_app_auth: + secret_id = await model.add_secret( + name=f"{app_name}-gh-app-key", + data_args=[f"private-key={github_config.private_key}"], + ) + default_config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] = github_config.app_client_id + default_config[GITHUB_APP_INSTALLATION_ID_CONFIG_NAME] = github_config.installation_id + default_config[GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME] = secret_id + else: + default_config[TOKEN_CONFIG_NAME] = github_config.token + if config: default_config.update(config) diff --git a/tests/integration/test_e2e.py b/tests/integration/test_e2e.py index a4a8f2e825..5ff836dbad 100644 --- a/tests/integration/test_e2e.py +++ b/tests/integration/test_e2e.py @@ -1,5 +1,12 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. + +"""End-to-end integration test. + +Uses GitHub App authentication when credentials are provided, falling back to PAT. +""" + +import logging from typing import AsyncIterator import pytest @@ -9,6 +16,7 @@ from juju.application import Application from juju.model import Model +from tests.integration.conftest import GitHubConfig from tests.integration.helpers.common import ( DISPATCH_E2E_TEST_RUN_WORKFLOW_FILENAME, dispatch_workflow, @@ -16,6 +24,27 @@ from tests.integration.helpers.openstack import OpenStackInstanceHelper +@pytest.fixture(scope="module") +def github_config(pytestconfig: pytest.Config, github_config: GitHubConfig) -> GitHubConfig: + """Override github_config to prefer GitHub App auth when credentials are available.""" + app_client_id = pytestconfig.getoption("--github-app-client-id") or None + installation_id_raw = pytestconfig.getoption("--github-app-installation-id") or None + private_key = pytestconfig.getoption("--github-app-private-key") or None + + if not all((app_client_id, installation_id_raw, private_key)): + logging.info("Using PAT authentication for e2e test") + return github_config + + logging.info("Using GitHub App authentication for e2e test") + return GitHubConfig( + token=github_config.token, + path=github_config.path, + app_client_id=app_client_id, + installation_id=int(installation_id_raw), # type: ignore[arg-type] + private_key=private_key, + ) + + @pytest_asyncio.fixture(scope="function", name="app") async def app_fixture( model: Model, From ed425a17b041475bbf1a1dd5c343c21f9f5f536a Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 2 Apr 2026 14:40:20 +0200 Subject: [PATCH 03/13] fix(ci): pin charmcraft to latest/beta to work around 4.2.0 regression charmcraft 4.2.0 (promoted to latest/stable on 2026-04-01) rejects the short-form platform syntax in charmcraft.yaml. Pin to latest/beta (4.0.1) as a workaround. Revert to latest/stable once the upstream bug is fixed. --- .github/workflows/e2e_test.yaml | 4 ++++ .github/workflows/integration_test.yaml | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml index d1b49dad12..12e40cb651 100644 --- a/.github/workflows/e2e_test.yaml +++ b/.github/workflows/e2e_test.yaml @@ -15,6 +15,10 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: + # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. + # charmcraft 4.2.0 (latest/stable since 2026-04-01) rejects the short-form + # platform syntax in charmcraft.yaml. Pin to latest/beta (4.0.1) as a workaround. + charmcraft-channel: latest/beta juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 836f6ba4dd..5f1506c0f3 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -17,6 +17,10 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: + # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. + # charmcraft 4.2.0 (latest/stable since 2026-04-01) rejects the short-form + # platform syntax in charmcraft.yaml. Pin to latest/beta (4.0.1) as a workaround. + charmcraft-channel: latest/beta juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 @@ -43,6 +47,8 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: + # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. + charmcraft-channel: latest/beta juju-channel: 3.6/stable pre-run-script: tests/integration/setup-integration-tests.sh provider: lxd @@ -73,6 +79,8 @@ jobs: matrix: base: ["22.04", "24.04"] with: + # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. + charmcraft-channel: latest/beta juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 From 550b179d909da4797faca88ec5ab9483397f799f Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 2 Apr 2026 18:19:38 +0200 Subject: [PATCH 04/13] revert: remove charmcraft channel pin, upstream reverted 4.2.0 charmcraft latest/stable is back to 4.0.1. Remove the workaround pin. --- .github/workflows/e2e_test.yaml | 4 ---- .github/workflows/integration_test.yaml | 4 ---- 2 files changed, 8 deletions(-) diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml index 12e40cb651..d1b49dad12 100644 --- a/.github/workflows/e2e_test.yaml +++ b/.github/workflows/e2e_test.yaml @@ -15,10 +15,6 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: - # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. - # charmcraft 4.2.0 (latest/stable since 2026-04-01) rejects the short-form - # platform syntax in charmcraft.yaml. Pin to latest/beta (4.0.1) as a workaround. - charmcraft-channel: latest/beta juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 5f1506c0f3..8f10a0d055 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -17,10 +17,6 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: - # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. - # charmcraft 4.2.0 (latest/stable since 2026-04-01) rejects the short-form - # platform syntax in charmcraft.yaml. Pin to latest/beta (4.0.1) as a workaround. - charmcraft-channel: latest/beta juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 From 58e3dba40b93abd7ea24ebed0d48645a8bec5929 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 8 Apr 2026 09:11:36 +0200 Subject: [PATCH 05/13] fix(test): add GitHub App envs to tox env --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index 4c67d72ed3..b90b0ad2cd 100644 --- a/tox.ini +++ b/tox.ini @@ -124,6 +124,8 @@ pass_env = OS_PASSWORD OS_NETWORK OS_REGION_NAME + GITHUB_APP_INSTALLATION_ID + GITHUB_APP_PRIVATE_KEY deps = juju3.1: juju==3.1.* juju3.6: juju==3.6.* From a3e76298b23562fd55ee5c6eb945693754cbced3 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 8 Apr 2026 10:28:48 +0200 Subject: [PATCH 06/13] fix(test): grant Juju secret to app after deploy The GitHub App private key secret was created but never granted to the application, causing secret-get to fail during config-changed. --- tests/integration/helpers/common.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 5e3a9371e2..53629e93c7 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -160,9 +160,11 @@ async def deploy_github_runner_charm( RECONCILE_INTERVAL_CONFIG_NAME: reconcile_interval, } + secret_name = None if github_config.has_app_auth: + secret_name = f"{app_name}-gh-app-key" secret_id = await model.add_secret( - name=f"{app_name}-gh-app-key", + name=secret_name, data_args=[f"private-key={github_config.private_key}"], ) default_config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] = github_config.app_client_id @@ -184,6 +186,9 @@ async def deploy_github_runner_charm( **(deploy_kwargs or {}), ) + if secret_name: + await model.grant_secret(secret_name, app_name) + if wait_idle: await model.wait_for_idle(status=ACTIVE, timeout=60 * 20) From a9a35738ebc7490883769f3d1647d595ca73e88a Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 8 Apr 2026 16:00:20 +0200 Subject: [PATCH 07/13] Apply suggestions from code review Co-authored-by: Erin Conley --- docs/how-to/change-token.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/how-to/change-token.md b/docs/how-to/change-token.md index 2497213f15..7aa3d93522 100644 --- a/docs/how-to/change-token.md +++ b/docs/how-to/change-token.md @@ -3,7 +3,7 @@ The charm supports two authentication methods: a GitHub App or a personal access token (PAT). See [Authentication and token scopes](https://charmhub.io/github-runner/docs/reference-token-scopes) for required permissions. -## GitHub App authentication +## Authenticate using a GitHub App Create a [GitHub App](https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/registering-a-github-app) with the required permissions and install it on the target organization or repository. @@ -29,7 +29,7 @@ To rotate the private key, update the Juju secret: juju update-secret github-app-key private-key="$(cat /path/to/new-private-key.pem)" ``` -## Personal access token authentication +## Authenticate using a personal access token Create a new [GitHub Personal Access Token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens). From 75f4f2ba0962a2b3b2dd27ddd27d7fc9fa162594 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 8 Apr 2026 16:04:11 +0200 Subject: [PATCH 08/13] docs: clarify placeholder variables in GitHub App setup Add descriptions for all placeholder variables in the juju config command to help users find the required values. --- docs/how-to/change-token.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/how-to/change-token.md b/docs/how-to/change-token.md index 7aa3d93522..e7efbc6779 100644 --- a/docs/how-to/change-token.md +++ b/docs/how-to/change-token.md @@ -13,7 +13,12 @@ Store the App's PEM-encoded private key in a Juju secret: juju add-secret github-app-key private-key="$(cat /path/to/private-key.pem)" ``` -Note the secret ID from the output (e.g. `secret:abc123def`), then grant it to the charm and configure the App credentials: +Note the secret ID from the output (e.g. `secret:abc123def`), then grant it to the charm and configure the App credentials. + +- ``: the Juju application name (e.g. `github-runner`) +- ``: the Client ID shown on the App's settings page (Settings > Developer settings > GitHub Apps) +- ``: the numeric ID in the URL when viewing the App installation on the organization or user account (e.g. `https://github.com/organizations//settings/installations/`) +- ``: the Juju secret ID from the previous step ```shell juju grant-secret github-app-key From a04d173cf027da3f53b7153af1cc256d37124961 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 8 Apr 2026 16:10:42 +0200 Subject: [PATCH 09/13] chore: remove slot references from workflow comment --- .github/workflows/e2e_test.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml index d1b49dad12..fc49c266a6 100644 --- a/.github/workflows/e2e_test.yaml +++ b/.github/workflows/e2e_test.yaml @@ -19,8 +19,8 @@ jobs: provider: lxd test-tox-env: integration-juju3.6 modules: '["test_e2e"]' - # INTEGRATION_TOKEN, OS_PASSWORD, GITHUB_APP_INSTALLATION_ID (base slot), - # GITHUB_APP_PRIVATE_KEY (slot 11) are passed through + # INTEGRATION_TOKEN, OS_PASSWORD, GITHUB_APP_INSTALLATION_ID, + # GITHUB_APP_PRIVATE_KEY are passed through # INTEGRATION_TEST_SECRET_ENV_VALUE_ mapping. See CONTRIBUTING.md for more details. extra-arguments: | -m=openstack \ From fb98e530a08491f0b83cade3705a57ca389a5e8d Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 8 Apr 2026 16:13:59 +0200 Subject: [PATCH 10/13] docs: fix vale latinism in CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 195645d0c0..535306eeef 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,7 +50,7 @@ To prefer explicit setting arguments passing, use `extra-arguments` of the [following reusable workflow](https://github.com/canonical/operator-workflows/blob/main/.github/workflows/integration_test_run.yaml) to pass in non-sensitive values. For sensitive values (`INTEGRATION_TOKEN`, `OS_PASSWORD`, `GITHUB_APP_INSTALLATION_ID`, -`GITHUB_APP_PRIVATE_KEY`, etc.), map them through `INTEGRATION_TEST_SECRET_ENV_NAME_` / +`GITHUB_APP_PRIVATE_KEY`, and so on), map them through `INTEGRATION_TEST_SECRET_ENV_NAME_` / `INTEGRATION_TEST_SECRET_ENV_VALUE_` environment variable settings under [repository](https://github.com/canonical/github-runner-operator) > settings > variables > actions. This is to prevent GitHub from leaking secrets when passing them over the CLI calls (unresolved). From 9b4ad16eb9ce39972dcbca7ff5049424a5ceaf0b Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 10 Apr 2026 16:53:33 +0200 Subject: [PATCH 11/13] chore(ci): revert charmcraft channel to latest/stable The charmcraft 4.2.0 regression has been fixed, so the temporary pin to latest/beta is no longer needed. --- .github/workflows/integration_test.yaml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 8f10a0d055..51f6a69e5a 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -43,8 +43,7 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: - # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. - charmcraft-channel: latest/beta + charmcraft-channel: latest/stable juju-channel: 3.6/stable pre-run-script: tests/integration/setup-integration-tests.sh provider: lxd @@ -75,8 +74,7 @@ jobs: matrix: base: ["22.04", "24.04"] with: - # TODO: revert to latest/stable once charmcraft 4.2.0 regression is fixed. - charmcraft-channel: latest/beta + charmcraft-channel: latest/stable juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 From 58c2ec5aebe26834eed8cb6d9a59fc01a7fafd65 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 10 Apr 2026 17:01:20 +0200 Subject: [PATCH 12/13] fix: remove unnecessary defaults from GitHub App config options Charmcraft config options don't require defaults. The empty-string and zero defaults obscured whether the user had actually provided the GitHub App fields. Without defaults, unset config returns None which the validation logic already handles correctly. --- charmcraft.yaml | 3 --- src/charm.py | 28 ++++++++++++++-------------- src/charm_state.py | 6 +++--- tests/unit/factories.py | 6 ------ 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 952287d9f3..236385f112 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -143,19 +143,16 @@ config: https://charmhub.io/github-runner/docs/how-to-change-token. github-app-client-id: type: string - default: "" description: >- GitHub App Client ID used instead of `token` for GitHub API authentication. This is the Client ID shown on the GitHub App settings page (e.g. "Iv23liXXXXXX"). The legacy numeric App ID is also accepted. github-app-installation-id: type: int - default: 0 description: >- GitHub App installation ID used instead of `token` for GitHub API authentication. github-app-private-key-secret-id: type: string - default: "" description: >- Juju secret ID containing the PEM-encoded GitHub App private key under the `private-key` field. Use this together with `github-app-client-id` and `github-app-installation-id`. diff --git a/src/charm.py b/src/charm.py index 0ea409e88a..9686f35f32 100755 --- a/src/charm.py +++ b/src/charm.py @@ -205,15 +205,15 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self._stored.set_default( path=self.config[PATH_CONFIG_NAME], # for detecting changes token=self.config[TOKEN_CONFIG_NAME], # for detecting changes - github_app_client_id=self.config[ + github_app_client_id=self.config.get( GITHUB_APP_CLIENT_ID_CONFIG_NAME - ], # for detecting changes - github_app_installation_id=self.config[ + ), # for detecting changes + github_app_installation_id=self.config.get( GITHUB_APP_INSTALLATION_ID_CONFIG_NAME - ], # for detecting changes - github_app_private_key_secret_id=self.config[ + ), # for detecting changes + github_app_private_key_secret_id=self.config.get( GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME - ], # for detecting changes + ), # for detecting changes labels=self.config[LABELS_CONFIG_NAME], # for detecting changes allow_external_contributor=self.config[ ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME @@ -353,24 +353,24 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: if self.config[TOKEN_CONFIG_NAME] != self._stored.token: self._stored.token = self.config[TOKEN_CONFIG_NAME] flush_runners = True - if self.config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] != self._stored.github_app_client_id: - self._stored.github_app_client_id = self.config[GITHUB_APP_CLIENT_ID_CONFIG_NAME] + if self.config.get(GITHUB_APP_CLIENT_ID_CONFIG_NAME) != self._stored.github_app_client_id: + self._stored.github_app_client_id = self.config.get(GITHUB_APP_CLIENT_ID_CONFIG_NAME) flush_runners = True if ( - self.config[GITHUB_APP_INSTALLATION_ID_CONFIG_NAME] + self.config.get(GITHUB_APP_INSTALLATION_ID_CONFIG_NAME) != self._stored.github_app_installation_id ): - self._stored.github_app_installation_id = self.config[ + self._stored.github_app_installation_id = self.config.get( GITHUB_APP_INSTALLATION_ID_CONFIG_NAME - ] + ) flush_runners = True if ( - self.config[GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME] + self.config.get(GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME) != self._stored.github_app_private_key_secret_id ): - self._stored.github_app_private_key_secret_id = self.config[ + self._stored.github_app_private_key_secret_id = self.config.get( GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME - ] + ) flush_runners = True if self.config[PATH_CONFIG_NAME] != self._stored.path: self._stored.path = self.config[PATH_CONFIG_NAME] diff --git a/src/charm_state.py b/src/charm_state.py index 9e1c1faebb..c17a10b3b2 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -168,12 +168,12 @@ def from_charm(cls, charm: CharmBase) -> "GithubConfig": # noqa: C901 path_str = cast(str, charm.config.get(PATH_CONFIG_NAME, "")) token = cast(str, charm.config.get(TOKEN_CONFIG_NAME)) or None - app_client_id = cast(str, charm.config.get(GITHUB_APP_CLIENT_ID_CONFIG_NAME, "")) or None + app_client_id = cast(str, charm.config.get(GITHUB_APP_CLIENT_ID_CONFIG_NAME)) or None installation_id = ( - cast(int, charm.config.get(GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, 0)) or None + cast(int, charm.config.get(GITHUB_APP_INSTALLATION_ID_CONFIG_NAME)) or None ) private_key_secret_id = ( - cast(str, charm.config.get(GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, "")) or None + cast(str, charm.config.get(GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME)) or None ) if not path_str: diff --git a/tests/unit/factories.py b/tests/unit/factories.py index e58c11cefb..c5b965aeb3 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -24,9 +24,6 @@ DEBUG_SSH_INTEGRATION_NAME, DOCKERHUB_MIRROR_CONFIG_NAME, FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME, - GITHUB_APP_CLIENT_ID_CONFIG_NAME, - GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, - GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, GROUP_CONFIG_NAME, LABELS_CONFIG_NAME, MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME, @@ -135,9 +132,6 @@ class Meta: DOCKERHUB_MIRROR_CONFIG_NAME: "", FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME: "", GROUP_CONFIG_NAME: "default", - GITHUB_APP_CLIENT_ID_CONFIG_NAME: "", - GITHUB_APP_INSTALLATION_ID_CONFIG_NAME: 0, - GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME: "", LABELS_CONFIG_NAME: "", MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME: 0, OPENSTACK_CLOUDS_YAML_CONFIG_NAME: yaml.safe_dump( From c7bcd77396f4e63a7f37caaff6e782fe520eb327 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 10 Apr 2026 17:12:50 +0200 Subject: [PATCH 13/13] fix: add GITHUB_APP_CLIENT_ID to tox integration pass_env The other two GitHub App env vars were passed through but this one was missing, causing tox to silently drop it and fall back to PAT. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index b90b0ad2cd..01868c6d22 100644 --- a/tox.ini +++ b/tox.ini @@ -124,6 +124,7 @@ pass_env = OS_PASSWORD OS_NETWORK OS_REGION_NAME + GITHUB_APP_CLIENT_ID GITHUB_APP_INSTALLATION_ID GITHUB_APP_PRIVATE_KEY deps =