actions / actions-runner-controller

Kubernetes controller for GitHub Actions self-hosted runners
Apache License 2.0
4.76k stars 1.12k forks source link

Allow reading AutoscalingRunnerSet githubConfigSecret from controller namespace #3714

Open lacarvalho91 opened 3 months ago

lacarvalho91 commented 3 months ago

There isn't an open issue for this because the issue templates state that features for actions.github.com controllers should be raised elsewhere, which I have done here but I haven't had a response yet. I also raised a discussion on this repo here.

The reason I am raising this PR is because in my setup the AutoscalingRunnerSets we provide are specific to users, so users can mount their own config maps and secrets. This is a problem because it means they can see the secret referenced in githubConfigSecret since it must currently be in the same namespace as the runner itself. We are configuring ARC to register at the organisation level so this means that users will have access to secrets for a GitHub App with "Self-hosted runners: Read and write" org level permissions, which we do not want.

To resolve this problem, this PR introduces a flag for the controller that, when enabled, tells the controller to read the secret referenced by githubConfigSecret from the same namespace as the controller. This is disabled by default.

I spent some time looking into the code and I couldn't actually see a technical reason for the runner secret to be in the same namespace as the runner, because the secret that gets read by the runner pod is a completely different secret - that one is the jitconfig secret that is created on the fly by the controller.

I have run this on a local kind cluster and verified that it works, from what I can see the e2e tests are only for the legacy controllers so I haven't added any coverage there. If there are e2e tests for the new controllers that I should add to please let me know.

MichaelHudgins commented 3 months ago

+1 to adding something like this. I have a similar usecase and not having the secret in the runner's namespace would be a good win towards securing the api secret.

lacarvalho91 commented 2 months ago

@mumoshu @toast-gear @rentziass

hey all, just wondering if theres anything that can be done to progress this 🙂