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

Actions Runner rest api #33873

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented Mar 13, 2025

Implements runner apis based on https://docs.github.com/en/rest/actions/self-hosted-runners?apiVersion=2022-11-28#list-self-hosted-runners-for-an-organization

  • Add Post endpoints for registration-token, google/go-github revealed this as problem

    • We should deprecate Get Endpoints, leaving them for compatibility
    • Get endpoint of admin has api path /admin/runners/registration-token that feels wrong, /admin/actions/runners/registration-token seems more consistent with user/org/repo api
  • Get Runner Api

  • List Runner Api

  • Delete Runner Api

  • Tests admin / user / org / repo level endpoints

Related to #33750 (implements point 1 and 2)
Via needs discovered in #32461, this runner api is needed to allow cleanup of runners that are deallocated without user interaction.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 13, 2025
@pull-request-size pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 13, 2025
@github-actions github-actions bot added modifies/translation modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Mar 13, 2025
@lunny lunny added this to the 1.24.0 milestone Mar 13, 2025
ctx.JSON(http.StatusOK, &res)
}

func GetRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) {
Copy link
Member

Choose a reason for hiding this comment

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

I think most of them need permission checks. The web routers will invoke getRunnersCtx to get the context but not here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The permission check that this runner is from the correct user/admin/org/repo is done by

if !runner.Editable(ownerID, repoID) {

Everything else is part of the api.go file.

Now I see the existing GetRegistrationToken can be called by read:organization the CreateRegistrationToken API needs write:organization .

Runner delete with read:organization scope fails, while listrunner + getrunner works

   api_org_runner_test.go:59: Response:  {"message":"token does not have at least one of required scope(s), required=[write:organization], token scope=read:organization","url":"http://localhost:3003/api/swagger"}

See here add permission org test, where I added some tests.

Both runner ownership and token scope is checked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants