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

Add generate-jitconfig API for self-hosted runners #2801

Merged
merged 8 commits into from
Jun 19, 2023

Conversation

mkulke
Copy link
Contributor

@mkulke mkulke commented Jun 7, 2023

GitHub recently introduced just-in-time configuration for self-hosted runner, so runners do not need to be seeded with privileged tokens. This PR adds those endpoints for orgs + repos.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @mkulke.

github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners.go Outdated Show resolved Hide resolved
github/actions_runners.go Outdated Show resolved Hide resolved
@gmlewis
Copy link
Collaborator

gmlewis commented Jun 7, 2023

Please remember to run go generate ./... at the top-level of the repo and push (not force-push) the changes to this PR.
See CONTRIBUTING.md for more details.

mkulke and others added 3 commits June 7, 2023 14:29
@mkulke mkulke requested a review from gmlewis June 7, 2023 12:39
github/actions_runners.go Outdated Show resolved Hide resolved
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Jun 7, 2023
@gabriel-samfira
Copy link
Contributor

Hi @mkulke !

I know the docs don't mention enterprises (although the blog seems to). I would be surprised if this was missing for enterprises/%v/actions/runners/generate-jitconfig.

I think I can ask someone to test this out. But I think that can be added as a separate PR. I don't want to block this one 😄.

Signed-off-by: Magnus Kulke <[email protected]>
Signed-off-by: Magnus Kulke <[email protected]>
@mkulke mkulke requested a review from gmlewis June 8, 2023 06:19
@mkulke
Copy link
Contributor Author

mkulke commented Jun 8, 2023

Hi @mkulke !

I know the docs don't mention enterprises (although the blog seems to). I would be surprised if this was missing for enterprises/%v/actions/runners/generate-jitconfig.

I think I can ask someone to test this out. But I think that can be added as a separate PR. I don't want to block this one 😄.

yeah, I was also wondering about that and opted to limit it to what's documented atm.

@gabriel-samfira
Copy link
Contributor

yeah, I was also wondering about that and opted to limit it to what's documented atm.

Thanks for adding this!

github/actions_runners.go Outdated Show resolved Hide resolved
Signed-off-by: Magnus Kulke <[email protected]>
@mkulke mkulke requested a review from gmlewis June 8, 2023 14:12
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @mkulke !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #2801 (82526e3) into master (3efdd2c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2801   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         132      132           
  Lines       11650    11676   +26     
=======================================
+ Hits        11424    11450   +26     
  Misses        154      154           
  Partials       72       72           
Impacted Files Coverage Δ
github/messages.go 100.00% <ø> (ø)
github/actions_runners.go 100.00% <100.00%> (ø)
github/event.go 100.00% <100.00%> (ø)

Copy link
Contributor

@valbeat valbeat left a comment

Choose a reason for hiding this comment

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

LGTM!

@gmlewis
Copy link
Collaborator

gmlewis commented Jun 19, 2023

Thank you, @valbeat !
Merging.

@gmlewis gmlewis merged commit 4f74d0c into google:master Jun 19, 2023
@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Jun 19, 2023
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.

4 participants