feat!: allow pulling images by index sha#4879
Conversation
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
✅ Deploy Preview for zarf-docs canceled.
|
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
AustinAbro321
changed the title
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
AustinAbro321
changed the title
Signed-off-by: Austin Abro <[email protected]>
| } | ||
|
|
||
| // sumManifestsSize walks each descriptor (recursing into nested indexes) and totals up the byte | ||
| // size of every referenced blob plus one "arch[/variant]" string per leaf manifest. |
There was a problem hiding this comment.
More for my education looking at the OCI spec -
- do we need to consider OS? Variety of implications here
- seems unlikely but will there ever be a case of a shared layer?
There was a problem hiding this comment.
- For now no, we hardcode
os: linuxwhen pulling packages. - It's technically possible, this function should have had unit tests anyhow. Added logic to handle this
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
| require.GreaterOrEqual(t, count, 2, "expected per-platform SBOMs for the digested multi-platform image") | ||
|
|
||
| stdOut, stdErr, err = e2e.Zarf(t, "package", "deploy", pulledPkgPath, "--confirm", "--skip-version-check") | ||
| require.NoError(t, err, stdOut, stdErr) |
There was a problem hiding this comment.
Do we need fidelity in the state of the pod in the cluster to confirm any deployment behaviors or are we confident in it if the deployment occurs successfully?
There was a problem hiding this comment.
I'm confident if the deploy occurs successfully.
| Reason: "This package contains image archives which will only be recognized on v0.68.0+", | ||
| }) | ||
| break | ||
| hasIndex := false |
There was a problem hiding this comment.
Nit: nonblocking - the split here between buildPath, hasIndex and collectVersionRequirements is entirely functional but feels a little awkward. in so much as I think buildPath could be a parameter and then imageLayoutasIndex could be internal to collectVersionRequirements ?
There was a problem hiding this comment.
The reason I did it this way was so I could unit test collectVersionRequirements without a image archive. Also imageLayoutHasIndex is used packageLayout.HasImageIndex()
| return fmt.Errorf("failed to inspect package image layout: %w", err) | ||
| } | ||
| if hasImageIndex { | ||
| return nil |
There was a problem hiding this comment.
++ short-circuit validation. mental note for you mention of potentially checking for the desired target arch if we deemed necessary (not required).
2 participants
Breaking Changes
Breaking changes are limited to the SDK in
src/pkg/imagesDescription
This implements pulling image indexes. It adds a version requirements so that Zarf can correctly pull the sha'd images. It also changes the deploy check to allow deploying to any node when an index sha image exists in the package.
When there are multiple container images pointed to an index, Zarf will create an SBOM for each image.
Pulling Indexes place a minimumVersionRequirement of v0.76.0. Also due to our pull logic not properly handling indexes, Zarf will panic if it tries to pull a package with indexes before this change.
I considered adding a metadata field, or requiring
.metadata.architecture: multifor index pulls, but I think that will cause more confusion than improvements to the UX. We don't block users from pulling sha's of different digests. I'm hoping the log of the platform during pull will be enough of an indicator to those using index sha's mistakenly.Related Issue
Relates to #2425. I'll probably close this is merged, but I want to comment on how this doesn't implement tags or specific architectures and ask the issue thread to make an issue if this is a feature they desire.
Checklist before merging