Fix uploaded artifacts should be overwritten (#28726) backport v1.21

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{

routers/api/actions/artifacts_chunks.go

@@ -182,7 +182,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 {

tests/integration/api_actions_artifact_test.go

@@ -290,3 +290,93 @@ func TestActionsArtifactUploadWithRetentionDays(t *testing.T) {
  req = addTokenAuthHeader(req, "Bearer 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")
+ req = addTokenAuthHeader(req, "Bearer 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)
+ req = addTokenAuthHeader(req, "Bearer 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)
+ req = addTokenAuthHeader(req, "Bearer 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",
+ })
+ req = addTokenAuthHeader(req, "Bearer 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))
+ req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
+ req.Header.Add("Content-Range", "bytes 0-4095/4096")
+ req.Header.Add("x-tfs-filelength", "4096")
+ req.Header.Add("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")
+ req = addTokenAuthHeader(req, "Bearer 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")
+ req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
+ resp := MakeRequest(t, req, http.StatusOK)
+ var listResp listArtifactsResponse
+ DecodeJSON(t, resp, &listResp)
+
+ var uploadedItem listArtifactsResponseItem
+ for _, item := range listResp.Value {
+ if item.Name == "artifact" {
+ uploadedItem = item
+ break
+ }
+ }
+ assert.Equal(t, uploadedItem.Name, "artifact")
+
+ idx := strings.Index(uploadedItem.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
+ url := uploadedItem.FileContainerResourceURL[idx+1:] + "?itemPath=artifact"
+ req = NewRequest(t, "GET", url)
+ req = addTokenAuthHeader(req, "Bearer 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)
+ req = addTokenAuthHeader(req, "Bearer 8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
+ resp = MakeRequest(t, req, http.StatusOK)
+ body := strings.Repeat("B", 4096)
+ assert.Equal(t, resp.Body.String(), body)
+ }
+}