From 14953bff697041e2fe50134a11821a56990f2a4a Mon Sep 17 00:00:00 2001 From: yhaliaw Date: Fri, 17 Apr 2026 13:10:05 +0800 Subject: [PATCH 01/20] Add support for integration test with juju secrets --- charmcraft.yaml | 23 +++++- src/charm.py | 17 ++++ src/charm_state.py | 32 ++++++++ tests/integration/helpers/common.py | 34 +++++++- tests/unit/test_charm.py | 24 ++++++ tests/unit/test_charm_state.py | 117 ++++++++++++++++++++++++++++ 6 files changed, 240 insertions(+), 7 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 236385f112..63fc32fdc4 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. + (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/src/charm.py b/src/charm.py index 9686f35f32..04facbda44 100755 --- a/src/charm.py +++ b/src/charm.py @@ -55,7 +55,9 @@ LABELS_CONFIG_NAME, PATH_CONFIG_NAME, PLANNER_INTEGRATION_NAME, + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, TOKEN_CONFIG_NAME, + TOKEN_SECRET_ID_CONFIG_NAME, CharmConfigInvalidError, CharmState, OpenstackImage, @@ -205,6 +207,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 +217,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 @@ -353,6 +359,9 @@ 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.get(TOKEN_SECRET_ID_CONFIG_NAME) != self._stored.token_secret_id: + self._stored.token_secret_id = self.config.get(TOKEN_SECRET_ID_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 @@ -378,6 +387,14 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: if self.config[LABELS_CONFIG_NAME] != self._stored.labels: self._stored.labels = self.config[LABELS_CONFIG_NAME] flush_runners = True + if ( + self.config.get(OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME) + != self._stored.openstack_clouds_yaml_secret_id + ): + self._stored.openstack_clouds_yaml_secret_id = self.config.get( + OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME + ) + flush_runners = True if ( self.config[ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME] != self._stored.allow_external_contributor diff --git a/src/charm_state.py b/src/charm_state.py index 49f0e8ff4a..baac4599a8 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" 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" @@ -168,6 +170,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,6 +190,17 @@ 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" @@ -385,9 +399,27 @@ 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) ) + + if openstack_clouds_yaml_secret_id: + 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") diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 23e5102662..a7774d74eb 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -26,10 +26,13 @@ GITHUB_APP_CLIENT_ID_CONFIG_NAME, GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, + OPENSTACK_CLOUDS_YAML_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 @@ -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,14 +173,39 @@ 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) + + clouds_yaml = None + if config: + clouds_yaml = cast(str | None, config.get(OPENSTACK_CLOUDS_YAML_CONFIG_NAME)) + + if clouds_yaml and not (config and config.get(OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME)): + openstack_secret_name = f"{app_name}-openstack-clouds" + openstack_secret_id = juju.add_secret( + name=openstack_secret_name, + content={"clouds-yaml": clouds_yaml}, + ) + secret_names.append(openstack_secret_name) + default_config[OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME] = str(openstack_secret_id) + + default_config[TOKEN_CONFIG_NAME] = "" + default_config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" if config: default_config.update(config) + if OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME in default_config: + default_config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" juju.deploy( charm_file, @@ -189,7 +217,7 @@ def deploy_github_runner_charm( **(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/unit/test_charm.py b/tests/unit/test_charm.py index 600770129b..385e92775d 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 1bef202674..4b63c59813 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -30,6 +30,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 +38,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 +113,64 @@ 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_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 +410,63 @@ 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_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. From c8dbc15e3560241c1b83afa7f77480ac09a48a2e Mon Sep 17 00:00:00 2001 From: yhaliaw Date: Fri, 17 Apr 2026 13:11:46 +0800 Subject: [PATCH 02/20] Remove incorrect argument for juju.deploy --- tests/integration/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 15070d0b4e..d83da748d1 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -561,7 +561,6 @@ def image_builder_fixture( "virt-type": "virtual-machine", "cores": "2", }, - log=False, ) yield image_builder_app_name From 725e81c3c0c5501ca26fa4e972a101f25a232212 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 13:38:59 +0200 Subject: [PATCH 03/20] fix(test): remove invalid log kwarg from juju.deploy Jubilant's Juju.deploy() does not accept a log argument; passing it raised TypeError during integration test fixture setup. --- tests/integration/helpers/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index a7774d74eb..a6af6522af 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -213,7 +213,6 @@ def deploy_github_runner_charm( base=base, config=default_config, constraints=constraints or DEFAULT_RUNNER_CONSTRAINTS, - log=False, **(deploy_kwargs or {}), ) From 1715b6634a053a19217180389adac27fb81b58c0 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 13:39:03 +0200 Subject: [PATCH 04/20] docs: document juju secret config options in changelog Add an entry for the new token-secret-id and openstack-clouds-yaml-secret-id options and note that the plaintext token and openstack-clouds-yaml options remain as compatibility fallbacks. --- docs/changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index 93bf02c8a4..3bbafde804 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. From ef2e8015d50b11b6ec4edfd616c589e7f51f968c Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 16:31:07 +0200 Subject: [PATCH 05/20] test(integration): skip test_charm_upgrade until latest/edge has secret-id options The latest/edge charm predates the token-secret-id and openstack-clouds-yaml-secret-id config options, so the upgrade test cannot deploy the legacy revision with the new fixtures. Re-enable once a release containing these options has been promoted to latest/edge. --- tests/integration/test_charm_upgrade.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index 1a594d8058..dd8a8bb482 100644 --- a/tests/integration/test_charm_upgrade.py +++ b/tests/integration/test_charm_upgrade.py @@ -32,6 +32,13 @@ pytestmark = pytest.mark.openstack +@pytest.mark.skip( + reason=( + "latest/edge charm predates token-secret-id and openstack-clouds-yaml-secret-id " + "config options. Re-enable once a release containing these options has been " + "promoted to latest/edge." + ) +) def test_charm_upgrade( juju: jubilant.Juju, deployment_context: DeploymentContext, From e70587b1ee753230fd85a9d286f825713f978af8 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 17:53:09 +0200 Subject: [PATCH 06/20] fix(charm_state): mention token-secret-id in missing-auth error The error raised when neither a GitHub token nor App auth is configured only referenced the plaintext `token` option, which is misleading now that `token-secret-id` is a valid alternative source of the token. --- src/charm_state.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index baac4599a8..863788b0b2 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -207,7 +207,8 @@ def from_charm(cls, charm: CharmBase) -> "GithubConfig": # noqa: C901 ) 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( From cdf2060114c8957f7a22bbf92d1ca24c7552ac8d Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 17:54:32 +0200 Subject: [PATCH 07/20] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- charmcraft.yaml | 2 +- src/charm_state.py | 6 +++++- tests/integration/helpers/common.py | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 63fc32fdc4..19f3306349 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -148,7 +148,7 @@ config: 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 + For fine-grained token scopes, see https://charmhub.io/github-runner/docs/how-to-change-token. token-secret-id: type: string diff --git a/src/charm_state.py b/src/charm_state.py index 863788b0b2..954ee0b5ed 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -422,7 +422,11 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML ) 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( diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index a6af6522af..50414bfbfb 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -204,6 +204,8 @@ def deploy_github_runner_charm( if config: default_config.update(config) + if TOKEN_SECRET_ID_CONFIG_NAME in default_config: + default_config[TOKEN_CONFIG_NAME] = "" if OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME in default_config: default_config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" From 028b739c6e135ea3a1dce9eeb23d7d7a3ad241dc Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 19:04:21 +0200 Subject: [PATCH 08/20] docs(test): explain why plaintext fallback is not used in upgrade test skip Jubilant logs the config passed to deploy, so reverting to the plaintext token/openstack-clouds-yaml options to cover the upgrade path would leak the GitHub token into test logs. Record this in the skip reason. --- tests/integration/test_charm_upgrade.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index dd8a8bb482..08a6511328 100644 --- a/tests/integration/test_charm_upgrade.py +++ b/tests/integration/test_charm_upgrade.py @@ -35,8 +35,10 @@ @pytest.mark.skip( reason=( "latest/edge charm predates token-secret-id and openstack-clouds-yaml-secret-id " - "config options. Re-enable once a release containing these options has been " - "promoted to latest/edge." + "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( From 77067cbd65e3893b333c2d71616488a360fce677 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 19:06:11 +0200 Subject: [PATCH 09/20] fix(charm_state): tailor openstack clouds yaml parse error to source When the clouds.yaml comes from a Juju secret, a YAML parse or validation failure now blames `openstack-clouds-yaml-secret-id` instead of the plaintext config option, so operators look at the right place. --- src/charm_state.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/charm_state.py b/src/charm_state.py index 954ee0b5ed..b6db8d76ec 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -406,8 +406,10 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML 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") @@ -435,9 +437,9 @@ 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) + logger.error("Invalid %s config: %s.", source, exc) raise CharmConfigInvalidError( - f"Invalid {OPENSTACK_CLOUDS_YAML_CONFIG_NAME} config. Invalid yaml." + f"Invalid {source} config. Invalid yaml." ) from exc return openstack_clouds_yaml From 8de25cae84ba711f43b76ffd6d7204e45112d4e1 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 19:08:52 +0200 Subject: [PATCH 10/20] refactor(charm): collapse config-changed flush detection into a loop The per-option branching in _on_config_changed tripped the C901 complexity threshold after new secret-id options were added. Replace the repeated if-blocks with a table-driven loop over (config_key, stored_attr) pairs and apply black/isort fixes. --- src/charm.py | 68 +++++++++++------------------------ src/charm_state.py | 19 +++++----- tests/integration/conftest.py | 12 +++---- 3 files changed, 35 insertions(+), 64 deletions(-) diff --git a/src/charm.py b/src/charm.py index 04facbda44..32d20937fe 100755 --- a/src/charm.py +++ b/src/charm.py @@ -53,9 +53,9 @@ 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, - OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, TOKEN_CONFIG_NAME, TOKEN_SECRET_ID_CONFIG_NAME, CharmConfigInvalidError, @@ -86,6 +86,20 @@ 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. +_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__) @@ -356,53 +370,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(TOKEN_SECRET_ID_CONFIG_NAME) != self._stored.token_secret_id: - self._stored.token_secret_id = self.config.get(TOKEN_SECRET_ID_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.get(OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME) - != self._stored.openstack_clouds_yaml_secret_id - ): - self._stored.openstack_clouds_yaml_secret_id = self.config.get( - OPENSTACK_CLOUDS_YAML_SECRET_ID_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 b6db8d76ec..6fcdbc80d1 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -154,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. @@ -195,7 +196,9 @@ def from_charm(cls, charm: CharmBase) -> "GithubConfig": # noqa: C901 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 + 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" @@ -400,9 +403,9 @@ 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_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) ) @@ -412,7 +415,9 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML 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") + 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" @@ -438,9 +443,7 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML create_model_from_typeddict(OpenStackCloudsYAML)(**openstack_clouds_yaml) except (yaml.YAMLError, TypeError) as exc: logger.error("Invalid %s config: %s.", source, exc) - raise CharmConfigInvalidError( - f"Invalid {source} config. Invalid yaml." - ) from 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 d83da748d1..94f1127fe7 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -580,8 +580,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): @@ -594,8 +593,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", @@ -921,8 +919,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): @@ -936,8 +933,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( From 2bd7e7fd6f0b96f28511133bea473b3fc3e0ab88 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 21:07:44 +0200 Subject: [PATCH 11/20] refactor(test): thread openstack clouds yaml as helper parameter Mirror the token path: let deploy_github_runner_charm own the clouds secret creation through a dedicated `openstack_clouds_yaml` parameter instead of sniffing the plaintext value out of the caller's config dict and then clearing it. Removes the clear/update/clear dance and the lingering plaintext in the deploy config mapping. --- tests/integration/conftest.py | 3 +-- tests/integration/helpers/common.py | 19 ++++--------------- tests/integration/test_charm_upgrade.py | 3 +-- 3 files changed, 6 insertions(+), 19 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 94f1127fe7..ab902812a7 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, @@ -635,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), diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 50414bfbfb..e02eb25564 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -26,12 +26,10 @@ GITHUB_APP_CLIENT_ID_CONFIG_NAME, GITHUB_APP_INSTALLATION_ID_CONFIG_NAME, GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, - OPENSTACK_CLOUDS_YAML_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 @@ -121,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, @@ -137,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. @@ -186,28 +186,17 @@ def deploy_github_runner_charm( secret_names.append(token_secret_name) default_config[TOKEN_SECRET_ID_CONFIG_NAME] = str(token_secret_id) - clouds_yaml = None - if config: - clouds_yaml = cast(str | None, config.get(OPENSTACK_CLOUDS_YAML_CONFIG_NAME)) - - if clouds_yaml and not (config and config.get(OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME)): + 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": clouds_yaml}, + 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) - default_config[TOKEN_CONFIG_NAME] = "" - default_config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" - if config: default_config.update(config) - if TOKEN_SECRET_ID_CONFIG_NAME in default_config: - default_config[TOKEN_CONFIG_NAME] = "" - if OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME in default_config: - default_config[OPENSTACK_CLOUDS_YAML_CONFIG_NAME] = "" juju.deploy( charm_file, diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index 08a6511328..4960c56619 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, @@ -87,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", From 0b975971f99f32281aa7b7797c6fc2198aab962d Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 21:58:15 +0200 Subject: [PATCH 12/20] fix(charm_state): silence bandit on openstack secret-id constant The secret-id config constant holds a config option name, not a secret. Add the same `# nosec` marker used for the other `*-secret-id` and token constants so bandit stops flagging it as a hardcoded password. --- src/charm_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index 6fcdbc80d1..3b6c463b7a 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -52,7 +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" +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" From dbd8a6ac808b89f72d7018c59decbeeadab61c3b Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 22:03:35 +0200 Subject: [PATCH 13/20] fix(charm): track plaintext clouds yaml + sharpen secret error paths - Flush runners on plaintext openstack-clouds-yaml changes so the behavior matches the existing token/secret-id paths; operators get the same reconcile regardless of which config option they use. - When the YAML parsed from the Juju secret is invalid, name the secret in the error instead of pointing operators at the plaintext option. - Cover the SecretNotFoundError branches for both token-secret-id and openstack-clouds-yaml-secret-id with unit tests. --- src/charm.py | 5 +++++ src/charm_state.py | 10 ++++++++++ tests/unit/test_charm.py | 11 +++++++++++ tests/unit/test_charm_state.py | 36 ++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+) diff --git a/src/charm.py b/src/charm.py index 32d20937fe..71c5d73ec5 100755 --- a/src/charm.py +++ b/src/charm.py @@ -53,6 +53,7 @@ GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, + OPENSTACK_CLOUDS_YAML_CONFIG_NAME, OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, PATH_CONFIG_NAME, PLANNER_INTEGRATION_NAME, @@ -96,6 +97,7 @@ (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_CONFIG_NAME, "openstack_clouds_yaml"), (OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, "openstack_clouds_yaml_secret_id"), (ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, "allow_external_contributor"), ) @@ -231,6 +233,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=self.config[ + OPENSTACK_CLOUDS_YAML_CONFIG_NAME + ], # for detecting changes openstack_clouds_yaml_secret_id=self.config.get( OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME ), # for detecting changes diff --git a/src/charm_state.py b/src/charm_state.py index 3b6c463b7a..0342431fcf 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -442,6 +442,16 @@ 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: + if source == OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME: + logger.error( + "Invalid OpenStack clouds.yaml content in secret %s: %s.", + openstack_clouds_yaml_secret_id, + exc, + ) + raise CharmConfigInvalidError( + "Invalid OpenStack clouds.yaml content in secret " + f"{openstack_clouds_yaml_secret_id}. Invalid yaml." + ) from exc logger.error("Invalid %s config: %s.", source, exc) raise CharmConfigInvalidError(f"Invalid {source} config. Invalid yaml.") from exc diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 385e92775d..1b6197dffe 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -282,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_CONFIG_NAME, + "clouds: {old: {}}", + "clouds: {new: {}}", + id="OpenStack clouds yaml", + ), pytest.param( OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, "secret:clouds-old", @@ -337,6 +343,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_CONFIG_NAME, + f"clouds: {{auth: {secrets.token_hex(4)}}}", + id="OpenStack clouds yaml", + ), pytest.param( OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, f"secret:{secrets.token_hex(8)}", diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index 4b63c59813..a009c8bb53 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 @@ -154,6 +155,23 @@ def test_github_config_from_charm_token_secret_takes_precedence_over_plaintext() 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. @@ -450,6 +468,24 @@ def test_parse_openstack_clouds_config_secret_precedence(valid_yaml_config: str) 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. From d959a995bde71ebb0de8dca99ac0cd01b4187211 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 22:08:48 +0200 Subject: [PATCH 14/20] refactor(charm): don't track plaintext openstack-clouds-yaml in _stored Tracking the plaintext option for auto-flush would persist the OpenStack credentials to .unit-state.db on disk, which contradicts the direction of this PR (moving secrets into Juju secrets). Operators who want automatic flush on credential rotation should use openstack-clouds-yaml-secret-id. --- src/charm.py | 12 ++++++------ tests/unit/test_charm.py | 11 ----------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/charm.py b/src/charm.py index 71c5d73ec5..ff94d61e77 100755 --- a/src/charm.py +++ b/src/charm.py @@ -53,7 +53,6 @@ GITHUB_APP_PRIVATE_KEY_SECRET_ID_CONFIG_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, - OPENSTACK_CLOUDS_YAML_CONFIG_NAME, OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, PATH_CONFIG_NAME, PLANNER_INTEGRATION_NAME, @@ -88,7 +87,12 @@ 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. +# 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"), @@ -97,7 +101,6 @@ (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_CONFIG_NAME, "openstack_clouds_yaml"), (OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, "openstack_clouds_yaml_secret_id"), (ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, "allow_external_contributor"), ) @@ -233,9 +236,6 @@ 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=self.config[ - OPENSTACK_CLOUDS_YAML_CONFIG_NAME - ], # for detecting changes openstack_clouds_yaml_secret_id=self.config.get( OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME ), # for detecting changes diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1b6197dffe..385e92775d 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -282,12 +282,6 @@ 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_CONFIG_NAME, - "clouds: {old: {}}", - "clouds: {new: {}}", - id="OpenStack clouds yaml", - ), pytest.param( OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, "secret:clouds-old", @@ -343,11 +337,6 @@ 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_CONFIG_NAME, - f"clouds: {{auth: {secrets.token_hex(4)}}}", - id="OpenStack clouds yaml", - ), pytest.param( OPENSTACK_CLOUDS_YAML_SECRET_ID_CONFIG_NAME, f"secret:{secrets.token_hex(8)}", From 262747af5d8874f06e92182cd7e75bb3994a32d8 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Apr 2026 22:13:22 +0200 Subject: [PATCH 15/20] fix(charm_state): don't log decrypted clouds.yaml on parse error yaml.YAMLError and pydantic TypeError messages embed a snippet of the offending input. When the source is a Juju secret, that snippet is decrypted secret content that must not land in juju debug-log. Drop `%s exc` from the log call and use `raise ... from None` so the chained cause isn't printed by `logger.exception` in catch_charm_errors. The plaintext branch has the same leak shape but is pre-existing behavior; not changing it here. --- src/charm_state.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/charm_state.py b/src/charm_state.py index 0342431fcf..004b681190 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -443,15 +443,18 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML create_model_from_typeddict(OpenStackCloudsYAML)(**openstack_clouds_yaml) except (yaml.YAMLError, TypeError) as 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: %s.", + "Invalid OpenStack clouds.yaml content in secret %s.", openstack_clouds_yaml_secret_id, - exc, ) raise CharmConfigInvalidError( "Invalid OpenStack clouds.yaml content in secret " f"{openstack_clouds_yaml_secret_id}. Invalid yaml." - ) from exc + ) from None logger.error("Invalid %s config: %s.", source, exc) raise CharmConfigInvalidError(f"Invalid {source} config. Invalid yaml.") from exc From 5e80b6a64bec89be476c7244e9083727db718c90 Mon Sep 17 00:00:00 2001 From: yhaliaw Date: Mon, 20 Apr 2026 09:37:24 +0800 Subject: [PATCH 16/20] Enable the upgrade charm test --- tests/integration/test_charm_upgrade.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index 4960c56619..8fd92aac01 100644 --- a/tests/integration/test_charm_upgrade.py +++ b/tests/integration/test_charm_upgrade.py @@ -31,15 +31,6 @@ 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, From 507545571883af34cf4b90d22058bc89aedb9fb7 Mon Sep 17 00:00:00 2001 From: yhaliaw Date: Mon, 20 Apr 2026 10:27:58 +0800 Subject: [PATCH 17/20] Revert "Enable the upgrade charm test" This reverts commit 5e80b6a64bec89be476c7244e9083727db718c90. --- tests/integration/test_charm_upgrade.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index 8fd92aac01..4960c56619 100644 --- a/tests/integration/test_charm_upgrade.py +++ b/tests/integration/test_charm_upgrade.py @@ -31,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, From a517f9663886fca2f269eac03a12654f0530d7b3 Mon Sep 17 00:00:00 2001 From: yhaliaw Date: Mon, 20 Apr 2026 16:14:53 +0800 Subject: [PATCH 18/20] Remove extra code --- src/charm_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index 004b681190..9ecf851fd9 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -454,7 +454,7 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML 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 From b6ea872221ed62aa018ec4fc2c1624a5afacd59f Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 20 Apr 2026 11:30:41 +0200 Subject: [PATCH 19/20] test: skip fork path-change integration test Fork creation currently fails because the fine-grained PAT is scoped to the canonical org while the fork lands in a personal namespace. To be revisited once integration tests migrate to GitHub App auth. --- tests/integration/test_charm_fork_path_change.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration/test_charm_fork_path_change.py b/tests/integration/test_charm_fork_path_change.py index 29b185fa17..566dd1cfe6 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( From b3ca37607779a7afcbd965feb793e35da05cd6d8 Mon Sep 17 00:00:00 2001 From: yhaliaw Date: Tue, 21 Apr 2026 12:58:54 +0800 Subject: [PATCH 20/20] Add back from None for avoid lint --- src/charm_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index 9ecf851fd9..004b681190 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -454,7 +454,7 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML 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