Mirrors/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-10-31 09:31:53 +01:00
Code Issues Activity

Fix get / delete runner to use consistent http 404 and 500 status (#34480) (#34488)

Browse Source 
Backport #34480 by @ChristopherHX

* previously deleting an already deleted runner returned http 500
* previously any database error for the get endpoint was http 404 and
never 500

Co-authored-by: ChristopherHX <christopher.homberger@web.de>
This commit is contained in:
Giteabot 2025-05-17 00:59:29 +08:00 committed by GitHub
parent a0e272d95a
commit 21731c1370
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 35 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
routers/api/v1/shared/runners.go
View File

@ -67,6 +67,28 @@ func ListRunners(ctx *context.APIContext, ownerID, repoID int64) {
ctx.JSON(http.StatusOK, &res)
}
func getRunnerByID(ctx *context.APIContext, ownerID, repoID, runnerID int64) (*actions_model.ActionRunner, bool) {
if ownerID != 0 && repoID != 0 {
setting.PanicInDevOrTesting("ownerID and repoID should not be both set")
}
runner, err := actions_model.GetRunnerByID(ctx, runnerID)
if err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.APIErrorNotFound("Runner not found")
} else {
ctx.APIErrorInternal(err)
}
return nil, false
}
if !runner.EditableInContext(ownerID, repoID) {
ctx.APIErrorNotFound("No permission to access this runner")
return nil, false
}
return runner, true
}
// GetRunner get the runner for api route validated ownerID and repoID
// ownerID == 0 and repoID == 0 means any runner including global runners
// ownerID == 0 and repoID != 0 means any runner for the given repo
@ -77,13 +99,8 @@ func GetRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) {
if ownerID != 0 && repoID != 0 {
setting.PanicInDevOrTesting("ownerID and repoID should not be both set")
}
runner, err := actions_model.GetRunnerByID(ctx, runnerID)
if err != nil {
ctx.APIErrorNotFound(err)
return
}
if !runner.EditableInContext(ownerID, repoID) {
ctx.APIErrorNotFound("No permission to get this runner")
runner, ok := getRunnerByID(ctx, ownerID, repoID, runnerID)
if !ok {
return
}
ctx.JSON(http.StatusOK, convert.ToActionRunner(ctx, runner))
@ -96,20 +113,12 @@ func GetRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) {
// ownerID != 0 and repoID != 0 undefined behavior
// Access rights are checked at the API route level
func DeleteRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) {
if ownerID != 0 && repoID != 0 {
setting.PanicInDevOrTesting("ownerID and repoID should not be both set")
}
runner, err := actions_model.GetRunnerByID(ctx, runnerID)
if err != nil {
ctx.APIErrorInternal(err)
return
}
if !runner.EditableInContext(ownerID, repoID) {
ctx.APIErrorNotFound("No permission to delete this runner")
runner, ok := getRunnerByID(ctx, ownerID, repoID, runnerID)
if !ok {
return
}
err = actions_model.DeleteRunner(ctx, runner.ID)
err := actions_model.DeleteRunner(ctx, runner.ID)
if err != nil {
ctx.APIErrorInternal(err)
return

8
tests/integration/api_actions_runner_test.go
View File

@ -329,4 +329,12 @@ func testActionsRunnerRepo(t *testing.T) {
req := NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/repos/user2/repo1/actions/runners/%d", 34349)).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)
})
t.Run("DeleteAdminRunnerNotFoundUnknownID", func(t *testing.T) {
userUsername := "user2"
token := getUserToken(t, userUsername, auth_model.AccessTokenScopeWriteRepository)
// Verify delete a runner by unknown id is not found
req := NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/repos/user2/repo1/actions/runners/%d", 4384797347934)).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)
})
}