Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Self hosted runners in private repositories #108

Merged
merged 9 commits into from
Jan 18, 2025

Conversation

dscho
Copy link
Member

@dscho dscho commented Jan 17, 2025

This PR is part of the fruit of my labor regarding the embargoed Git for Windows v2.47.1(2): I had to build ARM64 artifacts in a private repository, and therefore I needed a way to manage the self-hosted runners without a public URL pointing to the post-deployment script.

This also addresses the problem that surfaced today: the app registration expired. Instead of renewing the registration, this PR switches to a federated identity (where any workflow running on the main branch of this here repository is authorized to deploy the Azure resources in question).

I still need to register the managed identity that is now required (a one-time cost) and verify that everything still works as expected even with a public repository ;-)

dscho added 9 commits January 17, 2025 19:01
To obtain the token that is necessary to register the runner, we need
some sort of credentials, and the way we do that is by using the
GitForWindowsHelper GitHub App's credentials. These have to be generated
from the App ID and the private key, therefore we require them as
secrets, which has previously not been documented.

Signed-off-by: Johannes Schindelin <[email protected]>
In private repositories, we can often get away with keeping a runner
around, at least as a deallocated VM for most of the time.

Signed-off-by: Johannes Schindelin <[email protected]>
Instead of storing stealable credentials in repository secrets, let's
create a managed identity instead and use federated credentials via
GitHub Actions' support for OpenID Connect.

This binds the authorization to GitHub workflows running in a specific
repository, and stealing the information won't enable an attacker to get
authorized to use the Azure subscription.

Note that this change _does_ require Client ID, Tenant ID and
Subscription ID to be stored separately as repository secrets (although
they do not _technically_ need to be kept secret, security is a game of
layers, so why give away this information?).

Also note that for some strange reason, the `contents: read` permission
seems to be lost when introducing a `permissions:` section. Therefore we
have to add it back explicitly, otherwise `actions/checkout` will fail
in a private repository.

Signed-off-by: Johannes Schindelin <[email protected]>
We do not need the custom Action: `Azure/login` logs in using the Azure
CLI, and subsequent `az` calls work just fine.

So let's drop the complexity of the custom Action and go back to using
`Azure/login` instead.

The only downside is that we now need to specify the subscription ID
even though `az login` would work without it. But that's a small price
to pay, as the `delete-self-hosted-runner` workflow _still_ uses the
`Azure/login` Action and has to have that information as a repository
secret anyway.

Signed-off-by: Johannes Schindelin <[email protected]>
When running this workflow in a private repository, providing a public
URL to the post-deployment script simply would not work.

It is not even possible to use the `GITHUB_TOKEN` to construct an
`Invoke-WebRequest` call: The `GITHUB_TOKEN` lacks the permission to
access the resource.

So let's just pass this post-deployment script as a parameter.

Since it is somewhat large-ish, weighing 14kB, let's compress it. And
since the compressed file is binary and cannot easily be passed around,
let's Base64-encode it. The result is still somewhat large (5.6kB) but
at least this works and still leaves some room for additional stuff to
be put into the post-deployment script.

Signed-off-by: Johannes Schindelin <[email protected]>
GitHub's documentation provides a stern warning against registering
self-hosted runners on public repositories:
https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#self-hosted-runner-security

To counter that, we specifically spin up ephemeral self-hosted runners
in `git-for-windows/git-for-windows-automation` and have automation to
prevent unauthorized people from trying to play games with our runners.

However, for testing in separate repositories, this strategy is utterly
inconvenient. And unnecessary, when running in a private repository
anyway. Except that we need to have a public URL for the post-deployment
script. So let's work around that by hard-coding the CI token into that
URL. This should be good enough, especially when we scrub the token from
the logs (manually, if necessary).

Signed-off-by: Johannes Schindelin <[email protected]>
Security is a game of layers, the less attack surface the better.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Jan 17, 2025

I still need to register the managed identity that is now required (a one-time cost) and verify that everything still works as expected even with a public repository ;-)

I have now done so and https://github.com/git-for-windows/git-for-windows-automation/actions/runs/12835971044/job/35796691295 demonstrates that this works as intended.

@dscho dscho marked this pull request as ready for review January 17, 2025 20:02
@dscho
Copy link
Member Author

dscho commented Jan 17, 2025

Most of the credit for figuring out how all of this federated credential business works goes to @mjcheetham! Couldn't have done it without you...

@dscho
Copy link
Member Author

dscho commented Jan 17, 2025

Oh, one thing I just noticed: I guess the commit message of 6269149 misses the point, the purpose of the patch is to register the runner in a private org or repository, it's not really about allowing the workflow to run in a private repository.

Copy link
Contributor

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! Nice changes.

I really do hope that the Hosted Windows ARM64 runners will be released soon, because it'd remove the need for all of this self-hosted runner wizardry...

I just learned that yesterday (Jan 16th), Linux ARM64 runners were released in Public Preview. I guess this means that Windows ARM64 runners also aren't too far out anymore... 👀

"firstFileName": "[variables('firstFileNameBreakString')[0]]",
"postDeploymentScriptArguments": "[concat('-GitHubActionsRunnerToken ', parameters('githubActionsRunnerToken'), ' -GithubActionsRunnerRegistrationUrl ', parameters('githubActionsRunnerRegistrationUrl'), ' -GithubActionsRunnerName ', parameters('virtualMachineName'), ' -StopService ', parameters('stopService'), ' -GitHubActionsRunnerPath ', parameters('githubActionsRunnerPath'))]"
"postDeploymentScriptArguments": "[concat('-GitHubActionsRunnerToken ', parameters('githubActionsRunnerToken'), ' -GithubActionsRunnerRegistrationUrl ', parameters('githubActionsRunnerRegistrationUrl'), ' -GithubActionsRunnerName ', parameters('virtualMachineName'), ' -Ephemeral ', parameters('ephemeral'), ' -StopService ', parameters('stopService'), ' -GitHubActionsRunnerPath ', parameters('githubActionsRunnerPath'))]",
"publicIpAddressName1": "[if(equals(parameters('publicIpAddressName1'), ''), 'dummy', parameters('publicIpAddressName1'))]",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the dummy name intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been quite a while since I actively worked on this, but I vaguely remember that I used the empty string at first and validation failed, so I used an obviously bogus name instead.

@dscho
Copy link
Member Author

dscho commented Jan 18, 2025

Amazing! Nice changes.

Thanks!

I really do hope that the Hosted Windows ARM64 runners will be released soon, because it'd remove the need for all of this self-hosted runner wizardry...

Me, too. I was really hopeful last year, but then fall came and went...

I just learned that yesterday (Jan 16th), Linux ARM64 runners were released in Public Preview. I guess this means that Windows ARM64 runners also aren't too far out anymore... 👀

Well, hope dies last.

The amount of effort, time and money projects had to invest to work around the lack of hosted Windows/ARM64 runners is staggering.

And even more staggering is the number of projects that decided simply not to support Windows/ARM64 "yet".

I consider that lack of hosted Windows/ARM64 runners the singularly most important contributing factor causing the Windows/ARM64 software landscape to lag behind.

@dscho dscho merged commit 3514912 into main Jan 18, 2025
1 check passed
@dscho dscho deleted the self-hosted-runners-in-private-repositories branch January 18, 2025 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants