mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-11 22:15:38 +02:00
Fix review issues: auth on log routes, LoadRepos removal, test coverage
- Add reqToken/reqRepoReader to GET log download endpoints for consistency
with the POST streaming endpoint
- Remove spurious LoadRepos call in DownloadActionsRunAllJobLogs; jobs are
already scoped to the repo by the query and Repo is never read
- Refactor reader.Close() in zip loop to use a closure with defer
- Update copyright year to 2026 on new services/actions/{cancel,log}.go
- Add TestAPIActionsListUserWorkflows and TestAPIActionsListRepoWorkflows
as standalone top-level tests (were dropped when breaking up the
orchestrator)
- Add idempotency assertion to TestAPIActionsApproveWorkflowRun: approving
an already-approved run returns 400
Co-Authored-By: Claude Sonnet <claude-sonnet-4-6@anthropic.com>
This commit is contained in:
parent
19d38065b0
commit
47ffd078dd
@ -1265,11 +1265,11 @@ func Routes() *web.Router {
|
||||
m.Post("/approve", reqToken(), reqRepoWriter(unit.TypeActions), repo.ApproveWorkflowRun)
|
||||
m.Group("/jobs", func() {
|
||||
m.Get("", repo.ListWorkflowRunJobs)
|
||||
m.Get("/{job_id}/logs", repo.GetWorkflowJobLogs)
|
||||
m.Get("/{job_id}/logs", reqToken(), reqRepoReader(unit.TypeActions), repo.GetWorkflowJobLogs)
|
||||
m.Post("/{job_id}/rerun", reqToken(), reqRepoWriter(unit.TypeActions), repo.RerunWorkflowJob)
|
||||
})
|
||||
m.Group("/logs", func() {
|
||||
m.Get("", repo.GetWorkflowRunLogs)
|
||||
m.Get("", reqToken(), reqRepoReader(unit.TypeActions), repo.GetWorkflowRunLogs)
|
||||
m.Post("", reqToken(), reqRepoReader(unit.TypeActions), repo.GetWorkflowRunLogsStream)
|
||||
})
|
||||
m.Get("/artifacts", repo.GetArtifactsOfRun)
|
||||
|
||||
@ -35,9 +35,6 @@ func DownloadActionsRunAllJobLogs(ctx *context.Base, ctxRepo *repo_model.Reposit
|
||||
if err != nil {
|
||||
return fmt.Errorf("GetLatestAttemptJobsByRepoAndRunID: %w", err)
|
||||
}
|
||||
if err = runJobs.LoadRepos(ctx); err != nil {
|
||||
return fmt.Errorf("LoadRepos: %w", err)
|
||||
}
|
||||
|
||||
if len(runJobs) == 0 {
|
||||
return util.NewNotExistErrorf("no jobs found for run %d", runID)
|
||||
@ -77,27 +74,27 @@ func DownloadActionsRunAllJobLogs(ctx *context.Base, ctxRepo *repo_model.Reposit
|
||||
continue
|
||||
}
|
||||
|
||||
reader, err := actions.OpenLogs(ctx, task.LogInStorage, task.LogFilename)
|
||||
if err != nil {
|
||||
return fmt.Errorf("OpenLogs for job %d: %w", job.ID, err)
|
||||
}
|
||||
|
||||
// Create file in zip with job name and task ID; sanitize to prevent Zip Slip
|
||||
safeJobName := strings.NewReplacer("/", "-", `\`, "-", "..", "__").Replace(job.Name)
|
||||
fileName := fmt.Sprintf("%s-%s-%d.log", safeWorkflowName, safeJobName, task.ID)
|
||||
zipFile, err := zipWriter.Create(fileName)
|
||||
if err != nil {
|
||||
reader.Close()
|
||||
return fmt.Errorf("Create zip file %s: %w", fileName, err)
|
||||
}
|
||||
|
||||
// Copy log content to zip file
|
||||
if _, err := io.Copy(zipFile, reader); err != nil {
|
||||
reader.Close()
|
||||
return fmt.Errorf("Copy logs for job %d: %w", job.ID, err)
|
||||
}
|
||||
if err := func() error {
|
||||
reader, err := actions.OpenLogs(ctx, task.LogInStorage, task.LogFilename)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer reader.Close()
|
||||
|
||||
reader.Close()
|
||||
zipFile, err := zipWriter.Create(fileName)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
_, err = io.Copy(zipFile, reader)
|
||||
return err
|
||||
}(); err != nil {
|
||||
return fmt.Errorf("job %d: %w", job.ID, err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
// Copyright 2025 The Gitea Authors. All rights reserved.
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package actions
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
// Copyright 2025 The Gitea Authors. All rights reserved.
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package actions
|
||||
|
||||
@ -358,6 +358,11 @@ jobs:
|
||||
assert.Equal(t, actions_model.StatusWaiting, job.Status)
|
||||
}
|
||||
|
||||
// Test approve already approved run (idempotency)
|
||||
req = NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/%d/approve", baseRepo.FullName(), run.ID)).
|
||||
AddTokenAuth(user2Token)
|
||||
MakeRequest(t, req, http.StatusBadRequest)
|
||||
|
||||
// Test approve non-existent run
|
||||
req = NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/actions/runs/999999/approve", baseRepo.FullName())).
|
||||
AddTokenAuth(user2Token)
|
||||
@ -436,6 +441,72 @@ func TestAPIActionsRerunWorkflowJob(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIActionsListUserWorkflows(t *testing.T) {
|
||||
defer prepareTestEnvActionsArtifacts(t)()
|
||||
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
session := loginUser(t, user.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser)
|
||||
|
||||
t.Run("Runs", func(t *testing.T) {
|
||||
req := NewRequest(t, "GET", "/api/v1/user/actions/runs").AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
var runs api.ActionWorkflowRunsResponse
|
||||
err := json.Unmarshal(resp.Body.Bytes(), &runs)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Positive(t, runs.TotalCount)
|
||||
assert.NotEmpty(t, runs.Entries)
|
||||
|
||||
for _, run := range runs.Entries {
|
||||
assert.NotEmpty(t, run.DisplayTitle, "display_title should be populated")
|
||||
assert.NotNil(t, run.Repository, "repository should be populated via batch loading")
|
||||
assert.NotEmpty(t, run.Repository.FullName, "repository full_name should be populated")
|
||||
assert.NotNil(t, run.TriggerActor, "trigger_actor should be populated via batch loading")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Jobs", func(t *testing.T) {
|
||||
req := NewRequest(t, "GET", "/api/v1/user/actions/jobs").AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
var jobs api.ActionWorkflowJobsResponse
|
||||
err := json.Unmarshal(resp.Body.Bytes(), &jobs)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Positive(t, jobs.TotalCount)
|
||||
assert.NotEmpty(t, jobs.Entries)
|
||||
|
||||
for _, job := range jobs.Entries {
|
||||
assert.NotEmpty(t, job.Name, "job name should be populated")
|
||||
assert.NotEmpty(t, job.HTMLURL, "html_url should be populated via batch-loaded repo")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIActionsListRepoWorkflows(t *testing.T) {
|
||||
defer prepareTestEnvActionsArtifacts(t)()
|
||||
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
|
||||
session := loginUser(t, user.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)
|
||||
|
||||
req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/runs", repo.FullName())).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
var runs api.ActionWorkflowRunsResponse
|
||||
err := json.Unmarshal(resp.Body.Bytes(), &runs)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Positive(t, runs.TotalCount)
|
||||
assert.NotEmpty(t, runs.Entries)
|
||||
|
||||
for _, run := range runs.Entries {
|
||||
assert.NotNil(t, run.Repository, "repository should be populated from ctx.Repo")
|
||||
assert.Equal(t, repo.FullName(), run.Repository.FullName, "repository full_name should match")
|
||||
assert.NotNil(t, run.TriggerActor, "trigger_actor should be populated")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPIActionsGetWorkflowRunLogs(t *testing.T) {
|
||||
defer prepareTestEnvActionsArtifacts(t)()
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user