search

CloudSnorkel / cdk-github-runners

CDK constructs for self-hosted GitHub Actions runners
https://constructs.dev/packages/@cloudsnorkel/cdk-github-runners/
Apache License 2.0
320 stars 37 forks source link

Use JIT runners #452

Open kichik opened 1 year ago

kichik commented 1 year ago

https://docs.github.com/en/rest/actions/self-hosted-runners?apiVersion=2022-11-28#create-configuration-for-a-just-in-time-runner-for-an-organization

https://github.blog/changelog/2023-06-02-github-actions-just-in-time-self-hosted-runners/

grochoge commented 1 year ago

Will this be optional to continue supporting older versions of GHES? (I initially thought it wasn't in GHES at all yet since it usually takes a while but I do see it when changing the docs to 3.10)

kichik commented 1 year ago

Will this be optional to continue supporting older versions of GHES? (I initially thought it wasn't in GHES at all yet since it usually takes a while but I do see it when changing the docs to 3.10)

I mostly want this for the ability to start runners with just one parameter. It will simplify a lot of our code. I don't think the security aspect affects too much as we have already separated the token retrieval away from the runner itself. The token is retrieved using a Lambda function ahead of time and passed on to the runner. We won't be able to remove any permissions from the runners with JIT runners (because they don't have any in the first place).

That's why making this feature optional would defeat its purpose. If it's optional, we still need all the extra code to setup a runner and more. By far the worst offender is in ec2.ts. Every {} is a parameter we have to pass in. If we just had one parameter, the code would be so much easier to understand.

@grochoge would you be able to confirm the API is actually working on 3.10?

grochoge commented 1 year ago

@kichik Unfortunately not, which is part of the reason I'm asking (being on an older version ATM). I suspect a lot of GHES instances are not on 3.10 yet; older versions don't go out of support until 2024-06-29.

So it would be good to add a note that this can only possibly work on 3.10 and above when this is merged.

Just looking at different options at the moment so I'm not sure the compatibility of the current version.

pharindoko commented 1 year ago

I can confirm that just in time runners are mentioned first in the changelig of version 3.10

https://docs.github.com/en/enterprise-server@3.10/actions/security-guides/security-hardening-for-github-actions#using-just-in-time-runners

And yes I assume as well that it might take some time until all ghes are on this version.

I'm quite sure that it's easy to identify the version via api call, but it definitely adds additional complexity to offer and determine both, jit and token-based runners dependent on the ghes version.

isker commented 7 months ago

@kichik how would you plan to handle the cdkghr:started: labels with --jitconfig? Seeing as the actual start time baked into those labels has to be even more jit 🌞 than the jit config you get from the github API?

kichik commented 7 months ago

@kichik how would you plan to handle the cdkghr:started: labels with --jitconfig? Seeing as the actual start time baked into those labels has to be even more jit 🌞 than the jit config you get from the github API?

That's a good point. It may have to go inside the retry loop. It's probably not a bad idea anyway, since even GitHub API can sometimes randomly fail or become unavailable.