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

Don't maintain URLs for foreign layers when pushing them #1998

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

drcapulet
Copy link

For images with non-distributable layers, if we're pushing the layer to the destination we want to remove any URLs on the layer since they're generally preferred over the repository itself (containerd example) - this is important for deployments to air-gapped installations that don't have access to any of the URLs.

Signed-off-by: Alex Coomans <alex.coomans@withpersona.com>
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

AFAICS c/image is, currently, falling back to using the registry if the external URL is unavailable. Though it does look that that’s not the case for containerd, and it seems to be unintentional for c/image.

It’s not clear to me that this is a behavior we can just change; it might need a separate option.


Note to self: The documentation for DownloadForeignLayers says that MIME types are updated, and I can’t see how that happens (nor, actually, whether it is desirable).

@drcapulet
Copy link
Author

@mtrmac Appreciate the review - funny enough the day I opened this Microsoft dropped the usage of foreign layers, I'm inclined to just close this out?

I can't personally tell on the expected behavior for URLs from the spec.

@mtrmac
Copy link
Collaborator

mtrmac commented Jun 28, 2023

I do think you’ve touched on a feature gap and/or a bug, and it’s worth fixing in principle.

OTOH the real-world need for the foreign layer feature is really becoming much less urgent. If you want to close this PR, I think that’s completely fair.

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.

None yet

2 participants