Add support for integration test with juju secrets#786
Merged
Conversation
Jubilant's Juju.deploy() does not accept a log argument; passing it raised TypeError during integration test fixture setup.
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.
…et-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.
cbartz
approved these changes
Apr 17, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Juju secret-backed configuration options so integration tests (and charm users) can supply the GitHub token and OpenStack clouds.yaml via Juju user secrets rather than plaintext config.
Changes:
- Added
token-secret-idandopenstack-clouds-yaml-secret-idconfig options (with secret content key parsing and precedence over plaintext). - Updated unit tests and integration deploy helper to create/grant secrets and use the new config options.
- Updated charm stored config-change detection and documented the user-facing change in the changelog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/charm_state.py |
Adds secret-ID config constants and resolves token/OpenStack YAML from Juju secrets with precedence and validation. |
src/charm.py |
Tracks new secret-id config values in StoredState and triggers runner flush when they change. |
charmcraft.yaml |
Exposes the new config options and clarifies plaintext options as compatibility fallbacks. |
docs/changelog.md |
Documents the new secret-backed configuration options and precedence behavior. |
tests/unit/test_charm_state.py |
Adds unit tests for secret-based token and OpenStack clouds parsing, including precedence and missing-field errors. |
tests/unit/test_charm.py |
Extends config-changed flush/no-flush parameterization to include the new secret-id options. |
tests/integration/helpers/common.py |
Updates integration deployment helper to create/grant secrets for token and OpenStack clouds and clear plaintext config by default. |
tests/integration/conftest.py |
Removes log=False from a deploy call used in integration fixtures. |
tests/integration/test_charm_upgrade.py |
Skips the upgrade test until latest/edge includes the newly added config options. |
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…t 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.
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.
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.
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.
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.
- 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.
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.
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.
This reverts commit 5e80b6a.
yhaliaw
requested review from
florentianayuwono,
javierdelapuente,
weiiwang01 and
yanksyoon
as code owners
yanksyoon
approved these changes
Apr 20, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applicable spec:
Overview
This pull request introduces new configuration options to allow sensitive credentials (the GitHub token and OpenStack
clouds.yaml) to be supplied securely via Juju secrets, rather than plaintext config. When these secret-based options are set, they take precedence over the previous plaintext options, which remain for compatibility. The implementation ensures that credential changes are handled securely and trigger proper runner flushes, and includes corresponding updates to documentation and tests.Secret-based credential support:
token-secret-idandopenstack-clouds-yaml-secret-idconfig options, allowing the GitHub token and OpenStackclouds.yamlto be provided via Juju secrets. These take precedence over the plaintexttokenandopenstack-clouds-yamloptions, which are now considered compatibility fallbacks. (charmcraft.yaml,docs/changelog.md, [1] [2] [3]GithubConfig.from_charmand OpenStack config parsing to retrieve and validate secrets, raising clear errors if secrets are missing or malformed. (src/charm_state.py, [1] [2] [3] [4]Runner lifecycle and config change handling:
src/charm.py, [1] [2] [3] [4]Testing and integration:
tests/integration/conftest.py,tests/integration/helpers/common.py, [1] [2] [3] [4]Documentation:
docs/changelog.md, docs/changelog.mdR5-R8)Rationale
Prevent leaking secrets in the integration test in the workflow.
Juju Events Changes
Module Changes
Library Changes
Checklist
urgent,trivial,complex).github-runner-manager/pyproject.toml.