-
Notifications
You must be signed in to change notification settings - Fork 207
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
A more spec-compliant/resilient OCI distribution implementation #310
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshspicer
changed the title
Wire up tests against Reference Implementation OCI Registry
A more spec-compliant/resilient OCI distribution implementation
Dec 2, 2022
samruddhikhandale
previously approved these changes
Dec 6, 2022
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.
🎉 This is awesome, thanks! ✨
Co-authored-by: Samruddhi Khandale <skhandale@microsoft.com>
chrmarti
approved these changes
Dec 7, 2022
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.
Left a question regarding 'localhost' implying 'http'. LGTM otherwise!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Exercises the
devcontainer features publish
command against the OCI registry specification's Reference Implementation , and updates our implementation to more closely follow the specification.More specifically, starts a
registry
container (with hardcoded auth generated with htpasswd).And then executing the following
publish
command against the registry server running on localhost.A new class of unit tests will spin up an ephemeral
registry
container and exercise the push and pull operations of our OCI distribution implementation.Details on changes made to support Reference Implementation
postUploadSessionId
for each uploaded blob layer. Previously, a single session ID was created for each manifest, but our implementation now matches the specification where it should be generated per blob. GHCR tolerates re-using the upload ID, but the reference implementation does not.Basic
auth if a session token cannot be fetched from the registry. For GHCR, operations are only accepted with a valid session token from the/session
endpoint of the registry, whereas the reference implementation talks only with Basic auth credentials. I don't see the session auth built in the official specification, so my implementation supports both methods, only using Basic auth if the /token endpoint does not return a success code and a valid token from it's JSON response body.Content-Length
header when uploading blobs, per the specification.Additional Changes
.tgz
files from disk several times throughout the process. Now, the file is read from disk into a buffer one time, and the buffer is passed around. This is more efficient.DEVCONTAINERS_OCI_AUTH
environment variable was always broken (a condition would always result tofalse
). Since there is no way this could be used by any active users (and it is not yet documented), I changed the contract a bit to include a username field, which is necessary to get this variable working with the reference implementation (GHCR ignores this field, which is why it was originally omitted). This variable is exercised in the provided test.features info
command two new, simple commandsdevcontainer info tags
anddevcontainer info manifest
. These are useful utilities to break out for understanding published Features. They are used in the newly added test.http
when communicating withlocalhost
, or if the parsed protocol ishttp
.