-
Notifications
You must be signed in to change notification settings - Fork 31
fix: add --oci-layout flag for OCI Image Layout blob paths #1723
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
base: main
Are you sure you want to change the base?
Conversation
da1e69b to
3c64d21
Compare
b5a9e8c to
ed1de51
Compare
4d5f150 to
4ece648
Compare
jakobmoellerdev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general this is lgtm in approach for the artifact downloader, but:
- I was expecting the download resource to be fixed alongside, i only see ocm download artifact. whats the behavior for downloading resources with a downloader
- I think the test is not setup stable. you expect the docker daemon to be available but we should test against an actual OCI registry imho. nothing that will block the PR though
| type accessObjectInfo struct { | ||
| accessobj.DefaultAccessObjectInfo | ||
| // ociBlobLayout enables OCI Image Layout Specification compliant blob paths. | ||
| // When true, blobs are stored at blobs/<algorithm>/<encoded> instead of blobs/<algorithm>.<encoded>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the artifactset format should not receive an alternative form to be stored as OCI layout imho.
| }) | ||
|
|
||
| AfterAll(func() { | ||
| GinkgoWriter.Printf("Cleaning up registry packages in: %s\n", registryHost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we have ginkgo logger for such uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakobmoellerdev I could not find such logger, can you point it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4ece648 to
1089c99
Compare
jakobmoellerdev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im pretty sure that this is breaking backwards compatibility. That is because the oci layout flag was present before and I believe we can now no longer read from layouts that had the "old" incorrect oci layout flag. I think we need to version this off... I have encountered a very similar issue during my testing of local blob compatibility
cmds/ocm/commands/ocicmds/artifacts/download/oci_layout_roundtrip_test.go
Show resolved
Hide resolved
1089c99 to
9822ee2
Compare
58d89bd to
0d1928c
Compare
api/oci/extensions/repositories/artifactset/ctf_roundtrip_test.go
Outdated
Show resolved
Hide resolved
0d1928c to
42a688b
Compare
42a688b to
a00fbed
Compare
<!-- markdownlint-disable MD041 --> This change adds an optional --oci-layout flag to the `ocm download artifacts|resources` command to store blobs at blobs/<algorithm>/<encoded> per OCI Image Layout Specification instead of the default blobs/<algorithm>.<encoded> format. This enables compatibility with tools that expect OCI-compliant blob paths. <!-- Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. --> Fixes open-component-model#1668 Signed-off-by: Piotr Janik <piotr.janik@sap.com>
a00fbed to
0fb86c7
Compare
What this PR does / why we need it
This PR adds an
--oci-layoutflag toocm download artifactandocm download resourcescommands to produce OCI Image Layout compliant output.Changes Introduced
New
--oci-layoutFlagWhen specified, OCI artifacts are downloaded with:
blobs/sha256/<digest>instead ofblobs/sha256.<digest>index.json+oci-layoutorg.opencontainers.image.ref.nameannotationNew Format Constant
Added
FORMAT_OCI_COMPLIANT(oci/v1+compliant) which behaves likeFORMAT_OCIbut uses nested blob directory structure per OCI Image Layout specification.Behavior
--oci-layout--oci-layoutdownload artifactdownload resourcesref.nameAnnotationdownload artifact: Uses tag from source reference (e.g.,latest)download resources: Uses resource versionFixes #1668