From 1fa0463763668f57c03741d50cfe02615f558101 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Mon, 8 Jan 2024 20:36:00 +0800 Subject: [PATCH 1/5] fix: overwrite artifact when same uploading --- routers/api/actions/artifacts.go | 9 +++++++-- routers/api/actions/artifacts_chunks.go | 9 ++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 5bfcc9dfcd760..0d2062ad05155 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -257,8 +257,11 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) { return } - // update artifact size if zero - if artifact.FileSize == 0 || artifact.FileCompressedSize == 0 { + // update artifact size if zero or not match, over write artifact size + if artifact.FileSize == 0 || + artifact.FileCompressedSize == 0 || + artifact.FileSize != fileRealTotalSize || + artifact.FileCompressedSize != chunksTotalSize { artifact.FileSize = fileRealTotalSize artifact.FileCompressedSize = chunksTotalSize artifact.ContentEncoding = ctx.Req.Header.Get("Content-Encoding") @@ -267,6 +270,8 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) { ctx.Error(http.StatusInternalServerError, "Error update artifact") return } + log.Debug("[artifact] update artifact size, artifact_id: %d, size: %d, compressed size: %d", + artifact.ID, artifact.FileSize, artifact.FileCompressedSize) } ctx.JSON(http.StatusOK, map[string]string{ diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 36432a0ca084e..0713c8bba8cea 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -186,7 +186,14 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st }() // save storage path to artifact - log.Debug("[artifact] merge chunks to artifact: %d, %s", artifact.ID, storagePath) + log.Debug("[artifact] merge chunks to artifact: %d, %s, old:%s", artifact.ID, storagePath, artifact.StoragePath) + // if artifact is already uploaded, delete the old file + if artifact.StoragePath != "" { + if err := st.Delete(artifact.StoragePath); err != nil { + log.Warn("Error deleting old artifact: %s, %v", artifact.StoragePath, err) + } + } + artifact.StoragePath = storagePath artifact.Status = int64(actions.ArtifactStatusUploadConfirmed) if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { From 9acf167e5a9c168e738bef847c0ac4e665f1e5ad Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Mon, 8 Jan 2024 21:26:08 +0800 Subject: [PATCH 2/5] fix: add testing for artifact overwrite --- .../integration/api_actions_artifact_test.go | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 2597d103746e6..4974af5333126 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -287,3 +287,84 @@ func TestActionsArtifactUploadWithRetentionDays(t *testing.T) { AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") MakeRequest(t, req, http.StatusOK) } + +func TestActionsArtifactOverwrite(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + { + // download old artifact uploaded by tests above, it should 1024 A + req := NewRequest(t, "GET", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts"). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp := MakeRequest(t, req, http.StatusOK) + var listResp listArtifactsResponse + DecodeJSON(t, resp, &listResp) + + idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") + url := listResp.Value[0].FileContainerResourceURL[idx+1:] + "?itemPath=artifact" + req = NewRequest(t, "GET", url). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp = MakeRequest(t, req, http.StatusOK) + var downloadResp downloadArtifactResponse + DecodeJSON(t, resp, &downloadResp) + + idx = strings.Index(downloadResp.Value[0].ContentLocation, "/api/actions_pipeline/_apis/pipelines/") + url = downloadResp.Value[0].ContentLocation[idx:] + req = NewRequest(t, "GET", url). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("A", 1024) + assert.Equal(t, resp.Body.String(), body) + } + + { + // upload same artifact, it uses 4096 B + req := NewRequestWithJSON(t, "POST", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts", getUploadArtifactRequest{ + Type: "actions_storage", + Name: "artifact", + }).AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp := MakeRequest(t, req, http.StatusOK) + var uploadResp uploadArtifactResponse + DecodeJSON(t, resp, &uploadResp) + + idx := strings.Index(uploadResp.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") + url := uploadResp.FileContainerResourceURL[idx:] + "?itemPath=artifact/abc.txt" + body := strings.Repeat("B", 4096) + req = NewRequestWithBody(t, "PUT", url, strings.NewReader(body)). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a"). + SetHeader("Content-Range", "bytes 0-4095/4096"). + SetHeader("x-tfs-filelength", "4096"). + SetHeader("x-actions-results-md5", "wUypcJFeZCK5T6r4lfqzqg==") // base64(md5(body)) + MakeRequest(t, req, http.StatusOK) + + // confirm artifact upload + req = NewRequest(t, "PATCH", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts?artifactName=artifact"). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + MakeRequest(t, req, http.StatusOK) + } + + { + // download artifact again, it should 4096 B + req := NewRequest(t, "GET", "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts"). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp := MakeRequest(t, req, http.StatusOK) + var listResp listArtifactsResponse + DecodeJSON(t, resp, &listResp) + + idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") + url := listResp.Value[0].FileContainerResourceURL[idx+1:] + "?itemPath=artifact" + req = NewRequest(t, "GET", url). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp = MakeRequest(t, req, http.StatusOK) + var downloadResp downloadArtifactResponse + DecodeJSON(t, resp, &downloadResp) + + idx = strings.Index(downloadResp.Value[0].ContentLocation, "/api/actions_pipeline/_apis/pipelines/") + url = downloadResp.Value[0].ContentLocation[idx:] + req = NewRequest(t, "GET", url). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("B", 4096) + assert.Equal(t, resp.Body.String(), body) + } + +} From bc44c9cb0901230d14daf65a00f796cbbb023de4 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Mon, 8 Jan 2024 21:41:10 +0800 Subject: [PATCH 3/5] fix lint problem --- tests/integration/api_actions_artifact_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 4974af5333126..ad4d449bce927 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -366,5 +366,4 @@ func TestActionsArtifactOverwrite(t *testing.T) { body := strings.Repeat("B", 4096) assert.Equal(t, resp.Body.String(), body) } - } From 25dd87c273d16e01fe60b37577c5e543542420e4 Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Tue, 9 Jan 2024 17:48:01 +0800 Subject: [PATCH 4/5] fix: db test fails in ArtifactOverwrite test --- tests/integration/api_actions_artifact_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index ad4d449bce927..a9e769e6addaa 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -350,8 +350,16 @@ func TestActionsArtifactOverwrite(t *testing.T) { var listResp listArtifactsResponse DecodeJSON(t, resp, &listResp) - idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") - url := listResp.Value[0].FileContainerResourceURL[idx+1:] + "?itemPath=artifact" + var uploadedItem *listArtifactsResponseItem + for _, item := range listResp.Value { + if item.Name == "artifact" { + uploadedItem = &item + } + } + assert.NotNil(t, uploadedItem) + + idx := strings.Index(uploadedItem.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") + url := uploadedItem.FileContainerResourceURL[idx+1:] + "?itemPath=artifact" req = NewRequest(t, "GET", url). AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") resp = MakeRequest(t, req, http.StatusOK) From 19ad0d22c3dee5f96230e33777e6fe78cb45346f Mon Sep 17 00:00:00 2001 From: fuxiaohei Date: Tue, 9 Jan 2024 18:05:12 +0800 Subject: [PATCH 5/5] fix: db test fails in ArtifactOverwrite test --- tests/integration/api_actions_artifact_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index a9e769e6addaa..ce2d14cc0c9c1 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -350,13 +350,14 @@ func TestActionsArtifactOverwrite(t *testing.T) { var listResp listArtifactsResponse DecodeJSON(t, resp, &listResp) - var uploadedItem *listArtifactsResponseItem + var uploadedItem listArtifactsResponseItem for _, item := range listResp.Value { if item.Name == "artifact" { - uploadedItem = &item + uploadedItem = item + break } } - assert.NotNil(t, uploadedItem) + assert.Equal(t, uploadedItem.Name, "artifact") idx := strings.Index(uploadedItem.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") url := uploadedItem.FileContainerResourceURL[idx+1:] + "?itemPath=artifact"