actions artifacts api list/download check status upload confirmed (#34273)

* fixes a fixture status to upload confirmed
* add another fixture as noise to break tests as soon they are exposed
to api
* v4 delete test added check that artifact is no longer visible in
internal api with status pending delete
* removal of http 404 on empty list: actions/upload-artifact@v4 now
backoff on http 404 of ListArtifacts endpoint
* fixes artifacts with pending delete etc. are able to be found and
downloaded if the storage is not freed

models/actions/artifact.go

@@ -30,6 +30,25 @@ const (
 	ArtifactStatusDeleted                                   // 6, ArtifactStatusDeleted is the status of an artifact that is deleted
 )
 
+func (status ArtifactStatus) ToString() string {
+	switch status {
+	case ArtifactStatusUploadPending:
+		return "upload is not yet completed"
+	case ArtifactStatusUploadConfirmed:
+		return "upload is completed"
+	case ArtifactStatusUploadError:
+		return "upload failed"
+	case ArtifactStatusExpired:
+		return "expired"
+	case ArtifactStatusPendingDeletion:
+		return "pending deletion"
+	case ArtifactStatusDeleted:
+		return "deleted"
+	default:
+		return "unknown"
+	}
+}
+
 func init() {
 	db.RegisterModel(new(ActionArtifact))
 }

models/fixtures/action_artifact.yml

@@ -11,6 +11,24 @@
   content_encoding: ""
   artifact_path: "abc.txt"
   artifact_name: "artifact-download"
+  status: 2
+  created_unix: 1712338649
+  updated_unix: 1712338649
+  expired_unix: 1720114649
+
+-
+  id: 2
+  run_id: 791
+  runner_id: 1
+  repo_id: 4
+  owner_id: 1
+  commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
+  storage_path: ""
+  file_size: 1024
+  file_compressed_size: 1024
+  content_encoding: "30/20/1712348022422036662.chunk"
+  artifact_path: "abc.txt"
+  artifact_name: "artifact-download-incomplete"
   status: 1
   created_unix: 1712338649
   updated_unix: 1712338649

routers/api/actions/artifacts.go

@@ -337,7 +337,10 @@ func (ar artifactRoutes) listArtifacts(ctx *ArtifactContext) {
 		return
 	}
 
-	artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{RunID: runID})
+	artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{
+		RunID:  runID,
+		Status: int(actions.ArtifactStatusUploadConfirmed),
+	})
 	if err != nil {
 		log.Error("Error getting artifacts: %v", err)
 		ctx.HTTPError(http.StatusInternalServerError, err.Error())
@@ -402,6 +405,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
 	artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{
 		RunID:        runID,
 		ArtifactName: itemPath,
+		Status:       int(actions.ArtifactStatusUploadConfirmed),
 	})
 	if err != nil {
 		log.Error("Error getting artifacts: %v", err)
@@ -473,6 +477,11 @@ func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) {
 		ctx.HTTPError(http.StatusBadRequest)
 		return
 	}
+	if artifact.Status != actions.ArtifactStatusUploadConfirmed {
+		log.Error("Error artifact not found: %s", artifact.Status.ToString())
+		ctx.HTTPError(http.StatusNotFound, "Error artifact not found")
+		return
+	}
 
 	fd, err := ar.fs.Open(artifact.StoragePath)
 	if err != nil {

routers/api/actions/artifactsv4.go

@@ -448,17 +448,15 @@ func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) {
 		return
 	}
 
-	artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{RunID: runID})
+	artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{
+		RunID:  runID,
+		Status: int(actions.ArtifactStatusUploadConfirmed),
+	})
 	if err != nil {
 		log.Error("Error getting artifacts: %v", err)
 		ctx.HTTPError(http.StatusInternalServerError, err.Error())
 		return
 	}
-	if len(artifacts) == 0 {
-		log.Debug("[artifact] handleListArtifacts, no artifacts")
-		ctx.HTTPError(http.StatusNotFound)
-		return
-	}
 
 	list := []*ListArtifactsResponse_MonolithArtifact{}
 
@@ -510,6 +508,11 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
 		ctx.HTTPError(http.StatusNotFound, "Error artifact not found")
 		return
 	}
+	if artifact.Status != actions.ArtifactStatusUploadConfirmed {
+		log.Error("Error artifact not found: %s", artifact.Status.ToString())
+		ctx.HTTPError(http.StatusNotFound, "Error artifact not found")
+		return
+	}
 
 	respData := GetSignedArtifactURLResponse{}
 
@@ -538,6 +541,11 @@ func (r *artifactV4Routes) downloadArtifact(ctx *ArtifactContext) {
 		ctx.HTTPError(http.StatusNotFound, "Error artifact not found")
 		return
 	}
+	if artifact.Status != actions.ArtifactStatusUploadConfirmed {
+		log.Error("Error artifact not found: %s", artifact.Status.ToString())
+		ctx.HTTPError(http.StatusNotFound, "Error artifact not found")
+		return
+	}
 
 	file, _ := r.fs.Open(artifact.StoragePath)
 

tests/integration/api_actions_artifact_v4_test.go

@@ -557,6 +557,26 @@ func TestActionsArtifactV4Delete(t *testing.T) {
 	var deleteResp actions.DeleteArtifactResponse
 	protojson.Unmarshal(resp.Body.Bytes(), &deleteResp)
 	assert.True(t, deleteResp.Ok)
+
+	// confirm artifact is no longer accessible by GetSignedArtifactURL
+	req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/GetSignedArtifactURL", toProtoJSON(&actions.GetSignedArtifactURLRequest{
+		Name:                    "artifact-v4-download",
+		WorkflowRunBackendId:    "792",
+		WorkflowJobRunBackendId: "193",
+	})).
+		AddTokenAuth(token)
+	_ = MakeRequest(t, req, http.StatusNotFound)
+
+	// confirm artifact is no longer enumerateable by ListArtifacts and returns length == 0 without error
+	req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/ListArtifacts", toProtoJSON(&actions.ListArtifactsRequest{
+		NameFilter:              wrapperspb.String("artifact-v4-download"),
+		WorkflowRunBackendId:    "792",
+		WorkflowJobRunBackendId: "193",
+	})).AddTokenAuth(token)
+	resp = MakeRequest(t, req, http.StatusOK)
+	var listResp actions.ListArtifactsResponse
+	protojson.Unmarshal(resp.Body.Bytes(), &listResp)
+	assert.Empty(t, listResp.Artifacts)
 }
 
 func TestActionsArtifactV4DeletePublicApi(t *testing.T) {