test: prefer github app auth in integration fixtures#790
Merged
Conversation
cbartz
force-pushed
the
feat/integration-github-app-auth-fallback
branch
from
6fa227a to
0aea0b4
Compare
The second GitHub token was only needed for the fork-path-change tests that were removed in recent commits. Drop the now-dead --github-token-alt option, INTEGRATION_TOKEN_ALT env plumbing, and alt_token field.
The inline comments naming which env var fills each INTEGRATION_TEST_SECRET_ENV_VALUE_<N> slot could silently desync when the mapping is reshuffled in repo settings. Point to CONTRIBUTING.md instead, which is the authoritative source.
Drop the --github-token pytest option so the token can only flow through the INTEGRATION_TOKEN environment variable. Matches how GITHUB_APP_PRIVATE_KEY is already handled and keeps secrets out of the process command line.
Add the unsuffixed INTEGRATION_TEST_SECRET_ENV_NAME slot plus _11 and _12 so the GitHub App client id, installation id, and private key can be delivered to integration tests.
Mirror the e2e workflow and forward INTEGRATION_TEST_GITHUB_APP_CLIENT_ID into the charm integration test jobs so they can opt into GitHub App authentication when the other app credentials are available.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the integration-test fixture plumbing to prefer GitHub App authentication when the necessary credentials are available, while retaining PAT-based behavior as a fallback and removing fork/alt-token paths that are no longer needed.
Changes:
- Prefer GitHub App auth in shared integration
github_configfixtures (root charm and github-runner-manager) when app credentials are present; otherwise use token auth. - Remove the fork/path-change integration test and the associated alternate-token plumbing (
INTEGRATION_TOKEN_ALT,--github-token-alt, etc.). - Update CI workflows and tox passthrough to support the revised secret/env-var approach (including additional secret slots for app credentials).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Stops passing INTEGRATION_TOKEN_ALT into the root integration tox env. |
| tests/integration/test_e2e.py | Removes the module-local github_config override so the shared fixture behavior applies. |
| tests/integration/test_charm_fork_path_change.py | Deletes the fork/path-change integration test module. |
| tests/integration/conftest.py | Centralizes GitHub App vs PAT selection in the shared github_config fixture; removes fork-related fixtures/plumbing. |
| tests/conftest.py | Removes the --github-app-private-key CLI option (private key now env-only). |
| github-runner-manager/tox.ini | Stops passing INTEGRATION_TOKEN_ALT into the manager integration tox env. |
| github-runner-manager/tests/unit/test_integration_factories.py | Adds unit coverage for factories producing token-auth vs app-auth configs. |
| github-runner-manager/tests/integration/factories.py | Refactors GitHubConfig to support GitHub App auth and emits the appropriate auth block. |
| github-runner-manager/tests/integration/conftest.py | Prefers GitHub App auth when app credentials are available; token is now env-only. |
| github-runner-manager/tests/conftest.py | Removes old token/alt-token CLI options; adds GitHub App client/installation CLI options. |
| .github/workflows/test_github_runner_manager.yaml | Updates secret-slot wiring to carry GitHub App credentials and references CONTRIBUTING.md for mapping. |
| .github/workflows/integration_test.yaml | Removes the deleted fork test from the matrix and forwards --github-app-client-id consistently. |
Mark the token, private key, and OpenStack password fields with repr=False so assertion failures, traceback locals, or stray logs of these dataclasses cannot leak credentials in transformed (non-masked) forms.
Token and private key are env-only; path and app IDs come from pytest options. Prior docstring said "pytest options or environment" without distinguishing which inputs come from where.
Only --github-app-client-id and --github-app-installation-id fall back to env vars; --github-repository does not. Previous wording implied all three did.
Earlier revert commit moved two any_charm blocks to a multi-line form that the current black config rejects. Match main's inline style so lint passes.
cbartz
commented
Apr 21, 2026
cbartz
requested review from
florentianayuwono,
javierdelapuente,
weiiwang01,
yanksyoon and
yhaliaw
as code owners
yanksyoon
approved these changes
Apr 22, 2026
Member
yanksyoon
left a comment
yanksyoon
left a comment
There was a problem hiding this comment.
LGTM, thanks for catching the security related issues!
yhaliaw
approved these changes
Apr 22, 2026
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.
What this PR does
Teach the integration test fixtures to prefer GitHub App authentication when credentials are available, falling back to PAT otherwise. Along the way, clean up the surrounding plumbing:
github_configfixture (both root charm andgithub-runner-manager) returns aGitHubConfigpopulated with app credentials when all three ofGITHUB_APP_CLIENT_ID,GITHUB_APP_INSTALLATION_ID, andGITHUB_APP_PRIVATE_KEYare present. Otherwise token auth is used. The ad-hoc override insidetest_e2e.pyis gone.test_charm_fork_path_changeintegration test and its workflow entry, along with the now-deadINTEGRATION_TOKEN_ALT/--github-token-alt/alt_tokenplumbing that only existed for that test.INTEGRATION_TOKENandGITHUB_APP_PRIVATE_KEYare read from env vars only — no more--github-tokenpytest CLI flag — keeping secrets off the process command line.CONTRIBUTING.md, which is the authoritative place for the slot-to-var mapping. The inline comments rot silently when the mapping is reshuffled in repo settings.INTEGRATION_TEST_SECRET_ENV_VALUE,_11,_12) intest_github_runner_manager.yamlto carry the app credentials.integration_test.yamlnow forwards--github-app-client-idfromvars.INTEGRATION_TEST_GITHUB_APP_CLIENT_ID, matching the pattern already ine2e_test.yaml.Why we need it
PATs are bound to individual people, so any rotation or role change risks breaking CI. Using a GitHub App is cleaner: credentials belong to the project, not a person, and the same auth path is exercised that the charm uses in production.
Checklist