Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions models/actions/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
18 changes: 18 additions & 0 deletions models/fixtures/action_artifact.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion routers/api/actions/artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 14 additions & 6 deletions routers/api/actions/artifactsv4.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down Expand Up @@ -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{}

Expand Down Expand Up @@ -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)

Expand Down
20 changes: 20 additions & 0 deletions tests/integration/api_actions_artifact_v4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down