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

Artifacts download api for artifact actions v4 #33510

Merged
merged 47 commits into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
32b6aef
Artifacts download api for artifact actions v4
ChristopherHX Feb 5, 2025
f7a1dca
Update swagger docs
ChristopherHX Feb 5, 2025
3f0cafd
fix swagger
ChristopherHX Feb 5, 2025
ef5b069
format code
ChristopherHX Feb 5, 2025
26514c9
use MakeAbsoluteURL
ChristopherHX Feb 5, 2025
e7308ac
fix lint? swagger command defect?
ChristopherHX Feb 5, 2025
e129fe8
revert encoding /
ChristopherHX Feb 5, 2025
d5520bd
302 => http.StatusFound
ChristopherHX Feb 5, 2025
d3dbfcb
extract getArtifactByID into a helper function
ChristopherHX Feb 5, 2025
d50358f
move old and new logic to modules
ChristopherHX Feb 5, 2025
bbb7017
fixup
ChristopherHX Feb 5, 2025
fcb8d55
intial tests
ChristopherHX Feb 5, 2025
db619c3
use correct json lib
ChristopherHX Feb 5, 2025
f4b0811
use http.ServeContent
ChristopherHX Feb 5, 2025
c6f1557
fix tests
ChristopherHX Feb 5, 2025
dfae484
use MakeAbsoluteURL
ChristopherHX Feb 5, 2025
ae2202d
add copyright header
ChristopherHX Feb 5, 2025
b4c318c
more tests fix old type from my side
ChristopherHX Feb 5, 2025
5e3c79d
fmt code
ChristopherHX Feb 5, 2025
b1b0ef3
add DeleteArtifact api
ChristopherHX Feb 6, 2025
e1ce404
Merge branch 'main' of https://github.com/go-gitea/gitea into artifac…
ChristopherHX Feb 9, 2025
59cf964
fix some swagger docu issues
ChristopherHX Feb 9, 2025
4721100
use repository apiurl method
ChristopherHX Feb 9, 2025
7a35443
remove unused ctx
ChristopherHX Feb 10, 2025
e137972
Merge branch 'main' into artifacts-download-api
ChristopherHX Feb 10, 2025
44bde87
fix merge
ChristopherHX Feb 10, 2025
a18c8dd
fix private repository artifact download redirect
ChristopherHX Feb 10, 2025
f8ce2f6
fmt
ChristopherHX Feb 10, 2025
5b76dbe
fix import
ChristopherHX Feb 10, 2025
6009e63
Merge branch 'main' into artifacts-download-api
silverwind Feb 11, 2025
8485044
add download test for private repo and remove the token to the redirect
ChristopherHX Feb 11, 2025
f58f33d
Merge branch 'main' into artifacts-download-api
wxiaoguang Feb 13, 2025
d56cebf
refactor path parm & error handling
wxiaoguang Feb 15, 2025
ee37003
fix type casting
wxiaoguang Feb 15, 2025
5d7bf81
refactor getArtifactByPathParam
wxiaoguang Feb 15, 2025
3d1b156
refactor build endpoint
wxiaoguang Feb 15, 2025
e6629fc
remove repoAssignment for raw endpoint
wxiaoguang Feb 15, 2025
7861808
fix test
wxiaoguang Feb 15, 2025
8e48777
fix comment
wxiaoguang Feb 15, 2025
421b7b4
remove TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerMis…
ChristopherHX Feb 15, 2025
a00f7f8
Update test comments and names to reflect only repo check
ChristopherHX Feb 15, 2025
156f0db
use unix timestamp in sig
ChristopherHX Feb 15, 2025
3df911a
fix comment
ChristopherHX Feb 15, 2025
4c1399d
revert owner_id changes to reflect only repoid check
ChristopherHX Feb 15, 2025
a0ed53d
Update routers/api/v1/repo/action.go
wxiaoguang Feb 15, 2025
12c627a
Merge branch 'main' into artifacts-download-api
wxiaoguang Feb 15, 2025
f80f4d1
Merge branch 'main' into artifacts-download-api
GiteaBot Feb 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/migrate_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func migrateActionsLog(ctx context.Context, dstStorage storage.ObjectStorage) er

func migrateActionsArtifacts(ctx context.Context, dstStorage storage.ObjectStorage) error {
return db.Iterate(ctx, nil, func(ctx context.Context, artifact *actions_model.ActionArtifact) error {
if artifact.Status == int64(actions_model.ArtifactStatusExpired) {
if artifact.Status == actions_model.ArtifactStatusExpired {
return nil
}

Expand Down
23 changes: 14 additions & 9 deletions models/actions/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type ActionArtifact struct {
ContentEncoding string // The content encoding of the artifact
ArtifactPath string `xorm:"index unique(runid_name_path)"` // The path to the artifact when runner uploads it
ArtifactName string `xorm:"index unique(runid_name_path)"` // The name of the artifact when runner uploads it
Status int64 `xorm:"index"` // The status of the artifact, uploading, expired or need-delete
Status ArtifactStatus `xorm:"index"` // The status of the artifact, uploading, expired or need-delete
CreatedUnix timeutil.TimeStamp `xorm:"created"`
UpdatedUnix timeutil.TimeStamp `xorm:"updated index"`
ExpiredUnix timeutil.TimeStamp `xorm:"index"` // The time when the artifact will be expired
Expand All @@ -68,7 +68,7 @@ func CreateArtifact(ctx context.Context, t *ActionTask, artifactName, artifactPa
RepoID: t.RepoID,
OwnerID: t.OwnerID,
CommitSHA: t.CommitSHA,
Status: int64(ArtifactStatusUploadPending),
Status: ArtifactStatusUploadPending,
ExpiredUnix: timeutil.TimeStamp(time.Now().Unix() + timeutil.Day*expiredDays),
}
if _, err := db.GetEngine(ctx).Insert(artifact); err != nil {
Expand Down Expand Up @@ -108,10 +108,11 @@ func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) erro

type FindArtifactsOptions struct {
db.ListOptions
RepoID int64
RunID int64
ArtifactName string
Status int
RepoID int64
RunID int64
ArtifactName string
Status int
FinalizedArtifactsV4 bool
}

func (opts FindArtifactsOptions) ToOrders() string {
Expand All @@ -134,6 +135,10 @@ func (opts FindArtifactsOptions) ToConds() builder.Cond {
if opts.Status > 0 {
cond = cond.And(builder.Eq{"status": opts.Status})
}
if opts.FinalizedArtifactsV4 {
cond = cond.And(builder.Eq{"status": ArtifactStatusUploadConfirmed}.Or(builder.Eq{"status": ArtifactStatusExpired}))
cond = cond.And(builder.Eq{"content_encoding": "application/zip"})
}

return cond
}
Expand Down Expand Up @@ -172,18 +177,18 @@ func ListPendingDeleteArtifacts(ctx context.Context, limit int) ([]*ActionArtifa

// SetArtifactExpired sets an artifact to expired
func SetArtifactExpired(ctx context.Context, artifactID int64) error {
_, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusExpired)})
_, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusExpired})
return err
}

// SetArtifactNeedDelete sets an artifact to need-delete, cron job will delete it
func SetArtifactNeedDelete(ctx context.Context, runID int64, name string) error {
_, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusPendingDeletion)})
_, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusPendingDeletion})
return err
}

// SetArtifactDeleted sets an artifact to deleted
func SetArtifactDeleted(ctx context.Context, artifactID int64) error {
_, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusDeleted)})
_, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusDeleted})
return err
}
18 changes: 18 additions & 0 deletions models/fixtures/action_artifact.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,21 @@
created_unix: 1730330775
updated_unix: 1730330775
expired_unix: 1738106775

-
id: 23
run_id: 793
runner_id: 1
repo_id: 2
owner_id: 2
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
storage_path: "27/5/1730330775594233150.chunk"
file_size: 1024
file_compressed_size: 1024
content_encoding: "application/zip"
artifact_path: "artifact-v4-download.zip"
artifact_name: "artifact-v4-download"
status: 2
created_unix: 1730330775
updated_unix: 1730330775
expired_unix: 1738106775
48 changes: 48 additions & 0 deletions modules/actions/artifacts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package actions

import (
"net/http"

actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/services/context"
)

// Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend
// The v4 backend ensures ContentEncoding is set to "application/zip", which is not the case for the old backend
func IsArtifactV4(art *actions_model.ActionArtifact) bool {
return art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip"
}

func DownloadArtifactV4ServeDirectOnly(ctx *context.Base, art *actions_model.ActionArtifact) (bool, error) {
if setting.Actions.ArtifactStorage.ServeDirect() {
u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this err should be returned if it's not nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, currently the error return value seems to be accidentally not returned.

But I want to make sure we get the expected behavior here, therefore I want some discussion about this before making changes

If we just return the error here (without changing the callers), this error may get returned by the api instead of triggering the fallback ( tunneling blob via gitea ).

What do we want to do with this error?

  • Return it from the api and abort the request? (needs changes)
  • Try the fallback and fail if the fallback doesn't work? (current behavior)
    • Log the error from the caller and continue with the fallback? if the boolean returns false (...and return the error from api if it returns true) (needs changes)
    • Otherwise Remove the error return value from the function? (needs changes)
    • Most ServeDirect() paths I saw in gitea dropped the error and only used it to detect if the url has been returned without error e.g. search for text ServeDirect in this repo

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine to keep as other behaviors, no need to introduce too many changes at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. I think it's OK to keep the current behavior. Maybe we can add some logs for this error later to help us debug cases where SERVE_DIRECT doesn't work.

if u != nil && err == nil {
ctx.Redirect(u.String(), http.StatusFound)
return true, nil
}
}
return false, nil
}

func DownloadArtifactV4Fallback(ctx *context.Base, art *actions_model.ActionArtifact) error {
f, err := storage.ActionsArtifacts.Open(art.StoragePath)
if err != nil {
return err
}
defer f.Close()
http.ServeContent(ctx.Resp, ctx.Req, art.ArtifactName+".zip", art.CreatedUnix.AsLocalTime(), f)
return nil
}

func DownloadArtifactV4(ctx *context.Base, art *actions_model.ActionArtifact) error {
ok, err := DownloadArtifactV4ServeDirectOnly(ctx, art)
if ok || err != nil {
return err
}
return DownloadArtifactV4Fallback(ctx, art)
}
31 changes: 31 additions & 0 deletions modules/structs/repo_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,34 @@ type ActionWorkflowResponse struct {
Workflows []*ActionWorkflow `json:"workflows"`
TotalCount int64 `json:"total_count"`
}

// ActionArtifact represents a ActionArtifact
type ActionArtifact struct {
ID int64 `json:"id"`
Name string `json:"name"`
SizeInBytes int64 `json:"size_in_bytes"`
URL string `json:"url"`
ArchiveDownloadURL string `json:"archive_download_url"`
Expired bool `json:"expired"`
WorkflowRun *ActionWorkflowRun `json:"workflow_run"`

// swagger:strfmt date-time
CreatedAt time.Time `json:"created_at"`
// swagger:strfmt date-time
UpdatedAt time.Time `json:"updated_at"`
// swagger:strfmt date-time
ExpiresAt time.Time `json:"expires_at"`
}

// ActionWorkflowRun represents a WorkflowRun
type ActionWorkflowRun struct {
ID int64 `json:"id"`
RepositoryID int64 `json:"repository_id"`
HeadSha string `json:"head_sha"`
}

// ActionArtifactsResponse returns ActionArtifacts
type ActionArtifactsResponse struct {
Entries []*ActionArtifact `json:"artifacts"`
TotalCount int64 `json:"total_count"`
}
2 changes: 1 addition & 1 deletion routers/api/actions/artifacts_chunks.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st
}

artifact.StoragePath = storagePath
artifact.Status = int64(actions.ArtifactStatusUploadConfirmed)
artifact.Status = actions.ArtifactStatusUploadConfirmed
if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil {
return fmt.Errorf("update artifact error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion routers/api/actions/artifactsv4.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ package actions
// 1.3. Continue Upload Zip Content to Blobstorage (unauthenticated request), repeat until everything is uploaded
// PUT: http://localhost:3000/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact?sig=mO7y35r4GyjN7fwg0DTv3-Fv1NDXD84KLEgLpoPOtDI=&expires=2024-01-23+21%3A48%3A37.20833956+%2B0100+CET&artifactName=test&taskID=75&comp=appendBlock
// 1.4. BlockList xml payload to Blobstorage (unauthenticated request)
// Files of about 800MB are parallel in parallel and / or out of order, this file is needed to enshure the correct order
// Files of about 800MB are parallel in parallel and / or out of order, this file is needed to ensure the correct order
// PUT: http://localhost:3000/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact?sig=mO7y35r4GyjN7fwg0DTv3-Fv1NDXD84KLEgLpoPOtDI=&expires=2024-01-23+21%3A48%3A37.20833956+%2B0100+CET&artifactName=test&taskID=75&comp=blockList
// Request
// <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
Expand Down
11 changes: 11 additions & 0 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,13 @@ func Routes() *web.Router {
}, reqToken(), reqAdmin())
m.Group("/actions", func() {
m.Get("/tasks", repo.ListActionTasks)
m.Get("/runs/{run}/artifacts", repo.GetArtifactsOfRun)
m.Get("/artifacts", repo.GetArtifacts)
m.Group("/artifacts/{artifact_id}", func() {
m.Get("", repo.GetArtifact)
m.Delete("", reqRepoWriter(unit.TypeActions), repo.DeleteArtifact)
})
m.Get("/artifacts/{artifact_id}/zip", repo.DownloadArtifact)
}, reqRepoReader(unit.TypeActions), context.ReferencesGitRepo(true))
m.Group("/keys", func() {
m.Combo("").Get(repo.ListDeployKeys).
Expand Down Expand Up @@ -1401,6 +1408,10 @@ func Routes() *web.Router {
}, repoAssignment(), checkTokenPublicOnly())
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository))

// Artifacts direct download endpoint authenticates via signed url
// it is protected by the "sig" parameter (to help to access private repo), so no need to use other middlewares
m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw)

// Notifications (requires notifications scope)
m.Group("/repos", func() {
m.Group("/{username}/{reponame}", func() {
Expand Down
Loading
Loading