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

ContainerRegistry: Tolerate missing Location header in the manifest PUT response #27

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

euanh
Copy link
Collaborator

@euanh euanh commented Oct 15, 2024

Motivation

According to distribution spec, the manifest PUT response MUST contain a Location header providing a URL from which the saved manifest can be downloaded. The same requirement applies to blob PUTs. However some registries return URLs which cannot be fetched, and ECR does not set this header at all.

The location header is not currently of critical important. If the header is not present, we can create a suitable value based on the spec for manifest downloads.

Modifications

Generate a suitable manifest location URL when the registry does not provide one.

Result

Uploads to ECR will no longer fail because of the missing Location header in the manifest PUT response.

Test Plan

Automated tests continue to pass; manually tested with ECR.

@euanh euanh added semver/minor Adds new public API. area/interoperability Improvements to compatibility with other systems. labels Oct 15, 2024
@euanh euanh force-pushed the tolerate-missing-manifest-location-header branch from 47d4f7f to e342043 Compare October 15, 2024 08:57
@euanh euanh force-pushed the tolerate-missing-manifest-location-header branch from e342043 to 3cc5555 Compare October 15, 2024 09:01
@euanh euanh merged commit 6537515 into apple:main Oct 15, 2024
16 checks passed
@euanh euanh deleted the tolerate-missing-manifest-location-header branch October 15, 2024 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/interoperability Improvements to compatibility with other systems. semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant