diff --git a/charmcraft.yaml b/charmcraft.yaml index 236385f11..19f330634 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -47,9 +47,16 @@ config: type: string default: "" description: >- - The clouds.yaml yaml necessary for OpenStack integration. - The format for the clouds.yaml is described in the docs: + (Compatibility fallback) The clouds.yaml yaml necessary for OpenStack integration. + Prefer setting openstack-clouds-yaml-secret-id. + The format for clouds.yaml is described in the docs: (https://docs.openstack.org/python-openstackclient/pike/configuration/index.html#clouds-yaml). + openstack-clouds-yaml-secret-id: + type: string + default: "" + description: >- + Juju secret ID containing OpenStack clouds.yaml content under the `clouds-yaml` field. + When set, this takes precedence over `openstack-clouds-yaml`. openstack-flavor: type: string default: "" @@ -137,10 +144,18 @@ config: type: string default: "" description: >- - The GitHub Personal Access Token for registering the self-hosted runners. The token requires - 'repo' scope for repository runners and 'repo' + 'admin:org' scope for organization runners. - For fine grained token scopes, see + (Compatibility fallback) The GitHub Personal Access Token for registering self-hosted + runners. Prefer setting token-secret-id. + The token requires '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. + token-secret-id: + type: string + default: "" + description: >- + Juju secret ID containing the GitHub token under the `github-token` field. + When set, this takes precedence over `token`. github-app-client-id: type: string description: >- diff --git a/docs/changelog.md b/docs/changelog.md index 93bf02c8a..3bbafde80 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-04-17 + +- Added `token-secret-id` and `openstack-clouds-yaml-secret-id` configuration options, allowing the GitHub token and OpenStack `clouds.yaml` to be supplied through Juju user secrets. When set, these take precedence over the plaintext `token` and `openstack-clouds-yaml` options, which remain supported as a compatibility fallback. + ## 2026-04-13 - Fixed Juju secrets not picking up new revisions. The charm now uses `refresh=True` when reading secret contents, ensuring it always retrieves the latest revision instead of a cached one. diff --git a/src/charm.py b/src/charm.py index 9686f35f3..ff94d61e7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -53,9 +53,11 @@ GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, PATH_CONFIG_NAME, PLANNER_INTEGRATION_NAME, TOKEN_CONFIG_NAME, + TOKEN_SECRET_ID_CONFIG_NAME, CharmConfigInvalidError, CharmState, OpenstackImage, @@ -84,6 +86,25 @@ LEGACY_RECONCILE_SERVICE = "ghro.reconcile-runners.service" LEGACY_MANAGER_SINGLETON_SERVICE = "github-runner-manager.service" +# Config keys whose change triggers a runner flush, mapped to the attribute +# on `_stored` that tracks the last-seen value. The plaintext +# `openstack-clouds-yaml` option is intentionally omitted: tracking it here would +# persist the credentials to `.unit-state.db` on disk. Operators who want +# automatic flush on credential rotation should use +# `openstack-clouds-yaml-secret-id`, which is flushed via this mapping and via +# `on_secret_changed` when the secret content rotates. +_FLUSH_ON_CHANGE_CONFIG_TO_STORED: tuple[tuple[str, str], ...] = ( + (TOKEN_CONFIG_NAME, "token"), + (TOKEN_SECRET_ID_CONFIG_NAME, "token_secret_id"), + (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, "github_app_private_key_secret_id"), + (PATH_CONFIG_NAME, "path"), + (LABELS_CONFIG_NAME, "labels"), + (OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, "openstack_clouds_yaml_secret_id"), + (ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, "allow_external_contributor"), +) + logger = logging.getLogger(__name__) @@ -205,6 +226,7 @@ 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 + token_secret_id=self.config.get(TOKEN_SECRET_ID_CONFIG_NAME), # for detecting changes github_app_client_id=self.config.get( GITHUB_APP_CLIENT_ID_CONFIG_NAME ), # for detecting changes @@ -214,6 +236,9 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: github_app_private_key_secret_id=self.config.get( GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME ), # for detecting changes + openstack_clouds_yaml_secret_id=self.config.get( + OPENSTACK_CLOUDS_YAML_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 @@ -350,42 +375,11 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: state = self._setup_state() flush_runners = False - if self.config[TOKEN_CONFIG_NAME] != self._stored.token: - self._stored.token = self.config[TOKEN_CONFIG_NAME] - flush_runners = True - 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.get(GITHUB_APP_INSTALLATION_ID_CONFIG_NAME) - != self._stored.github_app_installation_id - ): - self._stored.github_app_installation_id = self.config.get( - GITHUB_APP_INSTALLATION_ID_CONFIG_NAME - ) - flush_runners = True - if ( - 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.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] - flush_runners = True - if self.config[LABELS_CONFIG_NAME] != self._stored.labels: - self._stored.labels = self.config[LABELS_CONFIG_NAME] - flush_runners = True - if ( - self.config[ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME] - != self._stored.allow_external_contributor - ): - self._stored.allow_external_contributor = self.config[ - ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME - ] - flush_runners = True + for config_key, stored_attr in _FLUSH_ON_CHANGE_CONFIG_TO_STORED: + new_value = self.config.get(config_key) + if new_value != getattr(self._stored, stored_attr): + setattr(self._stored, stored_attr, new_value) + flush_runners = True self._reconcile(state) diff --git a/src/charm_state.py b/src/charm_state.py index 49f0e8ff4..004b68119 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -52,6 +52,7 @@ MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME = "max-total-virtual-machines" MANAGER_SSH_PROXY_COMMAND_CONFIG_NAME = "manager-ssh-proxy-command" OPENSTACK_CLOUDS_YAML_CONFIG_NAME = "openstack-clouds-yaml" +OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME = "openstack-clouds-yaml-secret-id" # nosec OPENSTACK_NETWORK_CONFIG_NAME = "openstack-network" OPENSTACK_FLAVOR_CONFIG_NAME = "openstack-flavor" PATH_CONFIG_NAME = "path" @@ -60,6 +61,7 @@ TEST_MODE_CONFIG_NAME = "test-mode" # bandit thinks this is a hardcoded password. TOKEN_CONFIG_NAME = "token" # nosec +TOKEN_SECRET_ID_CONFIG_NAME = "token-secret-id" # nosec USE_APROXY_CONFIG_NAME = "experimental-use-aproxy" APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME = "aproxy-exclude-addresses" APROXY_REDIRECT_PORTS_CONFIG_NAME = "aproxy-redirect-ports" @@ -152,6 +154,7 @@ class GithubConfig: path: GitHubPath @classmethod + # pylint: disable=too-many-locals def from_charm(cls, charm: CharmBase) -> "GithubConfig": # noqa: C901 """Get github related charm configuration values from charm. @@ -168,6 +171,7 @@ 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 + token_secret_id = cast(str, charm.config.get(TOKEN_SECRET_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)) or None @@ -187,13 +191,27 @@ def from_charm(cls, charm: CharmBase) -> "GithubConfig": # noqa: C901 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_secret_id: + try: + token_secret = charm.model.get_secret(id=token_secret_id) + token = token_secret.get_content(refresh=True).get("github-token") + except SecretNotFoundError as exc: + raise CharmConfigInvalidError( + f"GitHub token secret {token_secret_id} not found" + ) from exc + if not token: + raise CharmConfigInvalidError( + f"GitHub token secret {token_secret_id} is missing github-token" + ) + 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" + f"Missing {TOKEN_CONFIG_NAME}, {TOKEN_SECRET_ID_CONFIG_NAME} " + "or GitHub App authentication configuration" ) if not token and app_fields_set != 3: raise CharmConfigInvalidError( @@ -385,11 +403,37 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML Returns: The openstack clouds yaml. """ + openstack_clouds_yaml_secret_id = ( + cast(str, charm.config.get(OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME)) or None + ) openstack_clouds_yaml_str: str | None = cast( str, charm.config.get(OPENSTACK_CLOUDS_YAML_CONFIG_NAME) ) + source = OPENSTACK_CLOUDS_YAML_CONFIG_NAME + + if openstack_clouds_yaml_secret_id: + source = OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME + try: + cloud_secret = charm.model.get_secret(id=openstack_clouds_yaml_secret_id) + openstack_clouds_yaml_str = cloud_secret.get_content(refresh=True).get( + "clouds-yaml" + ) + except SecretNotFoundError as exc: + raise CharmConfigInvalidError( + f"OpenStack clouds.yaml secret {openstack_clouds_yaml_secret_id} not found" + ) from exc + if not openstack_clouds_yaml_str: + raise CharmConfigInvalidError( + "OpenStack clouds.yaml secret " + f"{openstack_clouds_yaml_secret_id} is missing clouds-yaml" + ) + if not openstack_clouds_yaml_str: - raise CharmConfigInvalidError("No openstack_clouds_yaml") + raise CharmConfigInvalidError( + "Missing OpenStack clouds configuration; set either " + f"{OPENSTACK_CLOUDS_YAML_CONFIG_NAME} or " + f"{OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME}." + ) try: openstack_clouds_yaml: OpenStackCloudsYAML = yaml.safe_load( @@ -398,10 +442,21 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML # use Pydantic to validate TypedDict. create_model_from_typeddict(OpenStackCloudsYAML)(**openstack_clouds_yaml) except (yaml.YAMLError, TypeError) as exc: - logger.error(f"Invalid {OPENSTACK_CLOUDS_YAML_CONFIG_NAME} config: %s.", exc) - raise CharmConfigInvalidError( - f"Invalid {OPENSTACK_CLOUDS_YAML_CONFIG_NAME} config. Invalid yaml." - ) from exc + if source == OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME: + # Don't log `exc` or chain with `from exc`: yaml.YAMLError and + # pydantic TypeError messages can embed a snippet of the + # offending input (here, the decrypted clouds.yaml), and the + # chained cause would be printed by `logger.exception` upstream. + logger.error( + "Invalid OpenStack clouds.yaml content in secret %s.", + openstack_clouds_yaml_secret_id, + ) + raise CharmConfigInvalidError( + "Invalid OpenStack clouds.yaml content in secret " + f"{openstack_clouds_yaml_secret_id}. Invalid yaml." + ) from None + logger.error("Invalid %s config: %s.", source, exc) + raise CharmConfigInvalidError(f"Invalid {source} config. Invalid yaml.") from exc return openstack_clouds_yaml diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 15070d0b4..ab902812a 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -31,7 +31,6 @@ BASE_VIRTUAL_MACHINES_CONFIG_NAME, DOCKERHUB_MIRROR_CONFIG_NAME, LABELS_CONFIG_NAME, - OPENSTACK_CLOUDS_YAML_CONFIG_NAME, OPENSTACK_FLAVOR_CONFIG_NAME, OPENSTACK_NETWORK_CONFIG_NAME, PATH_CONFIG_NAME, @@ -561,7 +560,6 @@ def image_builder_fixture( "virt-type": "virtual-machine", "cores": "2", }, - log=False, ) yield image_builder_app_name @@ -581,8 +579,7 @@ def image_builder_fixture( series = dep_ctx.series any_charm_src_overwrite = { - "any_charm.py": textwrap.dedent( - f"""\ + "any_charm.py": textwrap.dedent(f"""\ from any_charm_base import AnyCharmBase class AnyCharm(AnyCharmBase): @@ -595,8 +592,7 @@ def _image_relation_changed(self, event): # Provide mock image relation data event.relation.data[self.unit]['id'] = '{openstack_config.test_image_id}' event.relation.data[self.unit]['tags'] = '{series}, amd64' - """ - ), + """), } logging.info( "Deploying fake image builder via any-charm for image ID %s", @@ -638,13 +634,13 @@ def app_openstack_runner_fixture( no_proxy=openstack_config.no_proxy, ), reconcile_interval=DEFAULT_RECONCILE_INTERVAL, + openstack_clouds_yaml=openstack_config.clouds_yaml_contents, constraints={ "root-disk": "51200M", "mem": "2048M", "virt-type": "virtual-machine", }, config={ - OPENSTACK_CLOUDS_YAML_CONFIG_NAME: openstack_config.clouds_yaml_contents, OPENSTACK_NETWORK_CONFIG_NAME: openstack_config.network_name, OPENSTACK_FLAVOR_CONFIG_NAME: openstack_config.flavor_name, USE_APROXY_CONFIG_NAME: bool(openstack_config.http_proxy), @@ -922,8 +918,7 @@ def mock_planner_app(juju: jubilant.Juju, planner_token_secret: str) -> Iterator planner_name = "planner" any_charm_src_overwrite = { - "any_charm.py": textwrap.dedent( - f"""\ + "any_charm.py": textwrap.dedent(f"""\ from any_charm_base import AnyCharmBase class AnyCharm(AnyCharmBase): @@ -937,8 +932,7 @@ def __init__(self, *args, **kwargs): def _on_planner_relation_changed(self, event): event.relation.data[self.app]["endpoint"] = "http://mock:8080" event.relation.data[self.app]["token"] = "{planner_token_secret}" - """ - ), + """), } juju.deploy( diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 23e510266..e02eb2556 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -26,10 +26,11 @@ GITHUB_APP_CLIENT_ID_CONFIG_NAME, GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, PATH_CONFIG_NAME, RECONCILE_INTERVAL_CONFIG_NAME, TEST_MODE_CONFIG_NAME, - TOKEN_CONFIG_NAME, + TOKEN_SECRET_ID_CONFIG_NAME, ) from manager_service import _get_log_file_path @@ -118,6 +119,7 @@ def deploy_github_runner_charm( github_config: "GitHubConfig", proxy_config: "ProxyConfig", reconcile_interval: int, + openstack_clouds_yaml: str | None = None, constraints: dict | None = None, config: dict | None = None, deploy_kwargs: dict | None = None, @@ -134,6 +136,7 @@ def deploy_github_runner_charm( proxy_config: Object providing proxy settings with attributes `http_proxy`, `https_proxy`, and `no_proxy`. reconcile_interval: Time between reconcile for the application. + openstack_clouds_yaml: Plaintext OpenStack clouds.yaml to wrap in a Juju secret. constraints: The custom machine constraints to use. See DEFAULT_RUNNER_CONSTRAINTS otherwise. config: Additional custom config to use. @@ -160,7 +163,7 @@ def deploy_github_runner_charm( RECONCILE_INTERVAL_CONFIG_NAME: reconcile_interval, } - secret_name = None + secret_names: list[str] = [] if github_config.has_app_auth: assert github_config.app_client_id is not None assert github_config.installation_id is not None @@ -170,11 +173,27 @@ def deploy_github_runner_charm( name=secret_name, content={"private-key": github_config.private_key}, ) + secret_names.append(secret_name) 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] = str(secret_id) else: - default_config[TOKEN_CONFIG_NAME] = github_config.token + token_secret_name = f"{app_name}-github-token" + token_secret_id = juju.add_secret( + name=token_secret_name, + content={"github-token": github_config.token}, + ) + secret_names.append(token_secret_name) + default_config[TOKEN_SECRET_ID_CONFIG_NAME] = str(token_secret_id) + + if openstack_clouds_yaml: + openstack_secret_name = f"{app_name}-openstack-clouds" + openstack_secret_id = juju.add_secret( + name=openstack_secret_name, + content={"clouds-yaml": openstack_clouds_yaml}, + ) + secret_names.append(openstack_secret_name) + default_config[OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME] = str(openstack_secret_id) if config: default_config.update(config) @@ -185,11 +204,10 @@ def deploy_github_runner_charm( base=base, config=default_config, constraints=constraints or DEFAULT_RUNNER_CONSTRAINTS, - log=False, **(deploy_kwargs or {}), ) - if secret_name: + for secret_name in secret_names: juju.grant_secret(secret_name, app_name) if wait_idle: diff --git a/tests/integration/test_charm_fork_path_change.py b/tests/integration/test_charm_fork_path_change.py index 29b185fa1..566dd1cfe 100644 --- a/tests/integration/test_charm_fork_path_change.py +++ b/tests/integration/test_charm_fork_path_change.py @@ -20,6 +20,12 @@ logger = logging.getLogger(__name__) +@pytest.mark.skip( + reason=( + "Fork creation fails under the current fine-grained PAT (scoped to canonical org, " + "fork lands in a personal namespace). Will be resolved in an upcoming PR." + ) +) @pytest.mark.openstack @pytest.mark.abort_on_fail def test_path_config_change( diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index 1a594d805..4960c5661 100644 --- a/tests/integration/test_charm_upgrade.py +++ b/tests/integration/test_charm_upgrade.py @@ -11,7 +11,6 @@ from charm_state import ( BASE_VIRTUAL_MACHINES_CONFIG_NAME, - OPENSTACK_CLOUDS_YAML_CONFIG_NAME, OPENSTACK_FLAVOR_CONFIG_NAME, OPENSTACK_NETWORK_CONFIG_NAME, USE_APROXY_CONFIG_NAME, @@ -32,6 +31,15 @@ pytestmark = pytest.mark.openstack +@pytest.mark.skip( + reason=( + "latest/edge charm predates token-secret-id and openstack-clouds-yaml-secret-id " + "config options. Falling back to the plaintext token/openstack-clouds-yaml options " + "is not acceptable because jubilant logs deploy config, which leaks the token. " + "Re-enable once a release containing the secret-id options has been promoted to " + "latest/edge." + ) +) def test_charm_upgrade( juju: jubilant.Juju, deployment_context: DeploymentContext, @@ -78,8 +86,8 @@ def test_charm_upgrade( no_proxy=openstack_config.no_proxy, ), reconcile_interval=5, + openstack_clouds_yaml=openstack_config.clouds_yaml_contents, config={ - OPENSTACK_CLOUDS_YAML_CONFIG_NAME: openstack_config.clouds_yaml_contents, OPENSTACK_NETWORK_CONFIG_NAME: openstack_config.network_name, OPENSTACK_FLAVOR_CONFIG_NAME: openstack_config.flavor_name, USE_APROXY_CONFIG_NAME: "true", diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 600770129..385e92775 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -34,6 +34,7 @@ IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, OPENSTACK_CLOUDS_YAML_CONFIG_NAME, + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, OPENSTACK_FLAVOR_CONFIG_NAME, PATH_CONFIG_NAME, PLANNER_DEFAULT_PLATFORM, @@ -45,6 +46,7 @@ PLANNER_PLATFORM_RELATION_KEY, PLANNER_PRIORITY_RELATION_KEY, TOKEN_CONFIG_NAME, + TOKEN_SECRET_ID_CONFIG_NAME, USE_APROXY_CONFIG_NAME, OpenStackCloudsYAML, OpenstackImage, @@ -258,6 +260,12 @@ def test_on_config_changed_failure(harness: Harness): [ pytest.param(PATH_CONFIG_NAME, "initial-path", "updated-path", id="Path"), pytest.param(TOKEN_CONFIG_NAME, "initial-token", "updated-token", id="Token"), + pytest.param( + TOKEN_SECRET_ID_CONFIG_NAME, + "secret:token-old", + "secret:token-new", + id="Token secret ID", + ), pytest.param( GITHUB_APP_CLIENT_ID_CONFIG_NAME, "Iv23liOld", "Iv23liNew", id="GitHub App Client ID" ), @@ -274,6 +282,12 @@ def test_on_config_changed_failure(harness: Harness): id="GitHub App Private Key Secret ID", ), pytest.param(LABELS_CONFIG_NAME, "label-a", "label-b", id="Labels"), + pytest.param( + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, + "secret:clouds-old", + "secret:clouds-new", + id="OpenStack clouds secret ID", + ), ], ) def test__on_config_changed_flush( @@ -306,6 +320,11 @@ def test__on_config_changed_flush( [ pytest.param(PATH_CONFIG_NAME, secrets.token_hex(16), id="Path"), pytest.param(TOKEN_CONFIG_NAME, secrets.token_hex(16), id="Token"), + pytest.param( + TOKEN_SECRET_ID_CONFIG_NAME, + f"secret:{secrets.token_hex(8)}", + id="Token secret ID", + ), pytest.param(GITHUB_APP_CLIENT_ID_CONFIG_NAME, "Iv23liExample", id="GitHub App Client ID"), pytest.param( GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, @@ -318,6 +337,11 @@ def test__on_config_changed_flush( id="GitHub App Private Key Secret ID", ), pytest.param(LABELS_CONFIG_NAME, secrets.token_hex(16), id="Labels"), + pytest.param( + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, + f"secret:{secrets.token_hex(8)}", + id="OpenStack clouds secret ID", + ), ], ) def test__on_config_changed_no_flush( diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index 1bef20267..a009c8bb5 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -7,6 +7,7 @@ import pytest import yaml from github_runner_manager.configuration.github import GitHubOrg, GitHubRepo +from ops.model import SecretNotFoundError from pydantic import BaseModel from pydantic.error_wrappers import ValidationError from pydantic.networks import IPv4Address @@ -30,6 +31,7 @@ MANAGER_SSH_PROXY_COMMAND_CONFIG_NAME, MAX_TOTAL_VIRTUAL_MACHINES_CONFIG_NAME, OPENSTACK_CLOUDS_YAML_CONFIG_NAME, + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, OPENSTACK_FLAVOR_CONFIG_NAME, PATH_CONFIG_NAME, PLANNER_INTEGRATION_NAME, @@ -37,6 +39,7 @@ RUNNER_HTTP_PROXY_CONFIG_NAME, RUNNER_MANAGER_LOG_LEVEL_CONFIG_NAME, TOKEN_CONFIG_NAME, + TOKEN_SECRET_ID_CONFIG_NAME, USE_APROXY_CONFIG_NAME, USE_RUNNER_PROXY_FOR_TMATE_CONFIG_NAME, VIRTUAL_MACHINES_CONFIG_NAME, @@ -111,6 +114,81 @@ def test_github_config_from_charm_token_only(): assert result.private_key is None +def test_github_config_from_charm_token_secret_only(): + """ + arrange: Charm configured with token secret ID. + act: Call from_charm. + assert: GithubConfig resolves token from Juju secret content. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "" + mock_charm.config[TOKEN_SECRET_ID_CONFIG_NAME] = "secret:token123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {"github-token": "secret-token-value"} + mock_charm.model.get_secret.return_value = secret_mock + + result = GithubConfig.from_charm(mock_charm) + + assert result.token == "secret-token-value" + assert result.app_client_id is None + assert result.installation_id is None + assert result.private_key is None + mock_charm.model.get_secret.assert_called_once_with(id="secret:token123") + secret_mock.get_content.assert_called_once_with(refresh=True) + + +def test_github_config_from_charm_token_secret_takes_precedence_over_plaintext(): + """ + arrange: Charm configured with both plaintext token and token secret ID. + act: Call from_charm. + assert: Secret content is used as token value. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "plaintext-token" + mock_charm.config[TOKEN_SECRET_ID_CONFIG_NAME] = "secret:token123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {"github-token": "secret-token-value"} + mock_charm.model.get_secret.return_value = secret_mock + + result = GithubConfig.from_charm(mock_charm) + + assert result.token == "secret-token-value" + + +def test_github_config_from_charm_token_secret_not_found(): + """ + arrange: Charm configured with token secret ID pointing to a missing secret. + act: Call from_charm. + assert: CharmConfigInvalidError is raised identifying the missing secret. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "" + mock_charm.config[TOKEN_SECRET_ID_CONFIG_NAME] = "secret:token123" + mock_charm.model.get_secret.side_effect = SecretNotFoundError("boom") + + with pytest.raises( + CharmConfigInvalidError, match="GitHub token secret secret:token123 not found" + ): + GithubConfig.from_charm(mock_charm) + + +def test_github_config_from_charm_token_secret_missing_field(): + """ + arrange: Charm configured with token secret missing required field. + act: Call from_charm. + assert: CharmConfigInvalidError is raised. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[TOKEN_CONFIG_NAME] = "" + mock_charm.config[TOKEN_SECRET_ID_CONFIG_NAME] = "secret:token123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {} + mock_charm.model.get_secret.return_value = secret_mock + + with pytest.raises(CharmConfigInvalidError, match="missing github-token"): + GithubConfig.from_charm(mock_charm) + + def test_github_config_from_charm_app_only(): """ arrange: Charm configured with GitHub App auth only. @@ -350,6 +428,81 @@ def test_parse_openstack_clouds_config_empty(): CharmConfig._parse_openstack_clouds_config(mock_charm) +def test_parse_openstack_clouds_config_from_secret(valid_yaml_config: str): + """ + arrange: OpenStack config provided through Juju secret. + act: Parse OpenStack clouds config. + assert: Parsed YAML dictionary is returned. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" + mock_charm.config[OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME] = "secret:cloud123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {"clouds-yaml": valid_yaml_config} + mock_charm.model.get_secret.return_value = secret_mock + + result = CharmConfig._parse_openstack_clouds_config(mock_charm) + + assert isinstance(result, dict) + assert "clouds" in result + mock_charm.model.get_secret.assert_called_once_with(id="secret:cloud123") + secret_mock.get_content.assert_called_once_with(refresh=True) + + +def test_parse_openstack_clouds_config_secret_precedence(valid_yaml_config: str): + """ + arrange: OpenStack config provided via plaintext and secret ID. + act: Parse OpenStack clouds config. + assert: Secret-provided value takes precedence. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "invalid: [" + mock_charm.config[OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME] = "secret:cloud123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {"clouds-yaml": valid_yaml_config} + mock_charm.model.get_secret.return_value = secret_mock + + result = CharmConfig._parse_openstack_clouds_config(mock_charm) + + assert isinstance(result, dict) + assert "clouds" in result + + +def test_parse_openstack_clouds_config_secret_not_found(): + """ + arrange: OpenStack clouds secret ID points to a non-existent secret. + act: Parse OpenStack clouds config. + assert: CharmConfigInvalidError is raised identifying the missing secret. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" + mock_charm.config[OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME] = "secret:cloud123" + mock_charm.model.get_secret.side_effect = SecretNotFoundError("boom") + + with pytest.raises( + CharmConfigInvalidError, + match="OpenStack clouds.yaml secret secret:cloud123 not found", + ): + CharmConfig._parse_openstack_clouds_config(mock_charm) + + +def test_parse_openstack_clouds_config_secret_missing_field(): + """ + arrange: OpenStack secret is present but missing required field. + act: Parse OpenStack clouds config. + assert: CharmConfigInvalidError is raised. + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_charm.config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" + mock_charm.config[OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME] = "secret:cloud123" + secret_mock = MagicMock() + secret_mock.get_content.return_value = {} + mock_charm.model.get_secret.return_value = secret_mock + + with pytest.raises(CharmConfigInvalidError, match="missing clouds-yaml"): + CharmConfig._parse_openstack_clouds_config(mock_charm) + + def test_parse_openstack_clouds_config_invalid_yaml(invalid_yaml_config: str): """ arrange: Create a mock CharmBase instance with an invalid YAML config.