Skip to content
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

actions/upload-artifact & actions/download-artifact v4 support #363

Open
Tracked by #448
Gedochao opened this issue Jan 23, 2024 · 11 comments · Fixed by #438
Open
Tracked by #448

actions/upload-artifact & actions/download-artifact v4 support #363

Gedochao opened this issue Jan 23, 2024 · 11 comments · Fixed by #438
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Gedochao
Copy link

Describe

The popular https://github.com/actions/upload-artifact & https://github.com/actions/download-artifact recently got a major update to v4, which is incompatible with v3 actions.
The v4 release offers a considerable boost in the efficiency of artifacts' uploads & downloads, so it's generally a much desired bump.
By the look of things, this breaks the flow for using the test-reporter with public repositories, as test-reporter uses those with v3.

Example of a bump which seems to break the test reports flow (Scala CLI repo)

I did not do in-depth research on this topic, but it seems that bumping to v4 in the Scala CLI repo breaks the test reports:

We use the public repo setup like this:
https://github.com/VirtusLab/scala-cli/blob/3dcfb3357d9c35bd71be27548e7e4e82fc6ef528/.github/workflows/test-report.yml
So we generate test reports with Mill, use our custom script to change them into the JUnit XML format and then upload the artifacts for the test report workflow to pick it up, nothing special. Should be seen as the standard java-junit flow for a public repo.

Proposed solution

Bump test-reporter actions to v4 & probably release a v2 version of the test-reporter action, to prevent breaking old workflows and let them bump their stuff in peace.

Alternatives considered

Stay at v3, I guess?

@Gedochao
Copy link
Author

Extra context: this is sneaky, as it's very easy to bump to v4 without reading the release notes, thus only realising the test reports are broken when inspecting them.
Users may also be confused when setting up the action for the first time and realising it doesn't work, not knowing why (so as a temporary measure, maybe it'd be useful if the test-reporter@v1 docs mentioned the incompatibility 🤔

@jozefizso jozefizso mentioned this issue Jan 25, 2024
@ScribbleTAS
Copy link

Just updated to v4 and indeed, test-reporter is causing issues...

Error: HTTPError: Response code 400 (Authentication information is not given in the correct format. Check the value of Authorization header.)

Will downgrade again, but I'm looking forward to v2...

And I agree, having a

Warning

actions/upload-artifact & actions/download-artifact version 4 is currently not supported, downgrade to v3

or something like that, would really help clear up any confusion

bgrainger added a commit to bgrainger/NGuid that referenced this issue Feb 6, 2024
bdach added a commit to bdach/osu that referenced this issue Feb 22, 2024
As is github tradition, workflows started yelling about running on a
node version that was getting sunset, so here we go again.

Relevant bumps:

- https://github.com/actions/checkout/releases/tag/v4.0.0
- https://github.com/actions/setup-dotnet/releases/tag/v4.0.0
- https://github.com/actions/cache/releases/tag/v4.0.0
- https://github.com/actions/setup-java/releases/tag/v4.0.0
- https://github.com/peter-evans/create-pull-request/releases/tag/v6.0.0
- https://github.com/dorny/test-reporter/releases/tag/v1.8.0

Notably, `actions/upload-artifact` is _not_ bumped to v4, although it
should be to resolve the node deprecation warnings, because it has more
breaking changes and bumping would break `dorny/test-reporter`
(see dorny/test-reporter#363).
bdach added a commit to bdach/osu that referenced this issue Feb 22, 2024
As is github tradition, workflows started yelling about running on a
node version that was getting sunset, so here we go again.

Relevant bumps:

- https://github.com/actions/checkout/releases/tag/v4.0.0
- https://github.com/actions/setup-dotnet/releases/tag/v4.0.0
- https://github.com/actions/cache/releases/tag/v4.0.0
- https://github.com/actions/setup-java/releases/tag/v4.0.0
- https://github.com/peter-evans/create-pull-request/releases/tag/v6.0.0
- https://github.com/dorny/test-reporter/releases/tag/v1.8.0

Notably, `actions/upload-artifact` is _not_ bumped to v4, although it
should be to resolve the node deprecation warnings, because it has more
breaking changes and bumping would break `dorny/test-reporter`
(see dorny/test-reporter#363).
ZIMkaRU added a commit to ZIMkaRU/bfx-report-electron that referenced this issue Feb 23, 2024
@lauxjpn
Copy link

lauxjpn commented Mar 6, 2024

As a workaround, we manually download and extract our test results containing artifact in a previous step (that we uploaded before using our CI workflow and actions/upload-artifact@v4) using actions/download-artifact@v4 or an actions/github-script@v7 script, and then use dorny/test-reporter@v1 without the artifact option. That way, path just represents the local file system instead the one contained in an artifact.

const inputProvider = this.artifact
? new ArtifactProvider(
this.octokit,
this.artifact,
this.name,
pattern,
this.context.sha,
this.context.runId,
this.token
)
: new LocalFileProvider(this.name, pattern)

@jozefizso
Copy link
Collaborator

The #438 should fix the problem reported by @ScribbleTAS. The original issue will need another fix I think.

@jozefizso jozefizso added this to the artifacts v4 support milestone May 8, 2024
@JojOatXGME
Copy link
Contributor

JojOatXGME commented May 8, 2024

The #438 should fix the problem reported by @ScribbleTAS. The original issue will need another fix I think.

@jozefizso and @Gedochao, what is the original issue? If there is another issue here, it is not clear to me what it is. I think the action should work with Artifacts v4 now.

Background: As far as I know, this action doesn't use @actions/artifacts, but instead communicates with the GitHub REST API. The backward incompatible changes which have been announced by GitHub for Artifacts v4 should not affect the GitHub REST API, but only some Runner-specific API to access Artifacts while the workflow was still in progress. Note that before Artifacts v4, Artifacts were not available over the GitHub REST API before the workflow has finished. Besides all of that, I suspect that GitHub made some internal architectural changes with Artifact v4 which caused the error reported by @ScribbleTAS. I am not aware of any other issue related to Artifacts v4.

@ScribbleTAS
Copy link

ScribbleTAS commented May 9, 2024

Hi @jozefizso and @JojOatXGME ,
thank you for working on this issue.

I can confirm that it works now!

@JojOatXGME
Copy link
Contributor

@ScribbleTAS Since you have edited your comment and removed the block about your issue, I assume you have resolved it.

I have been testing the new version and it fixes the error, but I'm getting a new error:

Error: HttpError: Resource not accessible by integration

For everyone else, the error did occur after the artifacts have already been downloaded and when the report was about to be created. While I don't know much about the case of @ScribbleTAS, this error usually means that your job doesn't have the checks: write permission. (Either because you configured restricted permissions by default and forgot to specify the permission in the workflow file, or because the build was triggered from a fork and you did not follow the recommended setup for public repos.)

PS: Also note that dorny/test-reporter@v1 still serves 1.9.0 which does not contain the fix. See #444. I just wanted to mention it as it might cause some confusion if someone wants to try out the new version.

@ScribbleTAS
Copy link

this error usually means that your job doesn't have the checks: write permission.

This was exactly the issue here. I checked the readme, saw the section with the permissions and immediately put two and two together on why my workflow was failing, even for upload v3.

0xced added a commit to serilog-contrib/serilog-formatting-log4net that referenced this issue Jul 8, 2024
Remove dorny/test-reporter since it does not support the artifact v4 actions

dorny/test-reporter#363
@AliakseiT
Copy link

For all who is affected by the CVE-2024-42471, fix permissions as explained here https://stackoverflow.com/a/78679516/5608789

@JojOatXGME
Copy link
Contributor

JojOatXGME commented Sep 29, 2024

FYI, dorny/test-reporter@v1 runs together with actions/upload-artifact@v4 at JetBrains/gradle-changelog-plugin. All the runs started to fail after actions/upload-artifact has been updated to version 4, but after #444 has been resolved, all the runs were green again.

In short: I still believe this issue and #343 is already resolved since version 1.9.1.

@HofmeisterAn
Copy link

Thanks, @JojOatXGME, for the info. After pinning the version to 1.9.1, I was able to remove actions/upload-artifact@v3 and update it to version 4 as well 👍.

bgrainger added a commit to bgrainger/NGuid that referenced this issue Oct 5, 2024
upload-artifact v3 is deprecated. Update test-reporter to v1.9.1; the v1 tag doesn't point at the latest version. Pinning to v1.9.1 was recommended here: dorny/test-reporter#363 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment