Decompress gzipped artifacts before saving to file

[bogsen]

Artifacts being gzipped is just an implementation detail of the artifact upload protocol. Decompressing gzipped uploads immediately during upload allows for easier access to artifact files from outside of act, and simplifies artifact serving logic a bit since it can assume that artifacts aren't compressed.

[mergify[bot]]

@bogsen this pull request has failed checks 🛠

[codecov[bot]]

Codecov Report

Merging #1188 (221c8a9) into master (4f8da0a) will increase coverage by 4.62%. The diff coverage is 78.37%.

@@            Coverage Diff             @@
##           master    #1188      +/-   ##
==========================================
+ Coverage   57.50%   62.13%   +4.62%     
==========================================
  Files          32       40       +8     
  Lines        4594     5342     +748     
==========================================
+ Hits         2642     3319     +677     
- Misses       1729     1761      +32     
- Partials      223      262      +39     

Impacted Files	Coverage Δ
pkg/model/action.go	0.00% <0.00%> (ø)
pkg/model/github_context.go	79.51% <ø> (ø)
pkg/model/planner.go	50.73% <ø> (+0.32%)	:arrow_up:
pkg/model/step_result.go	0.00% <ø> (ø)
pkg/model/workflow.go	54.29% <0.00%> (+3.38%)	:arrow_up:
pkg/container/docker_run.go	15.18% <22.22%> (+9.64%)	:arrow_up:
pkg/container/file_collector.go	44.85% <44.85%> (ø)
pkg/artifacts/server.go	65.33% <50.00%> (-1.99%)	:arrow_down:
pkg/exprparser/interpreter.go	73.37% <53.48%> (-0.02%)	:arrow_down:
pkg/runner/command.go	89.34% <75.00%> (-1.57%)	:arrow_down:
... and 30 more

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[ChristopherHX]

Decompressing the artifacts on the server really makes sense if you want direct access to the artifacts.

if a file was uploaded as zip already this breaks the client side, right?

I agree, if you want to access artifacts from previous act builds. Otherwise I'm fine with this change.

I'm not shure about the go lint security error, all workarounds I found doesn't look to be great / fix the actual security issue.

[bogsen]

You are breaking the artifacts protocol of github here. By removing the logic if a file was uploaded as zip already this breaks the client side, right?

I'm not sure which logic you are specifically referring to. If we decompress gzipped artifacts immediately at upload time we don't need any special logic anywhere else (except maybe for artifacts that existed before this change as @ChristopherHX mentioned). And since we don't return Content-Encoding when downloading existing artifacts anymore, the client will know that the artifact isn't gzipped and handle it accordingly.

I agree, if you want to access artifacts from previous act builds. Otherwise I'm fine with this change.

Does that currently work? I can leave those in but I'm not sure act implements the necessary APIs for something like https://github.com/marketplace/actions/download-workflow-artifact (for example) to work.

[ChristopherHX]

Does that currently work? I can leave those in but I'm not sure act implements the necessary APIs for something like https://github.com/marketplace/actions/download-workflow-artifact (for example) to work.

The action you mentioned won't work, but I think you can just use actions/download-artifact in a later workflow run with a different act version and the same artifacts folder. I don't fully understand the actual reason for the requested changes.

[KnisterPeter]

You are breaking the artifacts protocol of github here. By removing the logic if a file was uploaded as zip already this breaks the client side, right?

I'm not sure which logic you are specifically referring to. If we decompress gzipped artifacts immediately at upload time we don't need any special logic anywhere else (except maybe for artifacts that existed before this change as @ChristopherHX mentioned). And since we don't return Content-Encoding when downloading existing artifacts anymore, the client will know that the artifact isn't gzipped and handle it accordingly.

I agree, if you want to access artifacts from previous act builds. Otherwise I'm fine with this change.

Does that currently work? I can leave those in but I'm not sure act implements the necessary APIs for something like https://github.com/marketplace/actions/download-workflow-artifact (for example) to work.

This does work and should be :smiley: These are the test (which might not be sufficient as we are not uploading zipped files I think): https://github.com/nektos/act/blob/master/pkg/artifacts/testdata/upload-and-download/artifacts.yml

The download protocol for the artifacts is defined by github with the upload and download actions. The expectation of someone who uploads a gzipped file would be to download a gzipped file as well. If you uncompress the artifact during upload and store it like that, you will have to recreate the archive when downloading (and recreate an identical artifact).

[github-actions[bot]]

PR is stale and will be closed in 14 days unless there is new activity

[mergify[bot]]

@bogsen this pull request is now in conflict 😩

[github-actions[bot]]

PR is stale and will be closed in 14 days unless there is new activity