feat(github): add GitHub App authentication for self-hosted runner registration - #860
Open
adrianriobo wants to merge 1 commit into
Open
feat(github): add GitHub App authentication for self-hosted runner registration#860adrianriobo wants to merge 1 commit into
adrianriobo wants to merge 1 commit into
Conversation
|
Important Review skippedToo many files! This PR contains 205 files, which is 105 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (205)
You can disable this status message by setting the 📝 WalkthroughWalkthroughChangesGitHub Runner Authentication
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SetupRunner
participant PulumiGitHub
participant GitHub
participant DeploymentTarget
CLI->>DeploymentTarget: Provide GitHub runner arguments
DeploymentTarget->>SetupRunner: Initialize runner
SetupRunner->>PulumiGitHub: Create App provider
PulumiGitHub->>GitHub: Request registration token
GitHub-->>SetupRunner: Return registration token
SetupRunner-->>DeploymentTarget: Store token
DeploymentTarget->>DeploymentTarget: Generate runner userdata
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
oci/Containerfile (1)
14-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the vendored
pulumi-githubSDK
go.modandvendor/modules.txtstill referencegithub.com/pulumi/pulumi-github/sdk/v6, but the correspondingvendor/github.com/pulumi/pulumi-github/sources are missing. Regeneratevendor/and commit those files somake buildcan resolve imports under-mod=vendor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oci/Containerfile` around lines 14 - 19, Regenerate the Go vendor directory so the pulumi-github SDK referenced by go.mod and vendor/modules.txt is restored under vendor/github.com/pulumi/pulumi-github/, including all required source files and metadata. Commit the regenerated vendor contents and keep the existing make build flow unchanged.Sources: Linters/SAST tools, Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/self-hosted-runner.md`:
- Line 16: Update the authentication precedence description in the self-hosted
runner documentation to match GithubRunnerArgs: select GitHub App authentication
first, then the explicit --ghactions-runner-token value, and generate a token
from GITHUB_TOKEN only when no explicit token is supplied.
- Around line 24-25: Update the prerequisites in the self-hosted runner
documentation to describe repository-level GitHub App permissions and
installation only. Remove the organization-level permission and runner wording
from the entries associated with the --ghactions-runner-repo setup, leaving the
repository-scoped requirements unchanged.
---
Outside diff comments:
In `@oci/Containerfile`:
- Around line 14-19: Regenerate the Go vendor directory so the pulumi-github SDK
referenced by go.mod and vendor/modules.txt is restored under
vendor/github.com/pulumi/pulumi-github/, including all required source files and
metadata. Commit the regenerated vendor contents and keep the existing make
build flow unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 97347c14-74ad-4aab-8ac6-a8356f35750a
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
cmd/mapt/cmd/params/params.godocs/self-hosted-runner.mdgo.modoci/Containerfilepkg/integrations/github/api.gopkg/integrations/github/ghrunner.gopkg/integrations/github/types.gopkg/provider/aws/modules/mac/machine/setup/setup.gopkg/provider/azure/action/windows/windows.gopkg/provider/ibmcloud/action/ibm-power/ibm-power.gopkg/provider/ibmcloud/action/ibm-z/ibm-z.gopkg/target/host/fedora/fedora.gopkg/target/host/rhel/cloud-config.gopkg/target/host/windows-server/windows-server.govendor/modules.txt
|
@adrianriobo , it sounds like you're picturing that something or someone would run |
Collaborator
Author
|
My ideal world would be the controller who will check events for workloads on an specific type of runner, it will run automatically mapt create to create and join the runner. In a first phase if we define a set of static (long live) runners this will be executed either by an operator or a pipeline if we define them in a gitops approach |
|
Ok, thanks. gitops approach preferred. |
deekay2310
approved these changes
Jul 21, 2026
deekay2310
left a comment
deekay2310
left a comment
Contributor
There was a problem hiding this comment.
Looks good, just a handful of nits; nothing blocking.
| } | ||
| args.Token = result.Token | ||
| } else { | ||
| _, repo, _ := splitOwnerRepo(args.RepoURL) |
Contributor
There was a problem hiding this comment.
The error from splitOwnerRepo is discarded here. It's technically safe today because the same URL was validated at line 135, but if the code paths are ever refactored independently this becomes a silent failure.
Suggested change
| _, repo, _ := splitOwnerRepo(args.RepoURL) | |
| _, repo, err := splitOwnerRepo(args.RepoURL) | |
| if err != nil { | |
| return err | |
| } |
| AppAuth: pulgithub.ProviderAppAuthPtr(&pulgithub.ProviderAppAuthArgs{ | ||
| Id: pulumi.String(args.AppID), | ||
| InstallationId: pulumi.String(args.InstallationID), | ||
| PemFile: pulumi.String(string(pemBytes)), |
Contributor
There was a problem hiding this comment.
The provider SDK's NewProvider only auto-wraps Token as a secret (see provider.go:51), not AppAuth.PemFile. This means the RSA private key will appear in plaintext in Pulumi state.
Suggested change
| PemFile: pulumi.String(string(pemBytes)), | |
| PemFile: pulumi.ToSecret(pulumi.String(string(pemBytes))).(pulumi.StringInput), |
| "expires_at": "2024-07-12T19:01:48.478+05:30" | ||
| } | ||
| ``` | ||
| 1. Create a GitHub App with the `administration:write` permission on the target repository. |
Contributor
There was a problem hiding this comment.
For org-level runners (--ghactions-runner-org), the App needs the organization_self_hosted_runners:write permission instead of repo-level administration:write. Since the flags table and code already support the org path, consider something like:
Suggested change
| 1. Create a GitHub App with the `administration:write` permission on the target repository. | |
| 1. Create a GitHub App with the `administration:write` permission (for repository runners) or `organization_self_hosted_runners:write` permission (for organization runners). |
| To deploy a Windows runner on the Azure provider, we can use the following command: | ||
| ### Option B — Personal Access Token (PAT) | ||
|
|
||
| Set the `GITHUB_TOKEN` environment variable to a PAT with `repo` admin scope. `mapt` will call the GitHub API to generate a registration token automatically before deployment. |
Contributor
There was a problem hiding this comment.
This only covers the repo case. For org-level runners (--ghactions-runner-org), the PAT needs admin:org scope instead of repo admin. The error message in api.go:58 already tells the user this on failure, but it'd be better to document it upfront.
Suggested change
| Set the `GITHUB_TOKEN` environment variable to a PAT with `repo` admin scope. `mapt` will call the GitHub API to generate a registration token automatically before deployment. | |
| Set the `GITHUB_TOKEN` environment variable to a PAT with `repo` scope (for repository runners) or `admin:org` scope (for organization runners). `mapt` will call the GitHub API to generate a registration token automatically before deployment. |
…tion Adds GitHub App authentication for self-hosted runner registration and deregisters runners from the GitHub API at destroy time — replacing an earlier attempt that tried to call config.sh remove from inside the VM (config.sh remove always prompts for a fresh removal token; stored registration credentials cannot be reused). ## Registration (create) - Add GitHub App auth path: RS256 JWT generation via stdlib crypto (PKCS1 and PKCS8), exchanged for a short-lived installation access token. PAT (GITHUB_TOKEN) remains supported. - Add --ghactions-app-id, --ghactions-app-installation-id, --ghactions-app-private-key flags to all create commands. - Add --ghactions-runner-ephemeral flag to control runner lifetime. - Add org-level runner support via --ghactions-runner-org. - Add ppc64le/s390x fallback to the built layout when the vm.sh test step fails. ## Deregistration (destroy) - ExportRunnerOutputs persists runner name + org/repo URL as Pulumi stack outputs at create time. - Destroy reads stack outputs, calls GetManagementToken (App JWT or GITHUB_TOKEN) + TryDeregister (GitHub API list + delete by name) before tearing down the stack. Best-effort: failures are logged as warnings so destroy always proceeds. - GithubRunnerDeregisterArgs: returns minimal credentials for destroy without consuming a one-time registration token. - --ghactions-app-* flags exposed on all destroy commands. - Covered targets: IBM Z, IBM Power, AWS RHEL, AWS Fedora, AWS Windows, AWS Mac, Azure RHEL, Azure Windows. ## Fixes (review nits) - Handle splitOwnerRepo error in SetupRunner instead of discarding it. - Mark AppAuth.PemFile as a Pulumi secret so the RSA key does not appear in plaintext in state. - docs/self-hosted-runner.md: document org vs repo App permissions (administration:write vs organization_self_hosted_runners:write) and PAT scopes (repo vs admin:org). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Tested the GitHub App authentication path on IBM Power VS ( What was tested:
Smoke test runs (both passed):
Config used:
|
Contributor
|
Also tested the GitHub App authentication path on IBM Z ( What was tested:
Smoke test run (passed): Note: First run failed due to Config used:
|
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
3 participants
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.
Summary
pulumi-github/sdk/v6) is instantiated withAppAuthinside the deployment context and callsGetActionsRegistrationTokenat deploy time — no pre-generated tokens or JWT handling required--ghactions-runner-token(explicit token) andGITHUB_TOKEN(PAT auto-generate) both continue to work unchangedChanges
pkg/integrations/github/types.go— addAppID,InstallationID,PrivateKeyPathtoGithubRunnerArgspkg/integrations/github/ghrunner.go— addSetupRunner()usingpulumi-githubprovider withAppAuthpkg/integrations/github/api.go— refactor owner/repo parsing into shared helpercmd/mapt/cmd/params/params.go— add--ghactions-app-id,--ghactions-app-installation-id,--ghactions-app-private-keyflagsgithub.SetupRunner()before user-data generationgo.mod/go.sum— addgithub.com/pulumi/pulumi-github/sdk/v6 v6.14.0oci/Containerfile— installpulumi plugin resource github v6.14.0with renovate annotationdocs/self-hosted-runner.md— document all three auth options (App, PAT, manual token)Test plan
make build--ghactions-app-id/--ghactions-app-installation-id/--ghactions-app-private-keyGITHUB_TOKEN) still works--ghactions-runner-tokenpath still works🤖 Generated with Claude Code