From c41709daffddb0cdd7ff50916648677e8143105b Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 8 Dec 2022 20:45:23 +0800 Subject: [PATCH 01/38] feat: add upload-artifact http api --- models/actions/artifacts.go | 71 +++++++++++++++++ routers/api/actions/artifacts.go | 132 +++++++++++++++++++++++++++++++ routers/init.go | 2 + 3 files changed, 205 insertions(+) create mode 100644 models/actions/artifacts.go create mode 100644 routers/api/actions/artifacts.go diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go new file mode 100644 index 0000000000000..5a99b8c1ef793 --- /dev/null +++ b/models/actions/artifacts.go @@ -0,0 +1,71 @@ +package actions + +import ( + "io" + "io/fs" + "log" + "net" + "os" + "path/filepath" +) + +// ArtifactFile is a file that is stored in the artifact storage. +type ArtifactFile interface { + fs.File + io.Writer +} + +type MkdirFS interface { + MkdirAll(path string, perm fs.FileMode) error + Open(name string) (ArtifactFile, error) + OpenAtEnd(name string) (ArtifactFile, error) +} + +var _ MkdirFS = (*DiskMkdirFs)(nil) + +type DiskMkdirFs struct { + dir string +} + +func NewDiskMkdirFs(dir string) *DiskMkdirFs { + return &DiskMkdirFs{dir: dir} +} + +func (fsys DiskMkdirFs) MkdirAll(path string, perm fs.FileMode) error { + fpath := filepath.Join(fsys.dir, path) + return os.MkdirAll(fpath, perm) +} + +func (fsys DiskMkdirFs) Open(name string) (ArtifactFile, error) { + fpath := filepath.Join(fsys.dir, name) + dirpath := filepath.Dir(fpath) + os.MkdirAll(dirpath, os.ModePerm) + return os.OpenFile(fpath, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644) +} + +func (fsys DiskMkdirFs) OpenAtEnd(name string) (ArtifactFile, error) { + fpath := filepath.Join(fsys.dir, name) + file, err := os.OpenFile(fpath, os.O_CREATE|os.O_RDWR, 0644) + if err != nil { + return nil, err + } + _, err = file.Seek(0, os.SEEK_END) + if err != nil { + return nil, err + } + return file, nil +} + +// https://stackoverflow.com/a/37382208 +// Get preferred outbound ip of this machine +func GetOutboundIP() net.IP { + conn, err := net.Dial("udp", "8.8.8.8:80") + if err != nil { + log.Fatal(err) + } + defer conn.Close() + + localAddr := conn.LocalAddr().(*net.UDPAddr) + + return localAddr.IP +} diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go new file mode 100644 index 0000000000000..75eafa1a5b9f8 --- /dev/null +++ b/routers/api/actions/artifacts.go @@ -0,0 +1,132 @@ +package actions + +import ( + gocontext "context" + "fmt" + "io" + "net/http" + "net/url" + "strings" + + "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/web" +) + +func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { + m := web.NewRoute() + m.Use(withContexter(goctx)) + + routes := artifactRoutes{prefix: prefix, fs: actions.NewDiskMkdirFs("./artifacts-data")} + + m.Post("/_apis/pipelines/workflows/{runId}/artifacts", routes.getUploadArtifactURL) + m.Put("/_apis/pipelines/workflows/{runId}/artifacts/{artifactID}/upload", routes.uploadArtifact) + m.Patch("/_apis/pipelines/workflows/{runId}/artifacts", routes.patchArtifact) + m.Get("/_apis/pipelines/workflows/{runId}/artifacts", listJobArtifacts) + m.Get("/download/:container", listContainerArtifacts) + m.Get("/artifact/:path", downloadArtifact) + return m +} + +// PackageContexter initializes a package context for a request. +func withContexter(ctx gocontext.Context) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + ctx := context.Context{ + Resp: context.NewResponse(resp), + Data: map[string]interface{}{}, + } + defer ctx.Close() + + ctx.Req = context.WithContext(req, &ctx) + + next.ServeHTTP(ctx.Resp, ctx.Req) + }) + } +} + +type artifactRoutes struct { + prefix string + fs actions.MkdirFS +} + +func (ar artifactRoutes) openFile(fpath string, contentRange string) (actions.ArtifactFile, error) { + if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") { + return ar.fs.OpenAtEnd(fpath) + } + return ar.fs.Open(fpath) +} + +// getUploadArtifactURL generates a URL for uploading an artifact +func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { + jobID := ctx.Params("runId") + uploadURL := strings.TrimSuffix(setting.AppURL, "/") + ctx.Req.URL.Path + "/" + jobID + "/upload" + u, err := url.Parse(uploadURL) + if err != nil { + log.Error("Error parsing upload URL: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + // FIXME: fix localhost ?? act starts docker container to run action. + // It can't visit host network in container. + if strings.Contains(u.Host, "localhost") { + u.Host = strings.ReplaceAll(u.Host, "localhost", actions.GetOutboundIP().String()) + } + if strings.Contains(u.Host, "127.0.0.1") { + u.Host = strings.ReplaceAll(u.Host, "127.0.0.1", actions.GetOutboundIP().String()) + } + ctx.JSON(200, map[string]interface{}{ + "fileContainerResourceUrl": u.String(), + }) +} + +func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { + itemPath := ctx.Req.URL.Query().Get("itemPath") + runID := ctx.Params("runId") + + if ctx.Req.Header.Get("Content-Encoding") == "gzip" { + itemPath += ".gz" + } + filePath := fmt.Sprintf("%s/%s", runID, itemPath) + + file, err := ar.openFile(filePath, ctx.Req.Header.Get("Content-Range")) + if err != nil { + log.Error("Error opening file: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + defer file.Close() + + _, err = io.Copy(file, ctx.Req.Body) + if err != nil { + log.Error("Error copying body to file: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + ctx.JSON(200, map[string]string{ + "message": "success", + }) +} + +// TODO: why it is used? +func (ar artifactRoutes) patchArtifact(ctx *context.Context) { + ctx.JSON(200, map[string]string{ + "message": "success", + }) +} + +func listJobArtifacts(ctx *context.Context) { + +} + +func listContainerArtifacts(ctx *context.Context) { + +} + +func downloadArtifact(ctx *context.Context) { + +} diff --git a/routers/init.go b/routers/init.go index 2c26bb5b07671..692af6840fb2e 100644 --- a/routers/init.go +++ b/routers/init.go @@ -193,6 +193,8 @@ func NormalRoutes(ctx context.Context) *web.Route { if setting.Actions.Enabled { prefix := "/api/actions" r.Mount(prefix, actions_router.Routes(ctx, prefix)) + prefix = "/api/artifacts" + r.Mount(prefix, actions_router.ArtifactsRoutes(ctx, prefix)) } return r From 93cd14ba36d1be8d90f1c0bdb694bbea31bb0bc8 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 8 Dec 2022 23:27:20 +0800 Subject: [PATCH 02/38] feat: add action artifact table --- models/actions/artifacts.go | 57 ++++++++++++++++++++++++++++ routers/api/actions/artifacts.go | 65 ++++++++++++++++++++++++++++---- 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go index 5a99b8c1ef793..c4fa60d7070b9 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifacts.go @@ -1,12 +1,18 @@ package actions import ( + "context" + "fmt" "io" "io/fs" "log" "net" "os" "path/filepath" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" ) // ArtifactFile is a file that is stored in the artifact storage. @@ -69,3 +75,54 @@ func GetOutboundIP() net.IP { return localAddr.IP } + +func init() { + db.RegisterModel(new(ActionArtifact)) +} + +// ActionArtifact is a file that is stored in the artifact storage. +type ActionArtifact struct { + ID int64 + JobID int64 + RunnerID int64 `xorm:"index"` + RepoID int64 `xorm:"index"` + OwnerID int64 `xorm:"index"` + CommitSHA string `xorm:"index"` + FilePath string + FileSize int64 + ArtifactPath string + ArtifactName string + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated index"` +} + +// CreateArtifact creates a new artifact with task info +func CreateArtifact(ctx context.Context, t *ActionTask) (*ActionArtifact, error) { + artifact := &ActionArtifact{ + JobID: t.JobID, + RunnerID: t.RunnerID, + RepoID: t.RepoID, + OwnerID: t.OwnerID, + CommitSHA: t.CommitSHA, + } + _, err := db.GetEngine(ctx).Insert(artifact) + return artifact, err +} + +func GetArtifactByID(ctx context.Context, id int64) (*ActionArtifact, error) { + var art ActionArtifact + has, err := db.GetEngine(ctx).Where("id=?", id).Get(&art) + if err != nil { + return nil, err + } else if !has { + return nil, fmt.Errorf("task with id %d: %w", id, util.ErrNotExist) + } + + return &art, nil +} + +func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) error { + art.ID = id + _, err := db.GetEngine(ctx).ID(id).AllCols().Update(art) + return err +} diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 75eafa1a5b9f8..7c9719c178f14 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "strconv" "strings" "code.gitea.io/gitea/models/actions" @@ -52,17 +53,37 @@ type artifactRoutes struct { fs actions.MkdirFS } -func (ar artifactRoutes) openFile(fpath string, contentRange string) (actions.ArtifactFile, error) { +func (ar artifactRoutes) openFile(fpath string, contentRange string) (actions.ArtifactFile, bool, error) { if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") { - return ar.fs.OpenAtEnd(fpath) + f, err := ar.fs.OpenAtEnd(fpath) + return f, true, err } - return ar.fs.Open(fpath) + f, err := ar.fs.Open(fpath) + return f, false, err } // getUploadArtifactURL generates a URL for uploading an artifact func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { - jobID := ctx.Params("runId") - uploadURL := strings.TrimSuffix(setting.AppURL, "/") + ctx.Req.URL.Path + "/" + jobID + "/upload" + // get task + jobID := ctx.ParamsInt64("runId") + + task, err := actions.GetTaskByID(ctx, jobID) + if err != nil { + log.Error("Error getting task: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + artifact, err := actions.CreateArtifact(ctx, task) + if err != nil { + log.Error("Error creating artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + uploadURL := strings.TrimSuffix(setting.AppURL, "/") + + ctx.Req.URL.Path + "/" + strconv.FormatInt(artifact.ID, 10) + "/upload" + u, err := url.Parse(uploadURL) if err != nil { log.Error("Error parsing upload URL: %v", err) @@ -84,15 +105,26 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { } func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { + artifactID := ctx.ParamsInt64("artifactID") + + artifact, err := actions.GetArtifactByID(ctx, artifactID) + if err != nil { + log.Error("Error getting artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + itemPath := ctx.Req.URL.Query().Get("itemPath") runID := ctx.Params("runId") + artifactName := strings.Split(itemPath, "/")[0] if ctx.Req.Header.Get("Content-Encoding") == "gzip" { itemPath += ".gz" } - filePath := fmt.Sprintf("%s/%s", runID, itemPath) + filePath := fmt.Sprintf("%s/%d/%s", runID, artifactID, itemPath) - file, err := ar.openFile(filePath, ctx.Req.Header.Get("Content-Range")) + fSize := int64(0) + file, isChunked, err := ar.openFile(filePath, ctx.Req.Header.Get("Content-Range")) if err != nil { log.Error("Error opening file: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -100,12 +132,28 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { } defer file.Close() - _, err = io.Copy(file, ctx.Req.Body) + if isChunked { + // chunked means it is a continuation of a previous upload + fSize = artifact.FileSize + } + + n, err := io.Copy(file, ctx.Req.Body) if err != nil { log.Error("Error copying body to file: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return } + fSize += n + artifact.FilePath = filePath // path in storage + artifact.ArtifactName = artifactName + artifact.ArtifactPath = itemPath // path in container + artifact.FileSize = fSize + + if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { + log.Error("Error updating artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } ctx.JSON(200, map[string]string{ "message": "success", @@ -114,6 +162,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { // TODO: why it is used? func (ar artifactRoutes) patchArtifact(ctx *context.Context) { + ctx.JSON(200, map[string]string{ "message": "success", }) From 74f186a162b6058ef80c7259d421f397b3d30a65 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Wed, 14 Dec 2022 21:24:30 +0800 Subject: [PATCH 03/38] feat(actions-artifacts): finish download artifacts apis --- models/actions/artifacts.go | 8 ++ routers/api/actions/artifacts.go | 161 ++++++++++++++++++++++++------- 2 files changed, 136 insertions(+), 33 deletions(-) diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go index c4fa60d7070b9..c44f54ab51192 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifacts.go @@ -109,6 +109,7 @@ func CreateArtifact(ctx context.Context, t *ActionTask) (*ActionArtifact, error) return artifact, err } +// GetArtifactByID returns an artifact by id func GetArtifactByID(ctx context.Context, id int64) (*ActionArtifact, error) { var art ActionArtifact has, err := db.GetEngine(ctx).Where("id=?", id).Get(&art) @@ -121,8 +122,15 @@ func GetArtifactByID(ctx context.Context, id int64) (*ActionArtifact, error) { return &art, nil } +// UpdateArtifactByID updates an artifact by id func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) error { art.ID = id _, err := db.GetEngine(ctx).ID(id).AllCols().Update(art) return err } + +// ListArtifactByJobID returns all artifacts of a job +func ListArtifactByJobID(ctx context.Context, jobID int64) ([]*ActionArtifact, error) { + arts := make([]*ActionArtifact, 0, 10) + return arts, db.GetEngine(ctx).Where("job_id=?", jobID).Find(&arts) +} diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 7c9719c178f14..5f68afe5fd28b 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "path/filepath" "strconv" "strings" @@ -16,18 +17,24 @@ import ( "code.gitea.io/gitea/modules/web" ) +const artifactRouteBase = "/_apis/pipelines/workflows/{taskID}/artifacts" + func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { m := web.NewRoute() m.Use(withContexter(goctx)) - routes := artifactRoutes{prefix: prefix, fs: actions.NewDiskMkdirFs("./artifacts-data")} + r := artifactRoutes{prefix: prefix, fs: actions.NewDiskMkdirFs("./artifacts-data")} + + // retrieve, list and confirm artifacts + m.Post(artifactRouteBase, r.getUploadArtifactURL) + m.Get(artifactRouteBase, r.listArtifacts) + m.Patch(artifactRouteBase, r.patchArtifact) + + // handle container artifacts + m.Put(artifactRouteBase+"/{artifactID}/upload", r.uploadArtifact) + m.Get(artifactRouteBase+"/{artifactID}/path", r.getDownloadArtifactURL) + m.Get(artifactRouteBase+"/{artifactID}/download", r.downloadArtifact) - m.Post("/_apis/pipelines/workflows/{runId}/artifacts", routes.getUploadArtifactURL) - m.Put("/_apis/pipelines/workflows/{runId}/artifacts/{artifactID}/upload", routes.uploadArtifact) - m.Patch("/_apis/pipelines/workflows/{runId}/artifacts", routes.patchArtifact) - m.Get("/_apis/pipelines/workflows/{runId}/artifacts", listJobArtifacts) - m.Get("/download/:container", listContainerArtifacts) - m.Get("/artifact/:path", downloadArtifact) return m } @@ -62,12 +69,30 @@ func (ar artifactRoutes) openFile(fpath string, contentRange string) (actions.Ar return f, false, err } +func (ar artifactRoutes) buildArtifactUrl(taskID int64, artifactID int64, suffix string) (string, error) { + uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") + + strings.ReplaceAll(artifactRouteBase, "{taskID}", strconv.FormatInt(taskID, 10)) + + "/" + strconv.FormatInt(artifactID, 10) + "/" + suffix + u, err := url.Parse(uploadURL) + if err != nil { + return "", err + } + // FIXME: fix localhost ?? act starts docker container to run action. + // It can't visit host network in container. + if strings.Contains(u.Host, "localhost") { + u.Host = strings.ReplaceAll(u.Host, "localhost", actions.GetOutboundIP().String()) + } + if strings.Contains(u.Host, "127.0.0.1") { + u.Host = strings.ReplaceAll(u.Host, "127.0.0.1", actions.GetOutboundIP().String()) + } + return u.String(), nil +} + // getUploadArtifactURL generates a URL for uploading an artifact func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { // get task - jobID := ctx.ParamsInt64("runId") - - task, err := actions.GetTaskByID(ctx, jobID) + taskID := ctx.ParamsInt64("taskID") + task, err := actions.GetTaskByID(ctx, taskID) if err != nil { log.Error("Error getting task: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -80,27 +105,15 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - - uploadURL := strings.TrimSuffix(setting.AppURL, "/") + - ctx.Req.URL.Path + "/" + strconv.FormatInt(artifact.ID, 10) + "/upload" - - u, err := url.Parse(uploadURL) + url, err := ar.buildArtifactUrl(taskID, artifact.ID, "upload") if err != nil { log.Error("Error parsing upload URL: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return } - // FIXME: fix localhost ?? act starts docker container to run action. - // It can't visit host network in container. - if strings.Contains(u.Host, "localhost") { - u.Host = strings.ReplaceAll(u.Host, "localhost", actions.GetOutboundIP().String()) - } - if strings.Contains(u.Host, "127.0.0.1") { - u.Host = strings.ReplaceAll(u.Host, "127.0.0.1", actions.GetOutboundIP().String()) - } ctx.JSON(200, map[string]interface{}{ - "fileContainerResourceUrl": u.String(), + "fileContainerResourceUrl": url, }) } @@ -115,13 +128,13 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { } itemPath := ctx.Req.URL.Query().Get("itemPath") - runID := ctx.Params("runId") + taskID := ctx.Params("taskID") artifactName := strings.Split(itemPath, "/")[0] if ctx.Req.Header.Get("Content-Encoding") == "gzip" { itemPath += ".gz" } - filePath := fmt.Sprintf("%s/%d/%s", runID, artifactID, itemPath) + filePath := fmt.Sprintf("%s/%d/%s", taskID, artifactID, itemPath) fSize := int64(0) file, isChunked, err := ar.openFile(filePath, ctx.Req.Header.Get("Content-Range")) @@ -160,22 +173,104 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { }) } -// TODO: why it is used? +// TODO: why it is used? confirm artifact uploaded successfully? func (ar artifactRoutes) patchArtifact(ctx *context.Context) { - ctx.JSON(200, map[string]string{ "message": "success", }) } -func listJobArtifacts(ctx *context.Context) { - -} +func (ar artifactRoutes) listArtifacts(ctx *context.Context) { + // get task + taskID := ctx.ParamsInt64("taskID") + task, err := actions.GetTaskByID(ctx, taskID) + if err != nil { + log.Error("Error getting task: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } -func listContainerArtifacts(ctx *context.Context) { + artficats, err := actions.ListArtifactByJobID(ctx, task.JobID) + if err != nil { + log.Error("Error getting artifacts: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + artficatsData := make([]map[string]interface{}, 0, len(artficats)) + for _, a := range artficats { + url, err := ar.buildArtifactUrl(taskID, a.ID, "path") + if err != nil { + log.Error("Error parsing artifact URL: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + artficatsData = append(artficatsData, map[string]interface{}{ + "name": a.ArtifactName, + "fileContainerResourceUrl": url, + }) + } + respData := map[string]interface{}{ + "count": len(artficatsData), + "value": artficatsData, + } + ctx.JSON(200, respData) } -func downloadArtifact(ctx *context.Context) { +func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { + artifactID := ctx.ParamsInt64("artifactID") + artifact, err := actions.GetArtifactByID(ctx, artifactID) + if err != nil { + log.Error("Error getting artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + taskID := ctx.ParamsInt64("taskID") + url, err := ar.buildArtifactUrl(taskID, artifact.ID, "download") + if err != nil { + log.Error("Error parsing download URL: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + itemPath := ctx.Req.URL.Query().Get("itemPath") + artifactData := map[string]string{ + "path": filepath.Join(itemPath, artifact.ArtifactPath), + "itemType": "file", + "contentLocation": url, + } + respData := map[string]interface{}{ + "value": []interface{}{artifactData}, + } + ctx.JSON(200, respData) +} +func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { + artifactID := ctx.ParamsInt64("artifactID") + artifact, err := actions.GetArtifactByID(ctx, artifactID) + if err != nil { + log.Error("Error getting artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + taskID := ctx.ParamsInt64("taskID") + if artifact.JobID != taskID { + log.Error("Error dismatch taskID and artifactID, task: %v, artifact: %v", taskID, artifactID) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + fd, err := ar.fs.Open(artifact.FilePath) + if err != nil { + log.Error("Error opening file: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + if strings.HasSuffix(artifact.ArtifactPath, ".gz") { + ctx.Resp.Header().Set("Content-Encoding", "gzip") + } + _, err = io.Copy(ctx.Resp, fd) + if err != nil { + log.Error("Error copying file to response: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } } From 7a40012ef7299cd2d74af6858f957693739e022a Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 5 Jan 2023 10:33:11 +0800 Subject: [PATCH 04/38] fix(actions-artifacts): merge conflicts --- models/actions/artifacts.go | 51 -------------------------------- modules/setting/actions.go | 4 ++- modules/storage/storage.go | 9 +++++- routers/api/actions/artifacts.go | 24 +++++++++++---- 4 files changed, 30 insertions(+), 58 deletions(-) diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go index c44f54ab51192..5e7e2baf97da2 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifacts.go @@ -3,65 +3,14 @@ package actions import ( "context" "fmt" - "io" - "io/fs" "log" "net" - "os" - "path/filepath" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" ) -// ArtifactFile is a file that is stored in the artifact storage. -type ArtifactFile interface { - fs.File - io.Writer -} - -type MkdirFS interface { - MkdirAll(path string, perm fs.FileMode) error - Open(name string) (ArtifactFile, error) - OpenAtEnd(name string) (ArtifactFile, error) -} - -var _ MkdirFS = (*DiskMkdirFs)(nil) - -type DiskMkdirFs struct { - dir string -} - -func NewDiskMkdirFs(dir string) *DiskMkdirFs { - return &DiskMkdirFs{dir: dir} -} - -func (fsys DiskMkdirFs) MkdirAll(path string, perm fs.FileMode) error { - fpath := filepath.Join(fsys.dir, path) - return os.MkdirAll(fpath, perm) -} - -func (fsys DiskMkdirFs) Open(name string) (ArtifactFile, error) { - fpath := filepath.Join(fsys.dir, name) - dirpath := filepath.Dir(fpath) - os.MkdirAll(dirpath, os.ModePerm) - return os.OpenFile(fpath, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644) -} - -func (fsys DiskMkdirFs) OpenAtEnd(name string) (ArtifactFile, error) { - fpath := filepath.Join(fsys.dir, name) - file, err := os.OpenFile(fpath, os.O_CREATE|os.O_RDWR, 0644) - if err != nil { - return nil, err - } - _, err = file.Seek(0, os.SEEK_END) - if err != nil { - return nil, err - } - return file, nil -} - // https://stackoverflow.com/a/37382208 // Get preferred outbound ip of this machine func GetOutboundIP() net.IP { diff --git a/modules/setting/actions.go b/modules/setting/actions.go index b11500dab4a44..1b970e33b826f 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -10,7 +10,8 @@ import ( // Actions settings var ( Actions = struct { - Storage // how the created logs should be stored + Storage // how the created logs should be stored + Artifacts Storage // how the created artifacts should be stored Enabled bool DefaultActionsURL string `ini:"DEFAULT_ACTIONS_URL"` }{ @@ -26,4 +27,5 @@ func loadActionsFrom(rootCfg ConfigProvider) { } Actions.Storage = getStorage(rootCfg, "actions_log", "", nil) + Actions.Artifacts = getStorage(rootCfg, "actions_artifacts", "", nil) } diff --git a/modules/storage/storage.go b/modules/storage/storage.go index caecab306e630..30cc7497622b9 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -128,6 +128,8 @@ var ( // Actions represents actions storage Actions ObjectStorage = uninitializedStorage + // Actions Artifacts represents actions artifacts storage + ActionsArtifacts ObjectStorage = uninitializedStorage ) // Init init the stoarge @@ -212,9 +214,14 @@ func initPackages() (err error) { func initActions() (err error) { if !setting.Actions.Enabled { Actions = discardStorage("Actions isn't enabled") + ActionsArtifacts = discardStorage("ActionsArtifacts isn't enabled") return nil } log.Info("Initialising Actions storage with type: %s", setting.Actions.Storage.Type) - Actions, err = NewStorage(setting.Actions.Storage.Type, &setting.Actions.Storage) + if Actions, err = NewStorage(setting.Actions.Storage.Type, &setting.Actions.Storage); err != nil { + return err + } + log.Info("Initialising ActionsArtifacts storage with type: %s", setting.Actions.Artifacts.Type) + ActionsArtifacts, err = NewStorage(setting.Actions.Artifacts.Type, &setting.Actions.Artifacts) return err } diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 5f68afe5fd28b..68bb8e9ab6185 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "os" "path/filepath" "strconv" "strings" @@ -14,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/web" ) @@ -23,7 +25,9 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { m := web.NewRoute() m.Use(withContexter(goctx)) - r := artifactRoutes{prefix: prefix, fs: actions.NewDiskMkdirFs("./artifacts-data")} + r := artifactRoutes{ + prefix: prefix, + fs: storage.ActionsArtifacts} // retrieve, list and confirm artifacts m.Post(artifactRouteBase, r.getUploadArtifactURL) @@ -57,12 +61,16 @@ func withContexter(ctx gocontext.Context) func(next http.Handler) http.Handler { type artifactRoutes struct { prefix string - fs actions.MkdirFS + fs storage.ObjectStorage } -func (ar artifactRoutes) openFile(fpath string, contentRange string) (actions.ArtifactFile, bool, error) { +func (ar artifactRoutes) openFile(fpath string, contentRange string) (storage.Object, bool, error) { if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") { - f, err := ar.fs.OpenAtEnd(fpath) + f, err := ar.fs.Open(fpath) + if err != nil { + return nil, false, err + } + _, err = f.Seek(0, os.SEEK_END) return f, true, err } f, err := ar.fs.Open(fpath) @@ -149,8 +157,14 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { // chunked means it is a continuation of a previous upload fSize = artifact.FileSize } + writer, ok := file.(io.Writer) + if !ok { + log.Error("Error casting file to writer: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } - n, err := io.Copy(file, ctx.Req.Body) + n, err := io.Copy(writer, ctx.Req.Body) if err != nil { log.Error("Error copying body to file: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) From e40901fcd452fd9afcb11b304148ce80ddb55d69 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 5 Jan 2023 10:44:59 +0800 Subject: [PATCH 05/38] feat(actions-artifacts): use uniform pipeline api url for artifact, for future uses with other api in action runner --- routers/init.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/routers/init.go b/routers/init.go index 692af6840fb2e..8fd5298bcabd6 100644 --- a/routers/init.go +++ b/routers/init.go @@ -193,7 +193,11 @@ func NormalRoutes(ctx context.Context) *web.Route { if setting.Actions.Enabled { prefix := "/api/actions" r.Mount(prefix, actions_router.Routes(ctx, prefix)) - prefix = "/api/artifacts" + + // TODO: Pipeline api used for runner internal communication with gitea server. but only artifact is used for now. + // In Github, it uses ACTIONS_RUNTIME_URL=https://pipelines.actions.githubusercontent.com/fLgcSHkPGySXeIFrg8W8OBSfeg3b5Fls1A1CwX566g8PayEGlg/ + // TODO: this prefix should be generated with a token string with runner ? + prefix = "/api/actions_pipeline" r.Mount(prefix, actions_router.ArtifactsRoutes(ctx, prefix)) } From 45fc0391a30aaa82e866b817b6abec4063f424a9 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 5 Jan 2023 10:49:54 +0800 Subject: [PATCH 06/38] feat(actions-artifacts): add artifacts view page --- models/actions/artifacts.go | 6 +++ routers/api/actions/artifacts.go | 5 +- routers/web/repo/actions/view.go | 58 ++++++++++++++++++++++++ routers/web/web.go | 2 + web_src/js/components/RepoActionView.vue | 43 ++++++++++++++++++ 5 files changed, 113 insertions(+), 1 deletion(-) diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go index 5e7e2baf97da2..83232b9a53dbb 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifacts.go @@ -1,3 +1,9 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +// This artifact server is inspired by https://github.com/nektos/act/blob/master/pkg/artifacts/server.go. +// It updates url setting and uses ObjectStore to handle artifacts persistence. + package actions import ( diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 68bb8e9ab6185..d95c4f5287402 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -1,3 +1,6 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package actions import ( @@ -42,7 +45,7 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { return m } -// PackageContexter initializes a package context for a request. +// withContexter initializes a package context for a request. func withContexter(ctx gocontext.Context) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index c553aef9aef5d..216cfaabd4492 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/base" context_module "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -381,3 +382,60 @@ func getRunJobs(ctx *context_module.Context, runIndex, jobIndex int64) (*actions } return jobs[0], jobs } + +type ArtifactsViewResponse struct { + Artifacts []*ArtifactsViewItem `json:"artifacts"` +} + +type ArtifactsViewItem struct { + Name string `json:"name"` + Size int64 `json:"size"` + ID int64 `json:"id"` +} + +func ArtifactsView(ctx *context_module.Context) { + runIndex := ctx.ParamsInt64("run") + artifacts, err := actions_model.ListArtifactByJobID(ctx, runIndex) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + artifactsResponse := ArtifactsViewResponse{ + Artifacts: make([]*ArtifactsViewItem, 0, len(artifacts)), + } + for _, art := range artifacts { + artifactsResponse.Artifacts = append(artifactsResponse.Artifacts, &ArtifactsViewItem{ + Name: art.ArtifactName, + Size: art.FileSize, + ID: art.ID, + }) + } + ctx.JSON(http.StatusOK, artifactsResponse) +} + +func ArtifactsDownloadView(ctx *context_module.Context) { + runIndex := ctx.ParamsInt64("run") + artifactID := ctx.ParamsInt64("id") + + artifact, err := actions_model.GetArtifactByID(ctx, artifactID) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + if artifact.JobID != runIndex { + ctx.Error(http.StatusNotFound, "artifact not found") + return + } + + f, err := storage.ActionsArtifacts.Open(artifact.FilePath) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + defer f.Close() + + ctx.ServeContent(f, &context_module.ServeHeaderOptions{ + Filename: artifact.ArtifactName, + LastModified: artifact.Created.AsLocalTime(), + }) +} diff --git a/routers/web/web.go b/routers/web/web.go index a4ef96ecbe0f1..51aca75cf6686 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1189,6 +1189,8 @@ func registerRoutes(m *web.Route) { }) m.Post("/cancel", reqRepoActionsWriter, actions.Cancel) m.Post("/approve", reqRepoActionsWriter, actions.Approve) + m.Post("/artifacts", actions.ArtifactsView) + m.Get("/artifacts/{id}", actions.ArtifactsDownloadView) }) }, reqRepoActionsReader, actions.MustEnableActions) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 6fcd6ce880ce1..1fb1e7bdf6793 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -39,6 +39,17 @@ +
+
Artifacts
+ +
@@ -98,6 +109,7 @@ const sfc = { loading: false, intervalID: null, currentJobStepsStates: [], + artifacts: [], // provided by backend run: { @@ -149,6 +161,8 @@ const sfc = { // load job data and then auto-reload periodically this.loadJob(); this.intervalID = setInterval(this.loadJob, 1000); + // load artifactss list once + this.loadArtifacts(); }, methods: { @@ -277,6 +291,13 @@ const sfc = { } }, + + async loadArtifacts() { + const resp = await this.fetchPost(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); + const artifacts = await resp.json(); + this.artifacts = artifacts['artifacts'] || []; + }, + fetchPost(url, body) { return fetch(url, { method: 'POST', @@ -417,6 +438,28 @@ export function ansiLogToHTML(line) { margin: 5px 0; padding: 10px; } +.job-artifacts { + .job-artifacts-title { + font-size: 110%; + margin-top: 16px; + padding: 16px 10px 0px 20px; + border-top: 1px dashed var(--color-secondary); + } + + .job-artifacts-item { + margin: 5px 0; + padding: 6px; + } + + .job-artifacts-list { + padding-left: 12px; + list-style: none; + } + + .job-artifacts-icon { + padding-right: 3px; + } +} .job-group-section .job-brief-list .job-brief-item { margin: 5px 0; From 8787632525cbcde231e6af1a9028d1390c0f1c91 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 16 Feb 2023 22:08:34 +0800 Subject: [PATCH 07/38] feat(actions-artifacts): delete artifacts in storage after repo is deleted --- models/actions/artifacts.go | 6 ++++++ models/repo.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go index 83232b9a53dbb..ffb2987c06e28 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifacts.go @@ -89,3 +89,9 @@ func ListArtifactByJobID(ctx context.Context, jobID int64) ([]*ActionArtifact, e arts := make([]*ActionArtifact, 0, 10) return arts, db.GetEngine(ctx).Where("job_id=?", jobID).Find(&arts) } + +// ListArtifactsByRepoID returns all artifacts of a repo +func ListArtifactsByRepoID(ctx context.Context, repoID int64) ([]*ActionArtifact, error) { + arts := make([]*ActionArtifact, 0, 10) + return arts, db.GetEngine(ctx).Where("repo_id=?", repoID).Find(&arts) +} diff --git a/models/repo.go b/models/repo.go index 5a1e2e028e81c..4f47e035c134c 100644 --- a/models/repo.go +++ b/models/repo.go @@ -60,6 +60,12 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { return fmt.Errorf("find actions tasks of repo %v: %w", repoID, err) } + // Query the artifacts of this repo, they will be needed after they have been deleted to remove artifacts files in ObjectStorage + artifacts, err := actions_model.ListArtifactsByRepoID(ctx, repoID) + if err != nil { + return fmt.Errorf("list actions artifacts of repo %v: %w", repoID, err) + } + // In case is a organization. org, err := user_model.GetUserByID(ctx, uid) if err != nil { @@ -165,6 +171,7 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { &actions_model.ActionRunJob{RepoID: repoID}, &actions_model.ActionRun{RepoID: repoID}, &actions_model.ActionRunner{RepoID: repoID}, + &actions_model.ActionArtifact{RepoID: repoID}, ); err != nil { return fmt.Errorf("deleteBeans: %w", err) } @@ -337,6 +344,14 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { } } + // delete actions artifacts in ObjectStorage after the repo have already been deleted + for _, art := range artifacts { + if err := storage.ActionsArtifacts.Delete(art.FilePath); err != nil { + log.Error("remove artifact file %q: %v", art.FilePath, err) + // go on + } + } + return nil } From ed76a1aeff00d839e7e2d33824ef705654659e35 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 16 Feb 2023 22:10:16 +0800 Subject: [PATCH 08/38] fix(actions-artifacts): drop 'localhost' check, the runner should check 'localhost' gitea instance when docker mode --- models/actions/artifacts.go | 16 ++++++++-------- models/repo.go | 4 ++-- routers/api/actions/artifacts.go | 14 +++----------- routers/web/repo/actions/view.go | 2 +- 4 files changed, 14 insertions(+), 22 deletions(-) diff --git a/models/actions/artifacts.go b/models/actions/artifacts.go index ffb2987c06e28..ed35f256c216e 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifacts.go @@ -39,14 +39,14 @@ func init() { type ActionArtifact struct { ID int64 JobID int64 - RunnerID int64 `xorm:"index"` - RepoID int64 `xorm:"index"` - OwnerID int64 `xorm:"index"` - CommitSHA string `xorm:"index"` - FilePath string - FileSize int64 - ArtifactPath string - ArtifactName string + RunnerID int64 `xorm:"index"` + RepoID int64 `xorm:"index"` + OwnerID int64 `xorm:"index"` + CommitSHA string `xorm:"index"` + StoragePath string // The path to the artifact in the storage + FileSize int64 // The size of the artifact in bytes + ArtifactPath string // The path to the artifact when runner uploads it + ArtifactName string // The name of the artifact when runner uploads it Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated index"` } diff --git a/models/repo.go b/models/repo.go index 4f47e035c134c..6565d800ae245 100644 --- a/models/repo.go +++ b/models/repo.go @@ -346,8 +346,8 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { // delete actions artifacts in ObjectStorage after the repo have already been deleted for _, art := range artifacts { - if err := storage.ActionsArtifacts.Delete(art.FilePath); err != nil { - log.Error("remove artifact file %q: %v", art.FilePath, err) + if err := storage.ActionsArtifacts.Delete(art.StoragePath); err != nil { + log.Error("remove artifact file %q: %v", art.StoragePath, err) // go on } } diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index d95c4f5287402..b451eebc7b696 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -88,14 +88,6 @@ func (ar artifactRoutes) buildArtifactUrl(taskID int64, artifactID int64, suffix if err != nil { return "", err } - // FIXME: fix localhost ?? act starts docker container to run action. - // It can't visit host network in container. - if strings.Contains(u.Host, "localhost") { - u.Host = strings.ReplaceAll(u.Host, "localhost", actions.GetOutboundIP().String()) - } - if strings.Contains(u.Host, "127.0.0.1") { - u.Host = strings.ReplaceAll(u.Host, "127.0.0.1", actions.GetOutboundIP().String()) - } return u.String(), nil } @@ -123,7 +115,7 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { return } - ctx.JSON(200, map[string]interface{}{ + ctx.JSON(http.StatusOK, map[string]interface{}{ "fileContainerResourceUrl": url, }) } @@ -174,7 +166,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { return } fSize += n - artifact.FilePath = filePath // path in storage + artifact.StoragePath = filePath // path in storage artifact.ArtifactName = artifactName artifact.ArtifactPath = itemPath // path in container artifact.FileSize = fSize @@ -275,7 +267,7 @@ func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - fd, err := ar.fs.Open(artifact.FilePath) + fd, err := ar.fs.Open(artifact.StoragePath) if err != nil { log.Error("Error opening file: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 216cfaabd4492..6ffe8cc50148c 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -427,7 +427,7 @@ func ArtifactsDownloadView(ctx *context_module.Context) { return } - f, err := storage.ActionsArtifacts.Open(artifact.FilePath) + f, err := storage.ActionsArtifacts.Open(artifact.StoragePath) if err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return From c1e08de2cc90a9e87be8585526e03a928bd6d437 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 17 Feb 2023 21:53:08 +0800 Subject: [PATCH 09/38] fix(actions-artifacts): need create file when upload first time --- routers/api/actions/artifacts.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index b451eebc7b696..3efabe084419c 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -4,7 +4,9 @@ package actions import ( + "bytes" gocontext "context" + "errors" "fmt" "io" "net/http" @@ -30,7 +32,8 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { r := artifactRoutes{ prefix: prefix, - fs: storage.ActionsArtifacts} + fs: storage.ActionsArtifacts, + } // retrieve, list and confirm artifacts m.Post(artifactRouteBase, r.getUploadArtifactURL) @@ -68,6 +71,13 @@ type artifactRoutes struct { } func (ar artifactRoutes) openFile(fpath string, contentRange string) (storage.Object, bool, error) { + // if fpath is not exist, it should use Save to create a new file + if _, err := ar.fs.Stat(fpath); err != nil && errors.Is(err, os.ErrNotExist) { + if _, err = ar.fs.Save(fpath, bytes.NewBuffer(nil), -1); err != nil { + return nil, false, err + } + } + if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") { f, err := ar.fs.Open(fpath) if err != nil { @@ -177,14 +187,14 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { return } - ctx.JSON(200, map[string]string{ + ctx.JSON(http.StatusOK, map[string]string{ "message": "success", }) } // TODO: why it is used? confirm artifact uploaded successfully? func (ar artifactRoutes) patchArtifact(ctx *context.Context) { - ctx.JSON(200, map[string]string{ + ctx.JSON(http.StatusOK, map[string]string{ "message": "success", }) } @@ -223,7 +233,7 @@ func (ar artifactRoutes) listArtifacts(ctx *context.Context) { "count": len(artficatsData), "value": artficatsData, } - ctx.JSON(200, respData) + ctx.JSON(http.StatusOK, respData) } func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { @@ -250,7 +260,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { respData := map[string]interface{}{ "value": []interface{}{artifactData}, } - ctx.JSON(200, respData) + ctx.JSON(http.StatusOK, respData) } func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { From 163b7afda3478109455ec5455773dcf847b5d4bf Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Mon, 20 Feb 2023 23:07:26 +0800 Subject: [PATCH 10/38] feat(artifacts): save artifact chunks --- models/actions/{artifacts.go => artifact.go} | 45 +-- routers/api/actions/artifacts.go | 274 +++++++++++++++---- 2 files changed, 247 insertions(+), 72 deletions(-) rename models/actions/{artifacts.go => artifact.go} (61%) diff --git a/models/actions/artifacts.go b/models/actions/artifact.go similarity index 61% rename from models/actions/artifacts.go rename to models/actions/artifact.go index ed35f256c216e..1ed53b7fa7e0c 100644 --- a/models/actions/artifacts.go +++ b/models/actions/artifact.go @@ -17,6 +17,13 @@ import ( "code.gitea.io/gitea/modules/util" ) +const ( + // ArtifactUploadStatusPending is the status of an artifact upload that is pending + ArtifactUploadStatusPending = -1 + // ArtifactUploadStatusConfirmed is the status of an artifact upload that is confirmed + ArtifactUploadStatusConfirmed = 1 +) + // https://stackoverflow.com/a/37382208 // Get preferred outbound ip of this machine func GetOutboundIP() net.IP { @@ -37,28 +44,32 @@ func init() { // ActionArtifact is a file that is stored in the artifact storage. type ActionArtifact struct { - ID int64 - JobID int64 - RunnerID int64 `xorm:"index"` - RepoID int64 `xorm:"index"` - OwnerID int64 `xorm:"index"` - CommitSHA string `xorm:"index"` - StoragePath string // The path to the artifact in the storage - FileSize int64 // The size of the artifact in bytes - ArtifactPath string // The path to the artifact when runner uploads it - ArtifactName string // The name of the artifact when runner uploads it - Created timeutil.TimeStamp `xorm:"created"` - Updated timeutil.TimeStamp `xorm:"updated index"` + ID int64 + JobID int64 + RunnerID int64 `xorm:"index"` + RepoID int64 `xorm:"index"` + OwnerID int64 `xorm:"index"` + CommitSHA string `xorm:"index"` + StoragePath string // The path to the artifact in the storage + FileSize int64 // The size of the artifact in bytes + FileGzipSize int64 // The size of the artifact in bytes after gzip compression + ContentEncnoding string // The content encoding of the artifact + ArtifactPath string // The path to the artifact when runner uploads it + ArtifactName string // The name of the artifact when runner uploads it + UploadStatus int64 `xorm:"index"` // The status of the artifact upload + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated index"` } // CreateArtifact creates a new artifact with task info func CreateArtifact(ctx context.Context, t *ActionTask) (*ActionArtifact, error) { artifact := &ActionArtifact{ - JobID: t.JobID, - RunnerID: t.RunnerID, - RepoID: t.RepoID, - OwnerID: t.OwnerID, - CommitSHA: t.CommitSHA, + JobID: t.JobID, + RunnerID: t.RunnerID, + RepoID: t.RepoID, + OwnerID: t.OwnerID, + CommitSHA: t.CommitSHA, + UploadStatus: ArtifactUploadStatusPending, } _, err := db.GetEngine(ctx).Insert(artifact) return artifact, err diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 3efabe084419c..533a0b1e4c807 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -5,16 +5,19 @@ package actions import ( "bytes" + "compress/gzip" gocontext "context" - "errors" + "crypto/md5" + "encoding/base64" "fmt" "io" "net/http" "net/url" - "os" "path/filepath" + "sort" "strconv" "strings" + "time" "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/modules/context" @@ -24,6 +27,11 @@ import ( "code.gitea.io/gitea/modules/web" ) +const artifactXTfsFileLengthHeader = "x-tfs-filelength" +const artifactXActionsResultsMD5Header = "x-actions-results-md5" + +// const artifactXActionsResultsCRC64Header = "x-actions-results-crc64" + const artifactRouteBase = "/_apis/pipelines/workflows/{taskID}/artifacts" func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { @@ -38,7 +46,7 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { // retrieve, list and confirm artifacts m.Post(artifactRouteBase, r.getUploadArtifactURL) m.Get(artifactRouteBase, r.listArtifacts) - m.Patch(artifactRouteBase, r.patchArtifact) + m.Patch(artifactRouteBase, r.comfirmUploadArtifact) // handle container artifacts m.Put(artifactRouteBase+"/{artifactID}/upload", r.uploadArtifact) @@ -70,26 +78,6 @@ type artifactRoutes struct { fs storage.ObjectStorage } -func (ar artifactRoutes) openFile(fpath string, contentRange string) (storage.Object, bool, error) { - // if fpath is not exist, it should use Save to create a new file - if _, err := ar.fs.Stat(fpath); err != nil && errors.Is(err, os.ErrNotExist) { - if _, err = ar.fs.Save(fpath, bytes.NewBuffer(nil), -1); err != nil { - return nil, false, err - } - } - - if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") { - f, err := ar.fs.Open(fpath) - if err != nil { - return nil, false, err - } - _, err = f.Seek(0, os.SEEK_END) - return f, true, err - } - f, err := ar.fs.Open(fpath) - return f, false, err -} - func (ar artifactRoutes) buildArtifactUrl(taskID int64, artifactID int64, suffix string) (string, error) { uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") + strings.ReplaceAll(artifactRouteBase, "{taskID}", strconv.FormatInt(taskID, 10)) + @@ -125,11 +113,73 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { return } + log.Debug("[artifact] get upload url: %s, artifact id: %d", url, artifact.ID) + ctx.JSON(http.StatusOK, map[string]interface{}{ "fileContainerResourceUrl": url, }) } +// getUploadFileSize returns the size of the file to be uploaded. +// The raw size is the size of the file as reported by the header X-TFS-FileLength. +func (ar artifactRoutes) getUploadFileSize(ctx *context.Context) (int64, int64, error) { + contentLength := ctx.Req.ContentLength + xTfsLength, _ := strconv.ParseInt(ctx.Req.Header.Get(artifactXTfsFileLengthHeader), 10, 64) + if xTfsLength > 0 { + return xTfsLength, contentLength, nil + } + return contentLength, contentLength, nil +} + +func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, + artifact *actions.ActionArtifact, + contentSize int64, taskID int64) (int64, error) { + // read chunk data to calc md5 + chunkData, err := io.ReadAll(ctx.Req.Body) + if err != nil { + return -1, err + } + + // check md5 + md5Hash := md5.Sum(chunkData) + md5String := base64.StdEncoding.EncodeToString(md5Hash[:]) + if md5String != ctx.Req.Header.Get(artifactXActionsResultsMD5Header) { + return -1, fmt.Errorf("md5 not match") + } + + contentRange := ctx.Req.Header.Get("Content-Range") + start, end, length := int64(0), int64(0), int64(0) + if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &length); err != nil { + return -1, fmt.Errorf("parse content range error: %v", err) + } + + storagePath := fmt.Sprintf("tmp%d/%d-%d-%d.chunk", taskID, artifact.ID, start, end) + + // save chunk to storage + if _, err := ar.fs.Save(storagePath, bytes.NewBuffer(chunkData), -1); err != nil { + return -1, fmt.Errorf("save chunk to storage error: %v", err) + } + + log.Debug("[artifact] save chunk %s, size: %d, artifact id: %d, start: %d, end: %d", + storagePath, contentSize, artifact.ID, start, end) + + return length, nil +} + +var invalidArtifactNameChars = []string{"\\", "/", "\"", ":", "<", ">", "|", "*", "?", "\r", "\n"} + +// checkArtifactName checks if the artifact name contains invalid characters. +// If the name contains invalid characters, an error is returned. +// The rules are from https://github.com/actions/toolkit/blob/main/packages/artifact/src/internal/path-and-artifact-name-validation.ts#L32 +func checkArtifactName(name string) error { + for _, c := range invalidArtifactNameChars { + if strings.Contains(name, c) { + return fmt.Errorf("artifact name contains invalid character %s", c) + } + } + return nil +} + func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { artifactID := ctx.ParamsInt64("artifactID") @@ -141,50 +191,42 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { } itemPath := ctx.Req.URL.Query().Get("itemPath") - taskID := ctx.Params("taskID") + taskID := ctx.ParamsInt64("taskID") artifactName := strings.Split(itemPath, "/")[0] - - if ctx.Req.Header.Get("Content-Encoding") == "gzip" { - itemPath += ".gz" - } - filePath := fmt.Sprintf("%s/%d/%s", taskID, artifactID, itemPath) - - fSize := int64(0) - file, isChunked, err := ar.openFile(filePath, ctx.Req.Header.Get("Content-Range")) - if err != nil { - log.Error("Error opening file: %v", err) + if err = checkArtifactName(artifactName); err != nil { + log.Error("Error checking artifact name: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return } - defer file.Close() - if isChunked { - // chunked means it is a continuation of a previous upload - fSize = artifact.FileSize - } - writer, ok := file.(io.Writer) - if !ok { - log.Error("Error casting file to writer: %v", err) + // get upload file size + fileSize, contentLength, err := ar.getUploadFileSize(ctx) + if err != nil { + log.Error("Error getting upload file size: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return } - n, err := io.Copy(writer, ctx.Req.Body) + // save chunk + chunkAllLength, err := ar.saveUploadChunk(ctx, artifact, contentLength, taskID) if err != nil { - log.Error("Error copying body to file: %v", err) + log.Error("Error saving upload chunk: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return } - fSize += n - artifact.StoragePath = filePath // path in storage - artifact.ArtifactName = artifactName - artifact.ArtifactPath = itemPath // path in container - artifact.FileSize = fSize - if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { - log.Error("Error updating artifact: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) - return + // if artifact name is not set, update it + if artifact.ArtifactName == "" { + artifact.ArtifactName = artifactName + artifact.ArtifactPath = itemPath // path in container + artifact.FileSize = fileSize // this is total size of all chunks + artifact.FileGzipSize = chunkAllLength + artifact.ContentEncnoding = ctx.Req.Header.Get("Content-Encoding") + if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { + log.Error("Error updating artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } } ctx.JSON(http.StatusOK, map[string]string{ @@ -192,13 +234,135 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { }) } -// TODO: why it is used? confirm artifact uploaded successfully? -func (ar artifactRoutes) patchArtifact(ctx *context.Context) { +// comfirmUploadArtifact comfirm upload artifact. +// if all chunks are uploaded, merge them to one file. +func (ar artifactRoutes) comfirmUploadArtifact(ctx *context.Context) { + taskID := ctx.ParamsInt64("taskID") + if err := ar.mergeArtifactChunks(ctx, taskID); err != nil { + log.Error("Error merging chunks: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + ctx.JSON(http.StatusOK, map[string]string{ "message": "success", }) } +type chunkItem struct { + ArtifactID int64 + Start int64 + End int64 + Path string +} + +func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, taskID int64) error { + storageDir := fmt.Sprintf("tmp%d", taskID) + var chunks []*chunkItem + if err := ar.fs.IterateObjects(func(path string, obj storage.Object) error { + defer obj.Close() + if !strings.HasPrefix(path, storageDir) { + return nil + } + + item := chunkItem{Path: path} + if _, err := fmt.Sscanf(path, storageDir+"/%d-%d-%d.chunk", &item.ArtifactID, &item.Start, &item.End); err != nil { + return fmt.Errorf("parse content range error: %v", err) + } + chunks = append(chunks, &item) + return nil + }); err != nil { + return err + } + // group chunks by artifact id + chunksMap := make(map[int64][]*chunkItem) + for _, c := range chunks { + chunksMap[c.ArtifactID] = append(chunksMap[c.ArtifactID], c) + } + + for artifact_id, cs := range chunksMap { + // get artifact to handle merged chunks + artifact, err := actions.GetArtifactByID(ctx, cs[0].ArtifactID) + if err != nil { + return fmt.Errorf("get artifact error: %v", err) + } + + sort.Slice(cs, func(i, j int) bool { + return cs[i].Start < cs[j].Start + }) + + allChunks := make([]*chunkItem, 0) + startAt := int64(-1) + // check if all chunks are uploaded and in order and clean repeated chunks + for _, c := range cs { + // startAt is -1 means this is the first chunk + // previous c.ChunkEnd + 1 == c.ChunkStart means this chunk is in order + // StartAt is not -1 and c.ChunkStart is not startAt + 1 means there is a chunk missing + if c.Start == (startAt + 1) { + allChunks = append(allChunks, c) + startAt = c.End + } + } + + // if the last chunk.End + 1 is not equal to chunk.ChunkLength, means chunks are not uploaded completely + if startAt+1 != artifact.FileGzipSize { + log.Debug("[artifact] chunks are not uploaded completely, artifact_id: %d", artifact_id) + break + } + + // use multiReader + readers := make([]io.Reader, 0, len(allChunks)) + for _, c := range allChunks { + reader, err := ar.fs.Open(c.Path) + if err != nil { + return fmt.Errorf("open chunk error: %v, %s", err, c.Path) + } + readers = append(readers, reader) + } + mergedReader := io.MultiReader(readers...) + + // if chunk is gzip, decompress it + if artifact.ContentEncnoding == "gzip" { + var err error + mergedReader, err = gzip.NewReader(mergedReader) + if err != nil { + return fmt.Errorf("gzip reader error: %v", err) + } + } + + // save merged file + storagePath := fmt.Sprintf("%d/%d/%d.chunk", (taskID+artifact_id)%255, artifact.FileSize%255, time.Now().UnixNano()) + written, err := ar.fs.Save(storagePath, mergedReader, -1) + if err != nil { + return fmt.Errorf("save merged file error: %v", err) + } + if written != artifact.FileSize { + return fmt.Errorf("merged file size is not equal to chunk length") + } + + // close readers + for _, r := range readers { + r.(io.Closer).Close() + } + + // save storage path to artifact + log.Debug("[artifact] merge chunks to artifact: %d, %s", artifact.ID, storagePath) + artifact.StoragePath = storagePath + artifact.UploadStatus = actions.ArtifactUploadStatusConfirmed + if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { + return fmt.Errorf("update artifact error: %v", err) + } + + // drop chunks + for _, c := range cs { + if err := ar.fs.Delete(c.Path); err != nil { + return fmt.Errorf("delete chunk file error: %v", err) + } + } + } + return nil +} + func (ar artifactRoutes) listArtifacts(ctx *context.Context) { // get task taskID := ctx.ParamsInt64("taskID") From 65244372c1f5cd3497370532c3c0cf52af5cb15d Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Mon, 13 Mar 2023 21:05:02 +0800 Subject: [PATCH 11/38] feat(artifacts): add token auth for runner api, fix lint --- models/actions/artifact.go | 20 ++--------- routers/api/actions/artifacts.go | 44 ++++++++++++++++-------- web_src/js/components/RepoActionView.vue | 6 ++-- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 1ed53b7fa7e0c..984a62cc18a17 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -9,8 +9,6 @@ package actions import ( "context" "fmt" - "log" - "net" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/timeutil" @@ -24,28 +22,14 @@ const ( ArtifactUploadStatusConfirmed = 1 ) -// https://stackoverflow.com/a/37382208 -// Get preferred outbound ip of this machine -func GetOutboundIP() net.IP { - conn, err := net.Dial("udp", "8.8.8.8:80") - if err != nil { - log.Fatal(err) - } - defer conn.Close() - - localAddr := conn.LocalAddr().(*net.UDPAddr) - - return localAddr.IP -} - func init() { db.RegisterModel(new(ActionArtifact)) } // ActionArtifact is a file that is stored in the artifact storage. type ActionArtifact struct { - ID int64 - JobID int64 + ID int64 `xorm:"pk autoincr"` + JobID int64 `xorm:"index"` RunnerID int64 `xorm:"index"` RepoID int64 `xorm:"index"` OwnerID int64 `xorm:"index"` diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 533a0b1e4c807..1da4a7d19d92a 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -27,8 +27,10 @@ import ( "code.gitea.io/gitea/modules/web" ) -const artifactXTfsFileLengthHeader = "x-tfs-filelength" -const artifactXActionsResultsMD5Header = "x-actions-results-md5" +const ( + artifactXTfsFileLengthHeader = "x-tfs-filelength" + artifactXActionsResultsMD5Header = "x-actions-results-md5" +) // const artifactXActionsResultsCRC64Header = "x-actions-results-crc64" @@ -66,6 +68,21 @@ func withContexter(ctx gocontext.Context) func(next http.Handler) http.Handler { } defer ctx.Close() + // action task call server api with Bearer ACTIONS_RUNTIME_TOKEN + // we should verify the ACTIONS_RUNTIME_TOKEN + authHeader := req.Header.Get("Authorization") + if len(authHeader) == 0 || !strings.HasPrefix(authHeader, "Bearer ") { + ctx.Error(http.StatusUnauthorized, "Bad authorization header") + return + } + authToken := strings.TrimPrefix(authHeader, "Bearer ") + task, err := actions.GetRunningTaskByToken(req.Context(), authToken) + if err != nil { + log.Error("Error runner api getting task: %v", err) + ctx.Error(http.StatusInternalServerError, "Error runner api getting task") + } + ctx.Data["task"] = task + ctx.Req = context.WithContext(req, &ctx) next.ServeHTTP(ctx.Resp, ctx.Req) @@ -78,7 +95,7 @@ type artifactRoutes struct { fs storage.ObjectStorage } -func (ar artifactRoutes) buildArtifactUrl(taskID int64, artifactID int64, suffix string) (string, error) { +func (ar artifactRoutes) buildArtifactURL(taskID, artifactID int64, suffix string) (string, error) { uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") + strings.ReplaceAll(artifactRouteBase, "{taskID}", strconv.FormatInt(taskID, 10)) + "/" + strconv.FormatInt(artifactID, 10) + "/" + suffix @@ -106,7 +123,7 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - url, err := ar.buildArtifactUrl(taskID, artifact.ID, "upload") + url, err := ar.buildArtifactURL(taskID, artifact.ID, "upload") if err != nil { log.Error("Error parsing upload URL: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -133,7 +150,8 @@ func (ar artifactRoutes) getUploadFileSize(ctx *context.Context) (int64, int64, func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, artifact *actions.ActionArtifact, - contentSize int64, taskID int64) (int64, error) { + contentSize, taskID int64, +) (int64, error) { // read chunk data to calc md5 chunkData, err := io.ReadAll(ctx.Req.Body) if err != nil { @@ -259,12 +277,8 @@ type chunkItem struct { func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, taskID int64) error { storageDir := fmt.Sprintf("tmp%d", taskID) var chunks []*chunkItem - if err := ar.fs.IterateObjects(func(path string, obj storage.Object) error { + if err := ar.fs.IterateObjects(storageDir, func(path string, obj storage.Object) error { defer obj.Close() - if !strings.HasPrefix(path, storageDir) { - return nil - } - item := chunkItem{Path: path} if _, err := fmt.Sscanf(path, storageDir+"/%d-%d-%d.chunk", &item.ArtifactID, &item.Start, &item.End); err != nil { return fmt.Errorf("parse content range error: %v", err) @@ -280,7 +294,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, taskID int64) chunksMap[c.ArtifactID] = append(chunksMap[c.ArtifactID], c) } - for artifact_id, cs := range chunksMap { + for artifactID, cs := range chunksMap { // get artifact to handle merged chunks artifact, err := actions.GetArtifactByID(ctx, cs[0].ArtifactID) if err != nil { @@ -306,7 +320,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, taskID int64) // if the last chunk.End + 1 is not equal to chunk.ChunkLength, means chunks are not uploaded completely if startAt+1 != artifact.FileGzipSize { - log.Debug("[artifact] chunks are not uploaded completely, artifact_id: %d", artifact_id) + log.Debug("[artifact] chunks are not uploaded completely, artifact_id: %d", artifactID) break } @@ -331,7 +345,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, taskID int64) } // save merged file - storagePath := fmt.Sprintf("%d/%d/%d.chunk", (taskID+artifact_id)%255, artifact.FileSize%255, time.Now().UnixNano()) + storagePath := fmt.Sprintf("%d/%d/%d.chunk", (taskID+artifactID)%255, artifact.FileSize%255, time.Now().UnixNano()) written, err := ar.fs.Save(storagePath, mergedReader, -1) if err != nil { return fmt.Errorf("save merged file error: %v", err) @@ -382,7 +396,7 @@ func (ar artifactRoutes) listArtifacts(ctx *context.Context) { artficatsData := make([]map[string]interface{}, 0, len(artficats)) for _, a := range artficats { - url, err := ar.buildArtifactUrl(taskID, a.ID, "path") + url, err := ar.buildArtifactURL(taskID, a.ID, "path") if err != nil { log.Error("Error parsing artifact URL: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -409,7 +423,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { return } taskID := ctx.ParamsInt64("taskID") - url, err := ar.buildArtifactUrl(taskID, artifact.ID, "download") + url, err := ar.buildArtifactURL(taskID, artifact.ID, "download") if err != nil { log.Error("Error parsing download URL: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 1fb1e7bdf6793..342db3662b7d8 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -40,9 +40,11 @@
-
Artifacts
+
+ Artifacts +
- Artifacts + {{ artifactsTitle }}
{{template "base/footer" .}} From 5faba18996213869bcf30197c05a5911e4374245 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 27 Apr 2023 17:27:24 +0800 Subject: [PATCH 26/38] move artifact key to top-level in i18n --- options/locale/locale_en-US.ini | 4 ++-- templates/repo/actions/view.tmpl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9ebb58b34b892..34ecae92e8a0c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -113,6 +113,8 @@ unknown = Unknown rss_feed = RSS Feed +artifacts = Artifacts + [aria] navbar = Navigation Bar footer = Footer @@ -3412,8 +3414,6 @@ runners.status.offline = Offline runners.version = Version runners.reset_registration_token_success = Runner registration token reset successfully -artifacts.title = Artifacts - runs.all_workflows = All Workflows runs.open_tab = %d Open runs.closed_tab = %d Closed diff --git a/templates/repo/actions/view.tmpl b/templates/repo/actions/view.tmpl index 28da0dc9e068a..8ffb7f1b0b745 100644 --- a/templates/repo/actions/view.tmpl +++ b/templates/repo/actions/view.tmpl @@ -3,7 +3,7 @@
{{template "repo/header" .}}
+ data-actions-url="{{.ActionsURL}}" data-artifacts-title="{{$.locale.Tr "artifacts"}}">
{{template "base/footer" .}} From aacd427e336ae2f7a2b821e25189f772528ddc66 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 28 Apr 2023 09:35:26 +0800 Subject: [PATCH 27/38] merge artifact refresh into job interval in action view --- web_src/js/components/RepoActionView.vue | 27 +++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 5a96ab3c6c3ef..666f860be8653 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -165,12 +165,16 @@ const sfc = { mounted() { // load job data and then auto-reload periodically this.loadJob(); - // load artifactss list once - this.loadArtifacts(); - this.intervalID = setInterval(() => { - this.loadJob(); - this.loadArtifacts(); - }, 1000); + this.intervalID = setInterval(this.loadJob, 1000); + }, + + unmounted() { + // clear the interval timer when the component is unmounted + // even our page is rendered once, not spa style + if (this.intervalID) { + clearInterval(this.intervalID) + this.intervalID = null; + } }, methods: { @@ -271,6 +275,11 @@ const sfc = { try { this.loading = true; + // refresh artitfact list if upload-artifact step done + const resp = await this.fetchPost(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); + const artifacts = await resp.json(); + this.artifacts = artifacts['artifacts'] || []; + const response = await this.fetchJob(); // save the state to Vue data, then the UI will be updated @@ -300,12 +309,6 @@ const sfc = { }, - async loadArtifacts() { - const resp = await this.fetchPost(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); - const artifacts = await resp.json(); - this.artifacts = artifacts['artifacts'] || []; - }, - fetchPost(url, body) { return fetch(url, { method: 'POST', From 967bd697cb36f1dbad909353259571a3ecbaa2e9 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 28 Apr 2023 10:07:54 +0800 Subject: [PATCH 28/38] fix artifact unique index definition --- models/actions/artifact.go | 2 +- models/migrations/v1_20/v256.go | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 037f2ee83b9fe..a77c1f5b4ad79 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -28,7 +28,7 @@ func init() { // ActionArtifact is a file that is stored in the artifact storage. type ActionArtifact struct { ID int64 `xorm:"pk autoincr"` - RunID int64 `xorm:"index unique(run-id-name)"` // The run id of the artifact + RunID int64 `xorm:"index UNIQUE(runid_name)"` // The run id of the artifact RunnerID int64 RepoID int64 `xorm:"index"` OwnerID int64 diff --git a/models/migrations/v1_20/v256.go b/models/migrations/v1_20/v256.go index d6ec637ca93d4..e1869866e8017 100644 --- a/models/migrations/v1_20/v256.go +++ b/models/migrations/v1_20/v256.go @@ -10,20 +10,21 @@ import ( ) func CreateActionArtifactTable(x *xorm.Engine) error { + // ActionArtifact is a file that is stored in the artifact storage. type ActionArtifact struct { ID int64 `xorm:"pk autoincr"` - JobID int64 `xorm:"index"` + RunID int64 `xorm:"index UNIQUE(runid_name)"` // The run id of the artifact RunnerID int64 RepoID int64 `xorm:"index"` OwnerID int64 CommitSHA string - StoragePath string // The path to the artifact in the storage - FileSize int64 - FileGzipSize int64 - ContentEncnoding string // The content encoding of the artifact, such as gzip + StoragePath string // The path to the artifact in the storage + FileSize int64 // The size of the artifact in bytes + FileGzipSize int64 // The size of the artifact in bytes after gzip compression + ContentEncnoding string // The content encoding of the artifact ArtifactPath string // The path to the artifact when runner uploads it - ArtifactName string // The name of the artifact when runner uploads it - UploadStatus int64 `xorm:"index"` + ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it + UploadStatus int64 `xorm:"index"` // The status of the artifact upload Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated index"` } From 5eee187f2a86d11d02bfae9714e08fc9d70819d0 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 28 Apr 2023 10:44:09 +0800 Subject: [PATCH 29/38] update artifact response status code, routes defs, typed response --- routers/api/actions/artifacts.go | 141 +++++++++++------------ routers/web/repo/actions/view.go | 8 ++ web_src/js/components/RepoActionView.vue | 2 +- 3 files changed, 77 insertions(+), 74 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index d4fa6ae4140c5..275926eff9f85 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -67,7 +67,6 @@ import ( "crypto/md5" "encoding/base64" "fmt" - "hash" "io" "net/http" "net/url" @@ -104,15 +103,16 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { fs: storage.ActionsArtifacts, } - // retrieve, list and confirm artifacts - m.Get(artifactRouteBase, r.listArtifacts) - m.Post(artifactRouteBase, r.getUploadArtifactURL) - m.Patch(artifactRouteBase, r.comfirmUploadArtifact) - - // handle container artifacts - m.Put(artifactRouteBase+"/{artifactID}/upload", r.uploadArtifact) - m.Get(artifactRouteBase+"/{artifactID}/path", r.getDownloadArtifactURL) - m.Get(artifactRouteBase+"/{artifactID}/download", r.downloadArtifact) + m.Group(artifactRouteBase, func() { + // retrieve, list and confirm artifacts + m.Combo("").Get(r.listArtifacts).Post(r.getUploadArtifactURL).Patch(r.comfirmUploadArtifact) + // handle container artifacts list and download + m.Group("/{artifactID}", func() { + m.Put("/upload", r.uploadArtifact) + m.Get("/path", r.getDownloadArtifactURL) + m.Get("/download", r.downloadArtifact) + }) + }) return m } @@ -232,26 +232,6 @@ func (ar artifactRoutes) getUploadFileSize(ctx *context.Context) (int64, int64, return contentLength, contentLength, nil } -type hashReader struct { - Reader io.Reader - Hasher hash.Hash -} - -func (hr *hashReader) Read(p []byte) (n int, err error) { - n, err = hr.Reader.Read(p) - if n > 0 { - hr.Hasher.Write(p[:n]) - } - return n, err -} - -func (hr *hashReader) Match(md5Str string) bool { - md5Hash := hr.Hasher.Sum(nil) - md5String := base64.StdEncoding.EncodeToString(md5Hash) - log.Debug("[artifact] check chunk md5, sum: %s, header: %s", md5String, md5Str) - return md5Str == md5String -} - func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, artifact *actions.ActionArtifact, contentSize, runID int64, @@ -264,13 +244,11 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, storagePath := fmt.Sprintf("tmp%d/%d-%d-%d.chunk", runID, artifact.ID, start, end) - // use hashReader to avoid reading all body to md5 sum. + // use io.TeeReader to avoid reading all body to md5 sum. // it writes data to hasher after reading end // if hash is not matched, delete the read-end result - r := &hashReader{ - Reader: ctx.Req.Body, - Hasher: md5.New(), - } + hasher := md5.New() + r := io.TeeReader(ctx.Req.Body, hasher) // save chunk to storage if _, err := ar.fs.Save(storagePath, r, -1); err != nil { @@ -278,7 +256,10 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, } // check md5 - if !r.Match(ctx.Req.Header.Get(artifactXActionsResultsMD5Header)) { + reqMd5String := ctx.Req.Header.Get(artifactXActionsResultsMD5Header) + chunkMd5String := base64.StdEncoding.EncodeToString(hasher.Sum(nil)) + log.Debug("[artifact] check chunk md5, sum: %s, header: %s", chunkMd5String, reqMd5String) + if reqMd5String != chunkMd5String { if err := ar.fs.Delete(storagePath); err != nil { log.Error("Error deleting chunk: %s, %v", storagePath, err) } @@ -291,19 +272,8 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, return length, nil } -var invalidArtifactNameChars = []string{"\\", "/", "\"", ":", "<", ">", "|", "*", "?", "\r", "\n"} - -// checkArtifactName checks if the artifact name contains invalid characters. -// If the name contains invalid characters, an error is returned. // The rules are from https://github.com/actions/toolkit/blob/main/packages/artifact/src/internal/path-and-artifact-name-validation.ts#L32 -func checkArtifactName(name string) error { - for _, c := range invalidArtifactNameChars { - if strings.Contains(name, c) { - return fmt.Errorf("artifact name contains invalid character %s", c) - } - } - return nil -} +var invalidArtifactNameChars = strings.Join([]string{"\\", "/", "\"", ":", "<", ">", "|", "*", "?", "\r", "\n"}, "") func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { artifactID := ctx.ParamsInt64("artifactID") @@ -311,7 +281,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { artifact, err := actions.GetArtifactByID(ctx, artifactID) if err != nil { log.Error("Error getting artifact: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.Error(http.StatusNotFound, err.Error()) return } @@ -320,8 +290,11 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { itemPath := util.PathJoinRel(ctx.Req.URL.Query().Get("itemPath")) runID := ctx.ParamsInt64("runID") artifactName := strings.Split(itemPath, "/")[0] - if err = checkArtifactName(artifactName); err != nil { - log.Error("Error checking artifact name: %v", err) + + // checkArtifactName checks if the artifact name contains invalid characters. + // If the name contains invalid characters, an error is returned. + if strings.ContainsAny(artifactName, invalidArtifactNameChars) { + log.Error("Error checking artifact name contains invalid character") ctx.Error(http.StatusInternalServerError, err.Error()) return } @@ -485,6 +458,17 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) return nil } +type ( + listArtifactsResponse struct { + Count int64 `json:"count"` + Value []listArtifactsResponseItem `json:"value"` + } + listArtifactsResponseItem struct { + Name string `json:"name"` + FileContainerResourceURL string `json:"fileContainerResourceUrl"` + } +) + func (ar artifactRoutes) listArtifacts(ctx *context.Context) { task, ok := ctx.Data["task"].(*actions.ActionTask) if !ok { @@ -506,7 +490,7 @@ func (ar artifactRoutes) listArtifacts(ctx *context.Context) { return } - artficatsData := make([]map[string]interface{}, 0, len(artficats)) + artficatsData := make([]listArtifactsResponseItem, 0, len(artficats)) for _, a := range artficats { url, err := ar.buildArtifactURL(runID, a.ID, "path") if err != nil { @@ -514,24 +498,35 @@ func (ar artifactRoutes) listArtifacts(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - artficatsData = append(artficatsData, map[string]interface{}{ - "name": a.ArtifactName, - "fileContainerResourceUrl": url, + artficatsData = append(artficatsData, listArtifactsResponseItem{ + Name: a.ArtifactName, + FileContainerResourceURL: url, }) } - respData := map[string]interface{}{ - "count": len(artficatsData), - "value": artficatsData, + respData := listArtifactsResponse{ + Count: int64(len(artficatsData)), + Value: artficatsData, } ctx.JSON(http.StatusOK, respData) } +type ( + downloadArtifactResponse struct { + Value []downloadArtifactResponseItem `json:"value"` + } + downloadArtifactResponseItem struct { + Path string `json:"path"` + ItemType string `json:"itemType"` + ContentLocation string `json:"contentLocation"` + } +) + func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { artifactID := ctx.ParamsInt64("artifactID") artifact, err := actions.GetArtifactByID(ctx, artifactID) if err != nil { log.Error("Error getting artifact: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.Error(http.StatusNotFound, err.Error()) return } runID := ctx.ParamsInt64("runID") @@ -542,13 +537,12 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { return } itemPath := util.PathJoinRel(ctx.Req.URL.Query().Get("itemPath")) - artifactData := map[string]string{ - "path": util.PathJoinRel(itemPath, artifact.ArtifactPath), - "itemType": "file", - "contentLocation": url, - } - respData := map[string]interface{}{ - "value": []interface{}{artifactData}, + respData := downloadArtifactResponse{ + Value: []downloadArtifactResponseItem{{ + Path: util.PathJoinRel(itemPath, artifact.ArtifactPath), + ItemType: "file", + ContentLocation: url, + }}, } ctx.JSON(http.StatusOK, respData) } @@ -558,7 +552,7 @@ func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { artifact, err := actions.GetArtifactByID(ctx, artifactID) if err != nil { log.Error("Error getting artifact: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.Error(http.StatusNotFound, err.Error()) return } runID := ctx.ParamsInt64("runID") @@ -567,19 +561,20 @@ func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } + fd, err := ar.fs.Open(artifact.StoragePath) if err != nil { log.Error("Error opening file: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) return } + defer fd.Close() + if strings.HasSuffix(artifact.ArtifactPath, ".gz") { ctx.Resp.Header().Set("Content-Encoding", "gzip") } - _, err = io.Copy(ctx.Resp, fd) - if err != nil { - log.Error("Error copying file to response: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } + ctx.ServeContent(fd, &context.ServeHeaderOptions{ + Filename: artifact.ArtifactName, + LastModified: artifact.Created.AsLocalTime(), + }) } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index c7635ed5a3f9d..c2f99eb6a1628 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -397,6 +397,10 @@ func ArtifactsView(ctx *context_module.Context) { runIndex := ctx.ParamsInt64("run") run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.Error(http.StatusNotFound, err.Error()) + return + } ctx.Error(http.StatusInternalServerError, err.Error()) return } @@ -429,6 +433,10 @@ func ArtifactsDownloadView(ctx *context_module.Context) { } run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.Error(http.StatusNotFound, err.Error()) + return + } ctx.Error(http.StatusInternalServerError, err.Error()) return } diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 666f860be8653..4ad4b19503e73 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -172,7 +172,7 @@ const sfc = { // clear the interval timer when the component is unmounted // even our page is rendered once, not spa style if (this.intervalID) { - clearInterval(this.intervalID) + clearInterval(this.intervalID); this.intervalID = null; } }, From eb6c55e8941f8300737d550864ea2b205f79f8d3 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 28 Apr 2023 15:33:43 +0800 Subject: [PATCH 30/38] update artifact chunk naming params, update buildArtifactURL method --- routers/api/actions/artifacts.go | 53 +++++++++++--------------------- 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 275926eff9f85..0a9dd0eda68b8 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -69,7 +69,6 @@ import ( "fmt" "io" "net/http" - "net/url" "sort" "strconv" "strings" @@ -163,15 +162,11 @@ type artifactRoutes struct { fs storage.ObjectStorage } -func (ar artifactRoutes) buildArtifactURL(runID, artifactID int64, suffix string) (string, error) { +func (ar artifactRoutes) buildArtifactURL(runID, artifactID int64, suffix string) string { uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") + strings.ReplaceAll(artifactRouteBase, "{runID}", strconv.FormatInt(runID, 10)) + "/" + strconv.FormatInt(artifactID, 10) + "/" + suffix - u, err := url.Parse(uploadURL) - if err != nil { - return "", err - } - return u.String(), nil + return uploadURL } type getUploadArtifactRequest struct { @@ -179,6 +174,10 @@ type getUploadArtifactRequest struct { Name string } +type getUploadArtifactResponse struct { + FileContainerResourceURL string `json:"fileContainerResourceUrl"` +} + // getUploadArtifactURL generates a URL for uploading an artifact func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { task, ok := ctx.Data["task"].(*actions.ActionTask) @@ -207,18 +206,11 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - url, err := ar.buildArtifactURL(runID, artifact.ID, "upload") - if err != nil { - log.Error("Error parsing upload URL: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) - return + resp := getUploadArtifactResponse{ + FileContainerResourceURL: ar.buildArtifactURL(runID, artifact.ID, "upload"), } - - log.Debug("[artifact] get upload url: %s, artifact id: %d", url, artifact.ID) - - ctx.JSON(http.StatusOK, map[string]interface{}{ - "fileContainerResourceUrl": url, - }) + log.Debug("[artifact] get upload url: %s, artifact id: %d", resp.FileContainerResourceURL, artifact.ID) + ctx.JSON(http.StatusOK, resp) } // getUploadFileSize returns the size of the file to be uploaded. @@ -407,12 +399,14 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) // use multiReader readers := make([]io.Reader, 0, len(allChunks)) + readerClosers := make([]io.Closer, 0, len(allChunks)) for _, c := range allChunks { reader, err := ar.fs.Open(c.Path) if err != nil { return fmt.Errorf("open chunk error: %v, %s", err, c.Path) } readers = append(readers, reader) + readerClosers = append(readerClosers, reader) } mergedReader := io.MultiReader(readers...) @@ -426,7 +420,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) } // save merged file - storagePath := fmt.Sprintf("%d/%d/%d.chunk", (runID+artifactID)%255, artifact.FileSize%255, time.Now().UnixNano()) + storagePath := fmt.Sprintf("%d/%d/%d.chunk", runID%255, artifactID%255, time.Now().UnixNano()) written, err := ar.fs.Save(storagePath, mergedReader, -1) if err != nil { return fmt.Errorf("save merged file error: %v", err) @@ -436,8 +430,8 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) } // close readers - for _, r := range readers { - r.(io.Closer).Close() + for _, r := range readerClosers { + r.Close() } // save storage path to artifact @@ -492,15 +486,9 @@ func (ar artifactRoutes) listArtifacts(ctx *context.Context) { artficatsData := make([]listArtifactsResponseItem, 0, len(artficats)) for _, a := range artficats { - url, err := ar.buildArtifactURL(runID, a.ID, "path") - if err != nil { - log.Error("Error parsing artifact URL: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } artficatsData = append(artficatsData, listArtifactsResponseItem{ Name: a.ArtifactName, - FileContainerResourceURL: url, + FileContainerResourceURL: ar.buildArtifactURL(runID, a.ID, "path"), }) } respData := listArtifactsResponse{ @@ -530,18 +518,13 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { return } runID := ctx.ParamsInt64("runID") - url, err := ar.buildArtifactURL(runID, artifact.ID, "download") - if err != nil { - log.Error("Error parsing download URL: %v", err) - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } + downloadURL := ar.buildArtifactURL(runID, artifact.ID, "download") itemPath := util.PathJoinRel(ctx.Req.URL.Query().Get("itemPath")) respData := downloadArtifactResponse{ Value: []downloadArtifactResponseItem{{ Path: util.PathJoinRel(itemPath, artifact.ArtifactPath), ItemType: "file", - ContentLocation: url, + ContentLocation: downloadURL, }}, } ctx.JSON(http.StatusOK, respData) From 65c41533dea5d5700aa5d7ce43d99809bef95742 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Tue, 9 May 2023 21:47:14 +0800 Subject: [PATCH 31/38] begin artifact api integration tests --- models/fixtures/access_token.yml | 2 +- models/fixtures/action_run.yml | 20 +++++++ models/fixtures/action_run_job.yml | 19 +++++++ models/fixtures/action_task.yml | 23 ++++++++ .../integration/api_actions_artifact_test.go | 55 +++++++++++++++++++ tests/mssql.ini.tmpl | 3 + tests/mysql.ini.tmpl | 3 + tests/mysql8.ini.tmpl | 3 + tests/pgsql.ini.tmpl | 3 + tests/sqlite.ini.tmpl | 3 + 10 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 models/fixtures/action_run.yml create mode 100644 models/fixtures/action_run_job.yml create mode 100644 models/fixtures/action_task.yml create mode 100644 tests/integration/api_actions_artifact_test.go diff --git a/models/fixtures/access_token.yml b/models/fixtures/access_token.yml index 5ff5d66f662ed..791b3da1c459f 100644 --- a/models/fixtures/access_token.yml +++ b/models/fixtures/access_token.yml @@ -30,4 +30,4 @@ token_last_eight: 69d28c91 created_unix: 946687980 updated_unix: 946687980 -#commented out tokens so you can see what they are in plaintext \ No newline at end of file +#commented out tokens so you can see what they are in plaintext diff --git a/models/fixtures/action_run.yml b/models/fixtures/action_run.yml new file mode 100644 index 0000000000000..ebcd03230da6c --- /dev/null +++ b/models/fixtures/action_run.yml @@ -0,0 +1,20 @@ +- + id: 791 + title: "update actions" + repo_id: 4 + owner_id: 1 + workflow_id: "artifact.yaml" + index: 187 + trigger_user_id: 1 + ref: "refs/heads/master" + commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + event: "push" + is_fork_pull_request: 0 + event_payload: "" + status: 1 + started: 1683636528 + stopped: 1683636626 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml new file mode 100644 index 0000000000000..1df0f0411008d --- /dev/null +++ b/models/fixtures/action_run_job.yml @@ -0,0 +1,19 @@ +- + id: 192 + run_id: 791 + repo_id: 4 + owner_id: 1 + commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + is_fork_pull_request: 0 + name: "job_2" + attempt: 1 + workflow_payload: "" + job_id: "job_2" + needs: "" + runs_on: "[\"ubuntu-latest\"]" + task_id: 47 + status: 1 + started: 1683636528 + stopped: 1683636626 + created: 1683636108 + updated: 1683636626 diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml new file mode 100644 index 0000000000000..6d2500ee36aca --- /dev/null +++ b/models/fixtures/action_task.yml @@ -0,0 +1,23 @@ +- + id: 47 + job_id: 192 + attempt: 3 + runner_id: 1 + status: 6 # 6 is the status code for "running", running task can upload artifacts + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + is_fork_pull_request: 0 + token_hash: "6d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2a867e" + token_salt: "jVuKnSPGgy" + token_last_eight: "eeb1a71a" + log_filename: "artifact-test2/2f/47.log" + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_indexes: "" + log_expired: 0 + created: 1683636528 + updated: 1683646528 diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go new file mode 100644 index 0000000000000..16518af9ca28d --- /dev/null +++ b/tests/integration/api_actions_artifact_test.go @@ -0,0 +1,55 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/http" + "strings" + "testing" + + "code.gitea.io/gitea/tests" + "github.com/stretchr/testify/assert" +) + +func TestArtifactsUpload(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + type uploadArtifactResponse struct { + FileContainerResourceUrl string `json:"fileContainerResourceUrl"` + } + + type getUploadArtifactRequest struct { + Type string + Name string + } + + // acquire artifact upload url + req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts", getUploadArtifactRequest{ + Type: "actions_storage", + Name: "artifact", + }) + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp := MakeRequest(t, req, http.StatusOK) + var uploadResp uploadArtifactResponse + DecodeJSON(t, resp, &uploadResp) + assert.Contains(t, uploadResp.FileContainerResourceUrl, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + + // get upload url + idx := strings.Index(uploadResp.FileContainerResourceUrl, "/api/actions_pipeline/_apis/pipelines/") + url := uploadResp.FileContainerResourceUrl[idx:] + "?itemPath=artifact/abc.txt" + + // upload artifact chunk + body := strings.Repeat("A", 1024) + req = NewRequestWithBody(t, "PUT", url, strings.NewReader(body)) + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + req.Header.Add("Content-Range", "bytes 0-1023/1024") + req.Header.Add("x-tfs-filelength", "1024") + req.Header.Add("x-actions-results-md5", "1HsSe8LeLWh93ILaw1TEFQ==") // base64(md5(body)) + MakeRequest(t, req, http.StatusOK) + + // confirm artifact upload + req = NewRequest(t, "PATCH", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + MakeRequest(t, req, http.StatusOK) +} diff --git a/tests/mssql.ini.tmpl b/tests/mssql.ini.tmpl index 9cec6169f979b..9d9b5f6a89301 100644 --- a/tests/mssql.ini.tmpl +++ b/tests/mssql.ini.tmpl @@ -108,3 +108,6 @@ PATH = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-mssql/data/lfs [packages] ENABLED = true + +[actions] +ENABLED = true diff --git a/tests/mysql.ini.tmpl b/tests/mysql.ini.tmpl index b286f37bf83e2..fd0fab605805d 100644 --- a/tests/mysql.ini.tmpl +++ b/tests/mysql.ini.tmpl @@ -116,3 +116,6 @@ PASSWORD = debug USE_TLS = true SKIP_TLS_VERIFY = true REPLY_TO_ADDRESS = incoming+%{token}@localhost + +[actions] +ENABLED = true diff --git a/tests/mysql8.ini.tmpl b/tests/mysql8.ini.tmpl index f290efe1dcb19..2e5cfb0fbc758 100644 --- a/tests/mysql8.ini.tmpl +++ b/tests/mysql8.ini.tmpl @@ -105,3 +105,6 @@ PATH = tests/{{TEST_TYPE}}/gitea-{{TEST_TYPE}}-mysql8/data/lfs [packages] ENABLED = true + +[actions] +ENABLED = true diff --git a/tests/pgsql.ini.tmpl b/tests/pgsql.ini.tmpl index 15349aa4c217c..13a7b662d5d0b 100644 --- a/tests/pgsql.ini.tmpl +++ b/tests/pgsql.ini.tmpl @@ -129,3 +129,6 @@ MINIO_CHECKSUM_ALGORITHM = md5 [packages] ENABLED = true + +[actions] +ENABLED = true diff --git a/tests/sqlite.ini.tmpl b/tests/sqlite.ini.tmpl index d2e4a2d5ae5f9..5e6db5023ba88 100644 --- a/tests/sqlite.ini.tmpl +++ b/tests/sqlite.ini.tmpl @@ -114,3 +114,6 @@ FILE_EXTENSIONS = .html RENDER_COMMAND = `go run build/test-echo.go` IS_INPUT_FILE = false RENDER_CONTENT_MODE=sanitized + +[actions] +ENABLED = true From 880b249d7ecbc6499b02e4ffc5b5e35e4ffbd627 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 12 May 2023 10:44:35 +0800 Subject: [PATCH 32/38] fix typo, update lint, update actions router response, update migration --- models/actions/artifact.go | 84 ++++++++-------- models/migrations/v1_20/v257.go | 30 +++--- routers/api/actions/artifacts.go | 99 +++++++++++-------- routers/web/repo/actions/view.go | 8 +- .../integration/api_actions_artifact_test.go | 14 ++- web_src/js/components/RepoActionView.vue | 4 +- 6 files changed, 131 insertions(+), 108 deletions(-) diff --git a/models/actions/artifact.go b/models/actions/artifact.go index a77c1f5b4ad79..0dd4634752168 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -8,6 +8,7 @@ package actions import ( "context" + "errors" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/timeutil" @@ -19,6 +20,8 @@ const ( ArtifactUploadStatusPending = 1 // ArtifactUploadStatusConfirmed is the status of an artifact upload that is confirmed ArtifactUploadStatusConfirmed = 2 + // ArtifactUploadStatusError is the status of an artifact upload that is errored + ArtifactUploadStatusError = 3 ) func init() { @@ -27,62 +30,55 @@ func init() { // ActionArtifact is a file that is stored in the artifact storage. type ActionArtifact struct { - ID int64 `xorm:"pk autoincr"` - RunID int64 `xorm:"index UNIQUE(runid_name)"` // The run id of the artifact - RunnerID int64 - RepoID int64 `xorm:"index"` - OwnerID int64 - CommitSHA string - StoragePath string // The path to the artifact in the storage - FileSize int64 // The size of the artifact in bytes - FileGzipSize int64 // The size of the artifact in bytes after gzip compression - ContentEncnoding string // The content encoding of the artifact - ArtifactPath string // The path to the artifact when runner uploads it - ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it - UploadStatus int64 `xorm:"index"` // The status of the artifact upload - Created timeutil.TimeStamp `xorm:"created"` - Updated timeutil.TimeStamp `xorm:"updated index"` + ID int64 `xorm:"pk autoincr"` + RunID int64 `xorm:"index UNIQUE(runid_name)"` // The run id of the artifact + RunnerID int64 + RepoID int64 `xorm:"index"` + OwnerID int64 + CommitSHA string + StoragePath string // The path to the artifact in the storage + FileSize int64 // The size of the artifact in bytes + FileCompressedSize int64 // The size of the artifact in bytes after gzip compression + ContentEncoding string // The content encoding of the artifact + ArtifactPath string // The path to the artifact when runner uploads it + ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it + UploadStatus int64 `xorm:"index"` // The status of the artifact upload + CreatedUnix timeutil.TimeStamp `xorm:"created"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated index"` } // CreateArtifact create a new artifact with task info or get same named artifact in the same run func CreateArtifact(ctx context.Context, t *ActionTask, artifactName string) (*ActionArtifact, error) { - if t.Job == nil { - if err := t.LoadJob(ctx); err != nil { - return nil, err - } - } - if err := t.Job.LoadRun(ctx); err != nil { - return nil, err - } - artifact, err := GetArtifactByArtifactName(ctx, t.Job.RunID, artifactName) - if err != nil { + if err := t.LoadJob(ctx); err != nil { return nil, err } - if artifact != nil { + artifact, err := getArtifactByArtifactName(ctx, t.Job.RunID, artifactName) + if errors.Is(err, util.ErrNotExist) { + artifact := &ActionArtifact{ + RunID: t.Job.RunID, + RunnerID: t.RunnerID, + RepoID: t.RepoID, + OwnerID: t.OwnerID, + CommitSHA: t.CommitSHA, + UploadStatus: ArtifactUploadStatusPending, + } + if _, err := db.GetEngine(ctx).Insert(artifact); err != nil { + return nil, err + } return artifact, nil - } - artifact = &ActionArtifact{ - RunID: t.Job.RunID, - RunnerID: t.RunnerID, - RepoID: t.RepoID, - OwnerID: t.OwnerID, - CommitSHA: t.CommitSHA, - UploadStatus: ArtifactUploadStatusPending, - } - if _, err := db.GetEngine(ctx).Insert(artifact); err != nil { + } else if err != nil { return nil, err } return artifact, nil } -// GetArtifactByArtifactName returns an artifact by name -func GetArtifactByArtifactName(ctx context.Context, runID int64, name string) (*ActionArtifact, error) { +func getArtifactByArtifactName(ctx context.Context, runID int64, name string) (*ActionArtifact, error) { var art ActionArtifact has, err := db.GetEngine(ctx).Where("run_id = ? AND artifact_name = ?", runID, name).Get(&art) if err != nil { return nil, err } else if !has { - return nil, nil + return nil, util.ErrNotExist } return &art, nil } @@ -94,7 +90,7 @@ func GetArtifactByID(ctx context.Context, id int64) (*ActionArtifact, error) { if err != nil { return nil, err } else if !has { - return nil, util.NewNotExistErrorf("no ActionArtifact with id %d exists", id) + return nil, util.ErrNotExist } return &art, nil @@ -107,14 +103,14 @@ func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) erro return err } -// ListArtifactByRunID returns all artifacts of a run -func ListArtifactByRunID(ctx context.Context, runID int64) ([]*ActionArtifact, error) { +// ListArtifactsByRunID returns all artifacts of a run +func ListArtifactsByRunID(ctx context.Context, runID int64) ([]*ActionArtifact, error) { arts := make([]*ActionArtifact, 0, 10) return arts, db.GetEngine(ctx).Where("run_id=?", runID).Find(&arts) } -// ListUploadedArtifactByRunID returns all uploaded artifacts of a run -func ListUploadedArtifactByRunID(ctx context.Context, runID int64) ([]*ActionArtifact, error) { +// ListUploadedArtifactsByRunID returns all uploaded artifacts of a run +func ListUploadedArtifactsByRunID(ctx context.Context, runID int64) ([]*ActionArtifact, error) { arts := make([]*ActionArtifact, 0, 10) return arts, db.GetEngine(ctx).Where("run_id=? AND upload_status=?", runID, ArtifactUploadStatusConfirmed).Find(&arts) } diff --git a/models/migrations/v1_20/v257.go b/models/migrations/v1_20/v257.go index e1869866e8017..5d49be36d6baa 100644 --- a/models/migrations/v1_20/v257.go +++ b/models/migrations/v1_20/v257.go @@ -12,21 +12,21 @@ import ( func CreateActionArtifactTable(x *xorm.Engine) error { // ActionArtifact is a file that is stored in the artifact storage. type ActionArtifact struct { - ID int64 `xorm:"pk autoincr"` - RunID int64 `xorm:"index UNIQUE(runid_name)"` // The run id of the artifact - RunnerID int64 - RepoID int64 `xorm:"index"` - OwnerID int64 - CommitSHA string - StoragePath string // The path to the artifact in the storage - FileSize int64 // The size of the artifact in bytes - FileGzipSize int64 // The size of the artifact in bytes after gzip compression - ContentEncnoding string // The content encoding of the artifact - ArtifactPath string // The path to the artifact when runner uploads it - ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it - UploadStatus int64 `xorm:"index"` // The status of the artifact upload - Created timeutil.TimeStamp `xorm:"created"` - Updated timeutil.TimeStamp `xorm:"updated index"` + ID int64 `xorm:"pk autoincr"` + RunID int64 `xorm:"index UNIQUE(runid_name)"` // The run id of the artifact + RunnerID int64 + RepoID int64 `xorm:"index"` + OwnerID int64 + CommitSHA string + StoragePath string // The path to the artifact in the storage + FileSize int64 // The size of the artifact in bytes + FileCompressedSize int64 // The size of the artifact in bytes after gzip compression + ContentEncoding string // The content encoding of the artifact + ArtifactPath string // The path to the artifact when runner uploads it + ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it + UploadStatus int64 `xorm:"index"` // The status of the artifact upload + CreatedUnix timeutil.TimeStamp `xorm:"created"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated index"` } return x.Sync(new(ActionArtifact)) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 0a9dd0eda68b8..7f0589d48fe52 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -17,7 +17,7 @@ package actions // { // "fileContainerResourceUrl":"/api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/upload" // } -// it accquire a upload url for upload artifact +// it acquires an upload url for artifact upload // 1.2. Upload artifact // PUT: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/upload?itemPath=artifact%2Ffilename // it upload chunk with headers: @@ -66,6 +66,7 @@ import ( gocontext "context" "crypto/md5" "encoding/base64" + "errors" "fmt" "io" "net/http" @@ -89,8 +90,6 @@ const ( artifactXActionsResultsMD5Header = "x-actions-results-md5" ) -// const artifactXActionsResultsCRC64Header = "x-actions-results-crc64" - const artifactRouteBase = "/_apis/pipelines/workflows/{runID}/artifacts" func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { @@ -142,12 +141,10 @@ func withContexter(goctx gocontext.Context) func(next http.Handler) http.Handler } ctx.Data["task"] = task - if task.Job == nil { - if err := task.LoadJob(goctx); err != nil { - log.Error("Error runner api getting job: %v", err) - ctx.Error(http.StatusInternalServerError, "Error runner api getting job") - return - } + if err := task.LoadJob(goctx); err != nil { + log.Error("Error runner api getting job: %v", err) + ctx.Error(http.StatusInternalServerError, "Error runner api getting job") + return } ctx.Req = context.WithContext(req, &ctx) @@ -178,18 +175,26 @@ type getUploadArtifactResponse struct { FileContainerResourceURL string `json:"fileContainerResourceUrl"` } -// getUploadArtifactURL generates a URL for uploading an artifact -func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { +func (ar artifactRoutes) validateRunID(ctx *context.Context) (*actions.ActionTask, int64, bool) { task, ok := ctx.Data["task"].(*actions.ActionTask) if !ok { log.Error("Error getting task in context") ctx.Error(http.StatusInternalServerError, "Error getting task in context") - return + return nil, 0, false } runID := ctx.ParamsInt64("runID") if task.Job.RunID != runID { log.Error("Error runID not match") - ctx.Error(http.StatusInternalServerError, "Error runID not match") + ctx.Error(http.StatusBadRequest, "run-id does not match") + return nil, 0, false + } + return task, runID, true +} + +// getUploadArtifactURL generates a URL for uploading an artifact +func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) { + task, runID, ok := ar.validateRunID(ctx) + if !ok { return } @@ -243,7 +248,8 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, r := io.TeeReader(ctx.Req.Body, hasher) // save chunk to storage - if _, err := ar.fs.Save(storagePath, r, -1); err != nil { + writtenSize, err := ar.fs.Save(storagePath, r, -1) + if err != nil { return -1, fmt.Errorf("save chunk to storage error: %v", err) } @@ -251,7 +257,7 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, reqMd5String := ctx.Req.Header.Get(artifactXActionsResultsMD5Header) chunkMd5String := base64.StdEncoding.EncodeToString(hasher.Sum(nil)) log.Debug("[artifact] check chunk md5, sum: %s, header: %s", chunkMd5String, reqMd5String) - if reqMd5String != chunkMd5String { + if reqMd5String != chunkMd5String || writtenSize != contentSize { if err := ar.fs.Delete(storagePath); err != nil { log.Error("Error deleting chunk: %s, %v", storagePath, err) } @@ -268,26 +274,32 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context, var invalidArtifactNameChars = strings.Join([]string{"\\", "/", "\"", ":", "<", ">", "|", "*", "?", "\r", "\n"}, "") func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { + _, runID, ok := ar.validateRunID(ctx) + if !ok { + return + } artifactID := ctx.ParamsInt64("artifactID") artifact, err := actions.GetArtifactByID(ctx, artifactID) - if err != nil { + if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusNotFound, err.Error()) + } else if err != nil { + log.Error("Error getting artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) return } // itemPath is generated from upload-artifact action // it's formatted as {artifact_name}/{artfict_path_in_runner} itemPath := util.PathJoinRel(ctx.Req.URL.Query().Get("itemPath")) - runID := ctx.ParamsInt64("runID") artifactName := strings.Split(itemPath, "/")[0] // checkArtifactName checks if the artifact name contains invalid characters. // If the name contains invalid characters, an error is returned. if strings.ContainsAny(artifactName, invalidArtifactNameChars) { log.Error("Error checking artifact name contains invalid character") - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.Error(http.StatusBadRequest, err.Error()) return } @@ -312,8 +324,8 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { artifact.ArtifactName = artifactName artifact.ArtifactPath = itemPath // path in container artifact.FileSize = fileSize // this is total size of all chunks - artifact.FileGzipSize = chunkAllLength - artifact.ContentEncnoding = ctx.Req.Header.Get("Content-Encoding") + artifact.FileCompressedSize = chunkAllLength + artifact.ContentEncoding = ctx.Req.Header.Get("Content-Encoding") if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { log.Error("Error updating artifact: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -329,7 +341,10 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { // comfirmUploadArtifact comfirm upload artifact. // if all chunks are uploaded, merge them to one file. func (ar artifactRoutes) comfirmUploadArtifact(ctx *context.Context) { - runID := ctx.ParamsInt64("runID") + _, runID, ok := ar.validateRunID(ctx) + if !ok { + return + } if err := ar.mergeArtifactChunks(ctx, runID); err != nil { log.Error("Error merging chunks: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -392,7 +407,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) } // if the last chunk.End + 1 is not equal to chunk.ChunkLength, means chunks are not uploaded completely - if startAt+1 != artifact.FileGzipSize { + if startAt+1 != artifact.FileCompressedSize { log.Debug("[artifact] chunks are not uploaded completely, artifact_id: %d", artifactID) break } @@ -411,7 +426,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) mergedReader := io.MultiReader(readers...) // if chunk is gzip, decompress it - if artifact.ContentEncnoding == "gzip" { + if artifact.ContentEncoding == "gzip" { var err error mergedReader, err = gzip.NewReader(mergedReader) if err != nil { @@ -464,20 +479,12 @@ type ( ) func (ar artifactRoutes) listArtifacts(ctx *context.Context) { - task, ok := ctx.Data["task"].(*actions.ActionTask) + _, runID, ok := ar.validateRunID(ctx) if !ok { - log.Error("Error getting task in context") - ctx.Error(http.StatusInternalServerError, "Error getting task in context") - return - } - runID := ctx.ParamsInt64("runID") - if task.Job.RunID != runID { - log.Error("Error runID not match") - ctx.Error(http.StatusInternalServerError, "Error runID not match") return } - artficats, err := actions.ListArtifactByRunID(ctx, runID) + artficats, err := actions.ListArtifactsByRunID(ctx, runID) if err != nil { log.Error("Error getting artifacts: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -510,14 +517,21 @@ type ( ) func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { + _, runID, ok := ar.validateRunID(ctx) + if !ok { + return + } + artifactID := ctx.ParamsInt64("artifactID") artifact, err := actions.GetArtifactByID(ctx, artifactID) - if err != nil { + if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusNotFound, err.Error()) + } else if err != nil { + log.Error("Error getting artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) return } - runID := ctx.ParamsInt64("runID") downloadURL := ar.buildArtifactURL(runID, artifact.ID, "download") itemPath := util.PathJoinRel(ctx.Req.URL.Query().Get("itemPath")) respData := downloadArtifactResponse{ @@ -531,17 +545,24 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { } func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { + _, runID, ok := ar.validateRunID(ctx) + if !ok { + return + } + artifactID := ctx.ParamsInt64("artifactID") artifact, err := actions.GetArtifactByID(ctx, artifactID) - if err != nil { + if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusNotFound, err.Error()) + } else if err != nil { + log.Error("Error getting artifact: %v", err) + ctx.Error(http.StatusInternalServerError, err.Error()) return } - runID := ctx.ParamsInt64("runID") if artifact.RunID != runID { log.Error("Error dismatch runID and artifactID, task: %v, artifact: %v", runID, artifactID) - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.Error(http.StatusBadRequest, err.Error()) return } @@ -558,6 +579,6 @@ func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { } ctx.ServeContent(fd, &context.ServeHeaderOptions{ Filename: artifact.ArtifactName, - LastModified: artifact.Created.AsLocalTime(), + LastModified: artifact.CreatedUnix.AsLocalTime(), }) } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 210aef74afb33..211433861295e 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -441,7 +441,7 @@ func ArtifactsView(ctx *context_module.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - artifacts, err := actions_model.ListUploadedArtifactByRunID(ctx, run.ID) + artifacts, err := actions_model.ListUploadedArtifactsByRunID(ctx, run.ID) if err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return @@ -464,7 +464,9 @@ func ArtifactsDownloadView(ctx *context_module.Context) { artifactID := ctx.ParamsInt64("id") artifact, err := actions_model.GetArtifactByID(ctx, artifactID) - if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.Error(http.StatusNotFound, err.Error()) + } else if err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return } @@ -491,6 +493,6 @@ func ArtifactsDownloadView(ctx *context_module.Context) { ctx.ServeContent(f, &context_module.ServeHeaderOptions{ Filename: artifact.ArtifactName, - LastModified: artifact.Created.AsLocalTime(), + LastModified: artifact.CreatedUnix.AsLocalTime(), }) } diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 16518af9ca28d..66f988521e955 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -16,7 +16,7 @@ func TestArtifactsUpload(t *testing.T) { defer tests.PrepareTestEnv(t)() type uploadArtifactResponse struct { - FileContainerResourceUrl string `json:"fileContainerResourceUrl"` + FileContainerResourceURL string `json:"fileContainerResourceUrl"` } type getUploadArtifactRequest struct { @@ -24,6 +24,8 @@ func TestArtifactsUpload(t *testing.T) { Name string } + t.Logf("Create artifact request") + // acquire artifact upload url req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts", getUploadArtifactRequest{ Type: "actions_storage", @@ -33,11 +35,13 @@ func TestArtifactsUpload(t *testing.T) { resp := MakeRequest(t, req, http.StatusOK) var uploadResp uploadArtifactResponse DecodeJSON(t, resp, &uploadResp) - assert.Contains(t, uploadResp.FileContainerResourceUrl, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + assert.Contains(t, uploadResp.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + + t.Logf("upload artifact request") // get upload url - idx := strings.Index(uploadResp.FileContainerResourceUrl, "/api/actions_pipeline/_apis/pipelines/") - url := uploadResp.FileContainerResourceUrl[idx:] + "?itemPath=artifact/abc.txt" + idx := strings.Index(uploadResp.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") + url := uploadResp.FileContainerResourceURL[idx:] + "?itemPath=artifact/abc.txt" // upload artifact chunk body := strings.Repeat("A", 1024) @@ -48,6 +52,8 @@ func TestArtifactsUpload(t *testing.T) { req.Header.Add("x-actions-results-md5", "1HsSe8LeLWh93ILaw1TEFQ==") // base64(md5(body)) MakeRequest(t, req, http.StatusOK) + t.Logf("Create artifact confirm") + // confirm artifact upload req = NewRequest(t, "PATCH", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index ae5d56533ecb6..2f77bf3a2f1dc 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -110,8 +110,6 @@ const sfc = { data() { return { - ansiToHTML: new AnsiToHTML({escapeXML: true}), - // internal state loading: false, intervalID: null, @@ -283,7 +281,7 @@ const sfc = { try { this.loading = true; - // refresh artitfact list if upload-artifact step done + // refresh artifacts if upload-artifact step done const resp = await this.fetchPost(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); const artifacts = await resp.json(); this.artifacts = artifacts['artifacts'] || []; From 6a60390f6b9e0b318c6c032bbe59fd5382227057 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 12 May 2023 22:16:03 +0800 Subject: [PATCH 33/38] update artifact integration tests --- models/fixtures/action_task.yml | 12 ++- routers/api/actions/artifacts.go | 3 + .../integration/api_actions_artifact_test.go | 89 ++++++++++++++++++- 3 files changed, 93 insertions(+), 11 deletions(-) diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index 6d2500ee36aca..26c986349a4ae 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -8,16 +8,14 @@ stopped: 1683636626 repo_id: 4 owner_id: 1 - commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 - token_hash: "6d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2a867e" - token_salt: "jVuKnSPGgy" - token_last_eight: "eeb1a71a" - log_filename: "artifact-test2/2f/47.log" + token_hash: 6d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2a867e + token_salt: jVuKnSPGgy + token_last_eight: eeb1a71a + log_filename: artifact-test2/2f/47.log log_in_storage: 1 log_length: 707 log_size: 90179 log_indexes: "" log_expired: 0 - created: 1683636528 - updated: 1683646528 diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 7f0589d48fe52..732648277805a 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -284,6 +284,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusNotFound, err.Error()) + return } else if err != nil { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -527,6 +528,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusNotFound, err.Error()) + return } else if err != nil { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) @@ -555,6 +557,7 @@ func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusNotFound, err.Error()) + return } else if err != nil { log.Error("Error getting artifact: %v", err) ctx.Error(http.StatusInternalServerError, err.Error()) diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 66f988521e955..c60a49b618ade 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -24,8 +24,6 @@ func TestArtifactsUpload(t *testing.T) { Name string } - t.Logf("Create artifact request") - // acquire artifact upload url req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts", getUploadArtifactRequest{ Type: "actions_storage", @@ -37,8 +35,6 @@ func TestArtifactsUpload(t *testing.T) { DecodeJSON(t, resp, &uploadResp) assert.Contains(t, uploadResp.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") - t.Logf("upload artifact request") - // get upload url idx := strings.Index(uploadResp.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") url := uploadResp.FileContainerResourceURL[idx:] + "?itemPath=artifact/abc.txt" @@ -59,3 +55,88 @@ func TestArtifactsUpload(t *testing.T) { req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") MakeRequest(t, req, http.StatusOK) } + +func TestArtifactsUploadNotExist(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // artifact id 54321 not exist + url := "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts/54321/upload?itemPath=artifact/abc.txt" + body := strings.Repeat("A", 1024) + req := NewRequestWithBody(t, "PUT", url, strings.NewReader(body)) + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + req.Header.Add("Content-Range", "bytes 0-1023/1024") + req.Header.Add("x-tfs-filelength", "1024") + req.Header.Add("x-actions-results-md5", "1HsSe8LeLWh93ILaw1TEFQ==") // base64(md5(body)) + MakeRequest(t, req, http.StatusNotFound) +} + +func TestArtifactConfirmUpload(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + req := NewRequest(t, "PATCH", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp := MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "success") +} + +func TestArtifactUploadWithoutToken(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/1/artifacts", nil) + MakeRequest(t, req, http.StatusUnauthorized) +} + +func TestArtifactDownload(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + type ( + listArtifactsResponseItem struct { + Name string `json:"name"` + FileContainerResourceURL string `json:"fileContainerResourceUrl"` + } + listArtifactsResponse struct { + Count int64 `json:"count"` + Value []listArtifactsResponseItem `json:"value"` + } + ) + + req := NewRequest(t, "GET", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp := MakeRequest(t, req, http.StatusOK) + var listResp listArtifactsResponse + DecodeJSON(t, resp, &listResp) + assert.Equal(t, int64(1), listResp.Count) + assert.Equal(t, "artifact", listResp.Value[0].Name) + assert.Contains(t, listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + + type ( + downloadArtifactResponseItem struct { + Path string `json:"path"` + ItemType string `json:"itemType"` + ContentLocation string `json:"contentLocation"` + } + downloadArtifactResponse struct { + Value []downloadArtifactResponseItem `json:"value"` + } + ) + + idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") + url := listResp.Value[0].FileContainerResourceURL[idx:] + req = NewRequest(t, "GET", url) + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp = MakeRequest(t, req, http.StatusOK) + var downloadResp downloadArtifactResponse + DecodeJSON(t, resp, &downloadResp) + assert.Len(t, downloadResp.Value, 1) + assert.Equal(t, "artifact/abc.txt", downloadResp.Value[0].Path) + assert.Equal(t, "file", downloadResp.Value[0].ItemType) + assert.Contains(t, downloadResp.Value[0].ContentLocation, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") + + idx = strings.Index(downloadResp.Value[0].ContentLocation, "/api/actions_pipeline/_apis/pipelines/") + url = downloadResp.Value[0].ContentLocation[idx:] + req = NewRequest(t, "GET", url) + req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("A", 1024) + assert.Equal(t, resp.Body.String(), body) +} From 6bb55805ee102a1845e41c17ed32f56edf6ac8e8 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 12 May 2023 23:17:15 +0800 Subject: [PATCH 34/38] fix artifact integration tests failure --- models/fixtures/action_run_job.yml | 11 +++-------- tests/integration/api_actions_artifact_test.go | 11 ++++++----- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index 1df0f0411008d..071998b9796f9 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -3,17 +3,12 @@ run_id: 791 repo_id: 4 owner_id: 1 - commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 - name: "job_2" + name: job_2 attempt: 1 - workflow_payload: "" - job_id: "job_2" - needs: "" - runs_on: "[\"ubuntu-latest\"]" + job_id: job_2 task_id: 47 status: 1 started: 1683636528 stopped: 1683636626 - created: 1683636108 - updated: 1683636626 diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index c60a49b618ade..87570b8d8b697 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -9,10 +9,11 @@ import ( "testing" "code.gitea.io/gitea/tests" + "github.com/stretchr/testify/assert" ) -func TestArtifactsUpload(t *testing.T) { +func TestActionsArtifactUpload(t *testing.T) { defer tests.PrepareTestEnv(t)() type uploadArtifactResponse struct { @@ -56,7 +57,7 @@ func TestArtifactsUpload(t *testing.T) { MakeRequest(t, req, http.StatusOK) } -func TestArtifactsUploadNotExist(t *testing.T) { +func TestActionsArtifactUploadNotExist(t *testing.T) { defer tests.PrepareTestEnv(t)() // artifact id 54321 not exist @@ -70,7 +71,7 @@ func TestArtifactsUploadNotExist(t *testing.T) { MakeRequest(t, req, http.StatusNotFound) } -func TestArtifactConfirmUpload(t *testing.T) { +func TestActionsArtifactConfirmUpload(t *testing.T) { defer tests.PrepareTestEnv(t)() req := NewRequest(t, "PATCH", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") @@ -79,14 +80,14 @@ func TestArtifactConfirmUpload(t *testing.T) { assert.Contains(t, resp.Body.String(), "success") } -func TestArtifactUploadWithoutToken(t *testing.T) { +func TestActionsArtifactUploadWithoutToken(t *testing.T) { defer tests.PrepareTestEnv(t)() req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/1/artifacts", nil) MakeRequest(t, req, http.StatusUnauthorized) } -func TestArtifactDownload(t *testing.T) { +func TestActionsArtifactDownload(t *testing.T) { defer tests.PrepareTestEnv(t)() type ( From a6823285fee8b09fbbd8428df6a65d6a483b0c0f Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Mon, 15 May 2023 20:36:42 +0800 Subject: [PATCH 35/38] update artifact status field name, update tests fixture --- models/actions/artifact.go | 28 +++++++++---------- models/fixtures/action_run.yml | 1 - models/fixtures/action_task.yml | 1 - models/migrations/v1_20/v257.go | 2 +- routers/api/actions/artifacts.go | 2 +- .../integration/api_actions_artifact_test.go | 2 +- 6 files changed, 17 insertions(+), 19 deletions(-) diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 0dd4634752168..1b45fce0673bc 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -16,12 +16,12 @@ import ( ) const ( - // ArtifactUploadStatusPending is the status of an artifact upload that is pending - ArtifactUploadStatusPending = 1 - // ArtifactUploadStatusConfirmed is the status of an artifact upload that is confirmed - ArtifactUploadStatusConfirmed = 2 - // ArtifactUploadStatusError is the status of an artifact upload that is errored - ArtifactUploadStatusError = 3 + // ArtifactStatusUploadPending is the status of an artifact upload that is pending + ArtifactStatusUploadPending = 1 + // ArtifactStatusUploadConfirmed is the status of an artifact upload that is confirmed + ArtifactStatusUploadConfirmed = 2 + // ArtifactStatusUploadError is the status of an artifact upload that is errored + ArtifactStatusUploadError = 3 ) func init() { @@ -42,7 +42,7 @@ type ActionArtifact struct { ContentEncoding string // The content encoding of the artifact ArtifactPath string // The path to the artifact when runner uploads it ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it - UploadStatus int64 `xorm:"index"` // The status of the artifact upload + Status int64 `xorm:"index"` // The status of the artifact, uploading, expired or need-delete CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated index"` } @@ -55,12 +55,12 @@ func CreateArtifact(ctx context.Context, t *ActionTask, artifactName string) (*A artifact, err := getArtifactByArtifactName(ctx, t.Job.RunID, artifactName) if errors.Is(err, util.ErrNotExist) { artifact := &ActionArtifact{ - RunID: t.Job.RunID, - RunnerID: t.RunnerID, - RepoID: t.RepoID, - OwnerID: t.OwnerID, - CommitSHA: t.CommitSHA, - UploadStatus: ArtifactUploadStatusPending, + RunID: t.Job.RunID, + RunnerID: t.RunnerID, + RepoID: t.RepoID, + OwnerID: t.OwnerID, + CommitSHA: t.CommitSHA, + Status: ArtifactStatusUploadPending, } if _, err := db.GetEngine(ctx).Insert(artifact); err != nil { return nil, err @@ -112,7 +112,7 @@ func ListArtifactsByRunID(ctx context.Context, runID int64) ([]*ActionArtifact, // ListUploadedArtifactsByRunID returns all uploaded artifacts of a run func ListUploadedArtifactsByRunID(ctx context.Context, runID int64) ([]*ActionArtifact, error) { arts := make([]*ActionArtifact, 0, 10) - return arts, db.GetEngine(ctx).Where("run_id=? AND upload_status=?", runID, ArtifactUploadStatusConfirmed).Find(&arts) + return arts, db.GetEngine(ctx).Where("run_id=? AND status=?", runID, ArtifactStatusUploadConfirmed).Find(&arts) } // ListArtifactsByRepoID returns all artifacts of a repo diff --git a/models/fixtures/action_run.yml b/models/fixtures/action_run.yml index ebcd03230da6c..2c2151f354af6 100644 --- a/models/fixtures/action_run.yml +++ b/models/fixtures/action_run.yml @@ -10,7 +10,6 @@ commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" event: "push" is_fork_pull_request: 0 - event_payload: "" status: 1 started: 1683636528 stopped: 1683636626 diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index 26c986349a4ae..c78fb3c5d6377 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -17,5 +17,4 @@ log_in_storage: 1 log_length: 707 log_size: 90179 - log_indexes: "" log_expired: 0 diff --git a/models/migrations/v1_20/v257.go b/models/migrations/v1_20/v257.go index 5d49be36d6baa..6c6ca4c7486d0 100644 --- a/models/migrations/v1_20/v257.go +++ b/models/migrations/v1_20/v257.go @@ -24,7 +24,7 @@ func CreateActionArtifactTable(x *xorm.Engine) error { ContentEncoding string // The content encoding of the artifact ArtifactPath string // The path to the artifact when runner uploads it ArtifactName string `xorm:"UNIQUE(runid_name)"` // The name of the artifact when runner uploads it - UploadStatus int64 `xorm:"index"` // The status of the artifact upload + Status int64 `xorm:"index"` // The status of the artifact CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated index"` } diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 732648277805a..9171fb8095b71 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -453,7 +453,7 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) // save storage path to artifact log.Debug("[artifact] merge chunks to artifact: %d, %s", artifact.ID, storagePath) artifact.StoragePath = storagePath - artifact.UploadStatus = actions.ArtifactUploadStatusConfirmed + artifact.Status = actions.ArtifactStatusUploadConfirmed if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { return fmt.Errorf("update artifact error: %v", err) } diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 87570b8d8b697..5f3884d9d6d45 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -122,7 +122,7 @@ func TestActionsArtifactDownload(t *testing.T) { ) idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") - url := listResp.Value[0].FileContainerResourceURL[idx:] + url := listResp.Value[0].FileContainerResourceURL[idx+1:] req = NewRequest(t, "GET", url) req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a") resp = MakeRequest(t, req, http.StatusOK) From f2a4d87aaf81caec1732c4394e7e4d2c574893bd Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Thu, 18 May 2023 18:10:25 +0800 Subject: [PATCH 36/38] artifact title in wrong locale field --- web_src/js/components/RepoActionView.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 0a2cca89d4645..28adfbc6eca71 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -44,7 +44,7 @@
- {{ local.artifactsTitle }} + {{ locale.artifactsTitle }}
  • @@ -347,6 +347,7 @@ export function initRepositoryActionView() { approve: el.getAttribute('data-locale-approve'), cancel: el.getAttribute('data-locale-cancel'), rerun: el.getAttribute('data-locale-rerun'), + artifactsTitle: el.getAttribute('data-locale-artifacts-title'), status: { unknown: el.getAttribute('data-locale-status-unknown'), waiting: el.getAttribute('data-locale-status-waiting'), @@ -356,7 +357,6 @@ export function initRepositoryActionView() { cancelled: el.getAttribute('data-locale-status-cancelled'), skipped: el.getAttribute('data-locale-status-skipped'), blocked: el.getAttribute('data-locale-status-blocked'), - artifactsTitle: el.getAttribute('data-locale-artifacts-title'), } } }); From b60f970a0c62ab98bc244c3127906f38a4f75a81 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 19 May 2023 17:23:35 +0800 Subject: [PATCH 37/38] rename actions storages to LogStorage and ArtifactStorage --- models/unittest/testdb.go | 2 +- modules/setting/actions.go | 8 ++++---- modules/storage/storage.go | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 10a70ad9f8e8f..5351ff1139989 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -126,7 +126,7 @@ func MainTest(m *testing.M, testOpts *TestOptions) { setting.Packages.Storage.Path = filepath.Join(setting.AppDataPath, "packages") - setting.Actions.Storage.Path = filepath.Join(setting.AppDataPath, "actions_log") + setting.Actions.LogStorage.Path = filepath.Join(setting.AppDataPath, "actions_log") setting.Git.HomePath = filepath.Join(setting.AppDataPath, "home") diff --git a/modules/setting/actions.go b/modules/setting/actions.go index eca11bb6edbb1..eb1b637a1c734 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -10,8 +10,8 @@ import ( // Actions settings var ( Actions = struct { - Storage // how the created logs should be stored - Artifacts Storage // how the created artifacts should be stored + LogStorage Storage // how the created logs should be stored + ArtifactStorage Storage // how the created artifacts should be stored Enabled bool DefaultActionsURL string `ini:"DEFAULT_ACTIONS_URL"` }{ @@ -29,6 +29,6 @@ func loadActionsFrom(rootCfg ConfigProvider) { actionsSec := rootCfg.Section("actions.artifacts") storageType := actionsSec.Key("STORAGE_TYPE").MustString("") - Actions.Storage = getStorage(rootCfg, "actions_log", "", nil) - Actions.Artifacts = getStorage(rootCfg, "actions_artifacts", storageType, actionsSec) + Actions.LogStorage = getStorage(rootCfg, "actions_log", "", nil) + Actions.ArtifactStorage = getStorage(rootCfg, "actions_artifacts", storageType, actionsSec) } diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 30cc7497622b9..5b6efccb6a489 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -217,11 +217,11 @@ func initActions() (err error) { ActionsArtifacts = discardStorage("ActionsArtifacts isn't enabled") return nil } - log.Info("Initialising Actions storage with type: %s", setting.Actions.Storage.Type) - if Actions, err = NewStorage(setting.Actions.Storage.Type, &setting.Actions.Storage); err != nil { + log.Info("Initialising Actions storage with type: %s", setting.Actions.LogStorage.Type) + if Actions, err = NewStorage(setting.Actions.LogStorage.Type, &setting.Actions.LogStorage); err != nil { return err } - log.Info("Initialising ActionsArtifacts storage with type: %s", setting.Actions.Artifacts.Type) - ActionsArtifacts, err = NewStorage(setting.Actions.Artifacts.Type, &setting.Actions.Artifacts) + log.Info("Initialising ActionsArtifacts storage with type: %s", setting.Actions.ArtifactStorage.Type) + ActionsArtifacts, err = NewStorage(setting.Actions.ArtifactStorage.Type, &setting.Actions.ArtifactStorage) return err } From 9cfaf5a3a29f889aa82c789bf40013b9b75164e1 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Fri, 19 May 2023 17:41:51 +0800 Subject: [PATCH 38/38] rename artifact route params from camelCase to snake_case --- routers/api/actions/artifacts.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 9171fb8095b71..61d432c8621e0 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -7,7 +7,7 @@ package actions // // 1. Upload artifact // 1.1. Post upload url -// Post: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts?api-version=6.0-preview +// Post: /api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts?api-version=6.0-preview // Request: // { // "Type": "actions_storage", @@ -15,11 +15,11 @@ package actions // } // Response: // { -// "fileContainerResourceUrl":"/api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/upload" +// "fileContainerResourceUrl":"/api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/upload" // } // it acquires an upload url for artifact upload // 1.2. Upload artifact -// PUT: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/upload?itemPath=artifact%2Ffilename +// PUT: /api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/upload?itemPath=artifact%2Ffilename // it upload chunk with headers: // x-tfs-filelength: 1024 // total file length // content-length: 1024 // chunk length @@ -27,36 +27,36 @@ package actions // content-range: bytes 0-1023/1024 // chunk range // we save all chunks to one storage directory after md5sum check // 1.3. Confirm upload -// PATCH: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/upload?itemPath=artifact%2Ffilename +// PATCH: /api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/upload?itemPath=artifact%2Ffilename // it confirm upload and merge all chunks to one file, save this file to storage // // 2. Download artifact // 2.1 list artifacts -// GET: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts?api-version=6.0-preview +// GET: /api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts?api-version=6.0-preview // Response: // { // "count": 1, // "value": [ // { // "name": "artifact", -// "fileContainerResourceUrl": "/api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/path" +// "fileContainerResourceUrl": "/api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/path" // } // ] // } // 2.2 download artifact -// GET: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/path?api-version=6.0-preview +// GET: /api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/path?api-version=6.0-preview // Response: // { // "value": [ // { -// "contentLocation": "/api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/download", +// "contentLocation": "/api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/download", // "path": "artifact/filename", // "itemType": "file" // } // ] // } // 2.3 download artifact file -// GET: /api/actions_pipeline/_apis/pipelines/workflows/{runID}/artifacts/{artifactID}/download?itemPath=artifact%2Ffilename +// GET: /api/actions_pipeline/_apis/pipelines/workflows/{run_id}/artifacts/{artifact_id}/download?itemPath=artifact%2Ffilename // Response: // download file // @@ -90,7 +90,7 @@ const ( artifactXActionsResultsMD5Header = "x-actions-results-md5" ) -const artifactRouteBase = "/_apis/pipelines/workflows/{runID}/artifacts" +const artifactRouteBase = "/_apis/pipelines/workflows/{run_id}/artifacts" func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { m := web.NewRoute() @@ -105,7 +105,7 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route { // retrieve, list and confirm artifacts m.Combo("").Get(r.listArtifacts).Post(r.getUploadArtifactURL).Patch(r.comfirmUploadArtifact) // handle container artifacts list and download - m.Group("/{artifactID}", func() { + m.Group("/{artifact_id}", func() { m.Put("/upload", r.uploadArtifact) m.Get("/path", r.getDownloadArtifactURL) m.Get("/download", r.downloadArtifact) @@ -161,7 +161,7 @@ type artifactRoutes struct { func (ar artifactRoutes) buildArtifactURL(runID, artifactID int64, suffix string) string { uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") + - strings.ReplaceAll(artifactRouteBase, "{runID}", strconv.FormatInt(runID, 10)) + + strings.ReplaceAll(artifactRouteBase, "{run_id}", strconv.FormatInt(runID, 10)) + "/" + strconv.FormatInt(artifactID, 10) + "/" + suffix return uploadURL } @@ -182,7 +182,7 @@ func (ar artifactRoutes) validateRunID(ctx *context.Context) (*actions.ActionTas ctx.Error(http.StatusInternalServerError, "Error getting task in context") return nil, 0, false } - runID := ctx.ParamsInt64("runID") + runID := ctx.ParamsInt64("run_id") if task.Job.RunID != runID { log.Error("Error runID not match") ctx.Error(http.StatusBadRequest, "run-id does not match") @@ -278,7 +278,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) { if !ok { return } - artifactID := ctx.ParamsInt64("artifactID") + artifactID := ctx.ParamsInt64("artifact_id") artifact, err := actions.GetArtifactByID(ctx, artifactID) if errors.Is(err, util.ErrNotExist) { @@ -523,7 +523,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) { return } - artifactID := ctx.ParamsInt64("artifactID") + artifactID := ctx.ParamsInt64("artifact_id") artifact, err := actions.GetArtifactByID(ctx, artifactID) if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err) @@ -552,7 +552,7 @@ func (ar artifactRoutes) downloadArtifact(ctx *context.Context) { return } - artifactID := ctx.ParamsInt64("artifactID") + artifactID := ctx.ParamsInt64("artifact_id") artifact, err := actions.GetArtifactByID(ctx, artifactID) if errors.Is(err, util.ErrNotExist) { log.Error("Error getting artifact: %v", err)