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.

Sign up for GitHub

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

Jump to bottom

[feature] Use upload-artifact v4 #3068

Closed
laurentsimon opened this issue Jan 8, 2024 · 17 comments · Fixed by #3312
Closed

[feature] Use upload-artifact v4 #3068

laurentsimon opened this issue Jan 8, 2024 · 17 comments · Fixed by #3312
Labels
type:feature New feature or request
Milestone
Next release

Comments

@laurentsimon
Copy link
Collaborator

laurentsimon commented Jan 8, 2024 •
edited
Loading

we may need a compatibility option as argument to avoid breaking folks who currently use v3
@laurentsimon laurentsimon added type:feature New feature or request status:triage Issue that has not been triaged labels Jan 8, 2024
@jkreileder
Copy link
Contributor

upload-artifact@v4+ also is not yet available for GHES, so v3 should definitely be kept as the default version.

@behnazh-w
Copy link
Contributor

behnazh-w commented Jan 9, 2024 •
edited
Loading

I think it makes more sense to release a new version of the generator that is compatible with v4 by default (as soon as possible to avoid breaking pipelines that have already updated download-artifact to v4) and add a backward compatibility argument for v3.

I would argue that this release would have a breaking change. So the release version should be slsa-github-generator@v2.0.0.

The changelog and documentation should warn about the compatibility issue and that users should use download-artifact@v4 by default.

@behnazh-w behnazh-w mentioned this issue Jan 9, 2024
Merged
@ianlewis
Copy link
Member

ianlewis commented Jan 9, 2024 •
edited
Loading

Am I right in understanding that upload/download-artifact v3 and v4 are mutually incompatible? i.e. you can't download an artifact with download-artifact@v4 when uploaded with upload-artifact@v3 and you also cannot download an artifact with download-artifact@v3 that was uploaded with upload-artifact@v4 ?

@ianlewis ianlewis removed the status:triage Issue that has not been triaged label Jan 9, 2024
@behnazh-w
Copy link
Contributor

Am I right in understanding that upload/download-artifact v3 and v4 are mutually incompatible? i.e. you can't download an artifact with download-artifact@v4 when uploaded with upload-artifact@v3 and you also cannot download an artifact with download-artifact@v3 that was uploaded with upload-artifact@v4 ?

Yes, you're right. See actions/download-artifact#250 (comment).

@ianlewis
Copy link
Member

ianlewis commented Jan 9, 2024

Yes, you're right. See actions/download-artifact#250 (comment).

Got it. Thanks.

I think it makes more sense to release a new version of the generator that is compatible with v4 by default (as soon as possible to avoid breaking pipelines that have already updated download-artifact to v4) and add a backward compatibility argument for v3.

I would argue that this release would have a breaking change. So the release version should be slsa-github-generator@v2.0.0.

I agree about the compatibility and version number but is an option really necessary? In what situation do you think users would need to use it? I expect that it's not a huge deal to upgrade to download-artifact@v4 and that it's ok for us to ask them to upgrade it on a major version change of the generators but are there other blockers for users that we should know about?

@behnazh-w
Copy link
Contributor

As @jkreileder pointed out download-artifact@v4 is not compatible with GitHub Enterprise Server. So, if the you don't add the option for v3 compatibility, you might need to have a separate release of slsa-github-generator artifacts for GHES, which is too much work probably.

@ianlewis
Copy link
Member

As @jkreileder pointed out download-artifact@v4 is not compatible with GitHub Enterprise Server. So, if the you don't add the option for v3 compatibility, you might need to have a separate release of slsa-github-generator artifacts for GHES, which is too much work probably.

I'm not sure how hosted runners work on GHES. Are hosted runners supported or do users need to use self-hosted runners?

@behnazh-w
Copy link
Contributor

I'm not sure how hosted runners work on GHES. Are hosted runners supported or do users need to use self-hosted runners?

Yes, GitHub Enterprise supports standard GitHub-hosted runners.

@ianlewis
Copy link
Member

ianlewis commented Jan 10, 2024 •
edited
Loading

I think that's Github Enterprise Cloud which supports hosted runners (as it's hosted by Github) vs Github Enterprise Server which doesn't support hosted runners (as it's self-hosted).

I may be showing my ignorance though. I'm not super familiar with Github Enterprise offerings.

@kapilt kapilt mentioned this issue Jan 10, 2024
Merged
@jkreileder
Copy link
Contributor

@ianlewis you're right, GHES only supports self-hosted runners.

@kapilt
Copy link

kapilt commented Jan 10, 2024 •
edited
Loading

This is currently labeled feature, but I think its closer to a bug, in that its breakage for new users trying to use the generator workflow, as happened to me yesterday, or extant users dealing with an update to artifact actions. also fwiw having also done the dependabot driven update from v3 to v4 on artifact actions, it does have some nuance/work for projects potentially based on usage wrt to immutable cache names.

[update] - fwiw, gh cli does seem to be able to retrieve assets uploaded with either v3 or v4 re end user workarounds.

@jkreileder
Copy link
Contributor

@behnazh-w I'm not sure if maintaining two versions would be too much work: The only difference between slsa-github-generator@v1 and slsa-github-generator@v2 would be the dependency on the up/download-artifact version - 10 lines of yaml code if I counted correctly.
Merging backing other changes from slsa-github-generator@v2 to slsa-github-generator@v1 should be easy.

For users that would mean:

  • GHES users stay at slsa-github-generator@v1 just like they must stay at up/download-artifact@v3
  • All others can upgrade to slsa-github-generator@v2

@laurentsimon
Copy link
Collaborator Author

@ramonpetgrave64

@behnazh-w
Copy link
Contributor

@ianlewis you're right, GHES only supports self-hosted runners.

For users that would mean:

GHES users stay at slsa-github-generator@v1 just like they must stay at up/download-artifact@v3
All others can upgrade to slsa-github-generator@v2

@jkreileder I was wrongly thinking of the Github Enterprise Cloud when I proposed to have backward compatibility with GHES earlier. I don't think slsa-github-generator supports self-hosted runners needed by GHES anyway. So, we might not need backward-compatibility altogether?

@laurentsimon @ianlewis please correct me if I'm wrong.

@ianlewis
Copy link
Member

@jkreileder I was wrongly thinking of the Github Enterprise Cloud when I proposed to have backward compatibility with GHES earlier. I don't think slsa-github-generator supports self-hosted runners needed by GHES anyway. So, we might not need backward-compatibility altogether?

@laurentsimon @ianlewis please correct me if I'm wrong.

Yes. That was my thinking.

@davidism davidism mentioned this issue Jan 15, 2024
Closed
@laurentsimon laurentsimon added this to the Next release milestone Jan 16, 2024
@behnazh behnazh mentioned this issue Jan 23, 2024
Merged
behnazh added a commit to jenstroeger/python-package-template that referenced this issue Jan 23, 2024
@behnazh
fix(deps-dev): revert artifact download and upload GitHub Actions to …
22d00bc 
…v3 (#686)

The provenance generator GitHub Action `slsa-framework/generator_generic_slsa3.yml` is not compatible with `actions/download-artifact@v4` yet. We need to make sure these two Actions and `actions/upload-artifact` are all compatible.

This PR reverts `actions/download-artifact` and `actions/upload-artifact` GitHub Actions to v3. We will update them to v4 when the next version of `generator_generic_slsa3.yml` that will be compatible with `actions/download-artifact@v4` is released. See this relevant `slsa-framework/slsa-github-generator#3068`.
@behnazh-w behnazh-w mentioned this issue Feb 6, 2024
Closed
@loosebazooka loosebazooka mentioned this issue Feb 12, 2024
Closed
1 task
@ianlewis ianlewis mentioned this issue Feb 16, 2024
Closed
8 tasks
@ianlewis
Copy link
Member

This also seems to be an issue regarding the deprecation of Node 16. The v3 versions of upload-artifact use Node 16 and haven't been updated to Node 20.

[build / slsa-run / verify-token](https://github.com/slsa-framework/example-package/actions/runs/8042728772/job/21963776341)
Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32. For more information see: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

@laurentsimon
Copy link
Collaborator Author

We're working on this for next release. Given the comments above, we're going to do a major release that will only support v4 upload / download Action.

@malt3 malt3 mentioned this issue Feb 29, 2024
Merged
@ramonpetgrave64 ramonpetgrave64 mentioned this issue Mar 8, 2024
Merged
5 tasks
laurentsimon added a commit that referenced this issue Mar 13, 2024
@ramonpetgrave64 @laurentsimon
fix: upload-artifact and download-artifact v4 (#3312)
b595e06 
# Summary

- Fixes #3068 to use upload-artifact and download-artifact v4
- following up in
slsa-framework/example-package#336

## Testing Process

This change is tested with our existing PR Check workflows that use both
directly and indirectly call upload-artifact and download-artifact.
- One test for `secure-upload-folder` should fail in this PR because it
will use `secure-upload-artifact@main`. There's no workaround to
dynamically use the PR's ref instead of `@main`, but after merging this
PR, the test should start passing.

## Checklist

- [x] Review the contributing [guidelines](./../CONTRIBUTING.md)
- [x] Add a reference to related issues in the PR description.
- [x] Update documentation if applicable.
- [x] Add unit tests if applicable.
- [x] Add changes to the [CHANGELOG](./../CHANGELOG.md) if applicable.

---------

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
Signed-off-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type:feature New feature or request
Projects
None yet
Milestone
Next release
Development

Successfully merging a pull request may close this issue.

fix: upload-artifact and download-artifact v4 ramonpetgrave64/slsa-github-generator
5 participants
@kapilt @ianlewis @jkreileder @behnazh-w @laurentsimon