feat(github): add GitHub App authentication to charm layer#776
Merged
Conversation
Add charm config, state handling, secret-changed event, file permissions hardening, docs, and tests for GitHub App auth.
cbartz
requested review from
florentianayuwono,
javierdelapuente,
weiiwang01,
yanksyoon and
yhaliaw
as code owners
cbartz
commented
Apr 1, 2026
erinecon
reviewed
Apr 1, 2026
Contributor
erinecon
left a comment
erinecon
left a comment
There was a problem hiding this comment.
Thanks for updating the documentation!
The e2e test now uses GitHub App authentication when credentials are provided, falling back to PAT. This exercises the full charm wiring (Juju secret resolution, config propagation, runner registration) with App auth end-to-end. The app-level GitHub App auth integration test is removed as it is subsumed by the charm-level test.
charmcraft 4.2.0 (promoted to latest/stable on 2026-04-01) rejects the short-form platform syntax in charmcraft.yaml. Pin to latest/beta (4.0.1) as a workaround. Revert to latest/stable once the upstream bug is fixed.
charmcraft latest/stable is back to 4.0.1. Remove the workaround pin.
… feat/github-app-auth-charm
The GitHub App private key secret was created but never granted to the application, causing secret-get to fail during config-changed.
Co-authored-by: Erin Conley <erin.conley@canonical.com>
Add descriptions for all placeholder variables in the juju config command to help users find the required values.
cbartz
commented
Apr 8, 2026
Contributor
florentianayuwono
left a comment
florentianayuwono
left a comment
There was a problem hiding this comment.
thanks chris looks good!
yhaliaw
approved these changes
Apr 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds GitHub App authentication support to the GitHub runner charm (as a counterpart to the already-updated github-runner-manager), including charm config wiring, secret-based private key handling, updated tests, and refreshed Charmhub documentation.
Changes:
- Add GitHub App auth configuration (client ID, installation ID, private key via Juju secret) and wire it into charm state → application configuration generation.
- Reconcile/flush runners on secret rotation (
secret-changed) and expand unit/integration tests to cover App auth paths. - Update Charmhub docs/changelog and CI/test harness to support running with App auth credentials.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Pass-through env vars for integration/e2e GitHub App credentials |
| src/charm_state.py | Parse/validate GitHub App auth config + resolve private key from Juju secret; expose auth model |
| src/factories.py | Use derived state.charm_config.auth instead of PAT-only auth construction |
| src/charm.py | Track GitHub App config changes; add secret-changed handler to reconcile + flush |
| src/manager_service.py | Write config file with restrictive permissions; chown to runner-manager user/group |
| tests/unit/test_charm_state.py | Add unit tests for PAT vs App auth parsing/validation and secret resolution |
| tests/unit/test_factories.py | Add tests ensuring app config selects the correct auth model |
| tests/unit/test_charm.py | Extend config-change flush/no-flush coverage to new App auth config keys; add secret-changed tests |
| tests/unit/test_manager_service.py | Mock shutil.chown so unit tests don’t require the OS user |
| tests/unit/conftest.py | Extend complete_charm_state fixture with App auth fields |
| tests/unit/factories.py | Add default values for new GitHub App config keys in factories |
| tests/conftest.py | Add pytest CLI options for GitHub App credentials (with env var defaults) |
| tests/integration/helpers/common.py | Deploy helper optionally provisions GitHub App private-key secret and configures charm |
| tests/integration/test_e2e.py | Attempt to prefer App auth in e2e when credentials are available |
| tests/integration/conftest.py | Extend integration GitHubConfig with optional App auth fields and helper property |
| github-runner-manager/tests/integration/test_github_app_auth.py | Remove standalone manager App-auth integration test module |
| github-runner-manager/tests/conftest.py | Remove App-auth specific pytest CLI options (no longer needed for removed module) |
| docs/reference/token-scopes.md | Document App auth setup and required permissions; reorganize PAT scopes section |
| docs/how-to/change-token.md | Update guide to cover switching auth methods and key rotation via Juju secret |
| docs/reference/charm-architecture.md | Update GitHub API credential requirements to include App auth |
| docs/explanation/security.md | Prefer App auth; clarify permissions guidance |
| docs/changelog.md | Add entry announcing GitHub App auth support |
| charmcraft.yaml | Add new charm config options for GitHub App auth |
| CONTRIBUTING.md | Update sensitive-value passing guidance for CI |
| .vale/styles/config/vocabularies/local/accept.txt | Allow “GitHub App” in Vale vocab |
| .github/workflows/test_github_runner_manager.yaml | Remove test_github_app_auth from manager CI matrix |
| .github/workflows/integration_test.yaml | Pin charmcraft channel to latest/beta (with TODO) |
| .github/workflows/e2e_test.yaml | Pass GitHub App client ID into e2e test arguments |
yanksyoon
approved these changes
Apr 10, 2026
Member
yanksyoon
left a comment
yanksyoon
left a comment
There was a problem hiding this comment.
LGTM! Thanks! Also GARM supports GitHub app auth so we have all the ingredients for transition 💯
erinecon
approved these changes
Apr 10, 2026
Contributor
erinecon
left a comment
erinecon
left a comment
There was a problem hiding this comment.
Documentation changes LGTM -- thanks!
The charmcraft 4.2.0 regression has been fixed, so the temporary pin to latest/beta is no longer needed.
Charmcraft config options don't require defaults. The empty-string and zero defaults obscured whether the user had actually provided the GitHub App fields. Without defaults, unset config returns None which the validation logic already handles correctly.
The other two GitHub App env vars were passed through but this one was missing, causing tox to silently drop it and fall back to PAT.
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.
Overview
Add support for GitHub App authentication to the charm.
Follow-up of #775
Rationale
The application already supports it, now the charm needs to support it as well.
Checklist
urgent,trivial,complex).github-runner-manager/pyproject.toml.