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

chore: refactor the use of media types in uds-cli #581

Closed
wants to merge 3 commits into from

Conversation

Racer159
Copy link
Contributor

Description

This PR looks to begin the use of correct mediatypes for registry objects

Related Issue

Fixes #N/A

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@UncleGedd
Copy link
Collaborator

I really like this idea, even if some parts of zoci are untenable this is still simpler than what we had previously

@UncleGedd UncleGedd mentioned this pull request Apr 23, 2024
@UncleGedd
Copy link
Collaborator

Still debugging but it appears this solution works for GHCR but not registry/v2.

For the GHCR case, oras.Copy looks for the image manifest descs from the bundle root manifest at /manifests which is correct, and it doesn't find them because we intentionally store them under /blobs; this causes oras.Copy to behave as expected

However, when working with registry/v2, the /manifests and /blobs endpoints will both return the underlying layer even if the endpoint is incorrect. This means that even though we store the image manifests under /blobs, they can be found under /manifests, which makes oras.Copy think the pkg layers have already been copied, resulting in incomplete pkgs in the OCI repo

@UncleGedd
Copy link
Collaborator

UncleGedd commented May 15, 2024

I've tried this a few different ways now including:

  • pushing the image manifests after oras.Copy -> results in MANIFEST_BLOB_UNKNOWN because the Zarf pkg manifests aren't stored whenever the bundle root manifest is copied over
  • changing the media type of the bundle root manifest to a custom one -> results in MANIFEST_INVALID because the registries require the index to point to image manifests

If we really wanted to do this, we could do something like:

index -> image manifest -> custom uds manifest containing refs to Zarf image manifests -> Zarf image manifests

By removing the current image manifest -> image manifest double pointer, this would likely work. That said, at this point I don't think the juice is worth the squeeze.

The team is open to more feedback though @Racer159 , otherwise I'll close this PR (noting we're working on removing layer verification in a separate ticket which should speed up your deploys)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants