Support run-time manifest type support (e.g. schema1/schema2) autodetection #264
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
mtrmac
force-pushed
the
schema12
branch
2 times, most recently
from
70cff9c
to
c4c8d03
Compare
|
Updated to also recognize |
mtrmac
changed the title
|
@runcom PTAL. This is a series of 3 c/image PRs, with corresponding skopeo PRs (updating and extending tests):
|
|
👍 looks fine |
This will allow us to try several manifest formats before signing the one acceptable by the destination. This is a minor behavior change, however the user was previously waiting for upload of hundreds of megabytes of layer data, so upload of the small manifest should not be very noticeable. What _may_ be noticeable is that before, if the signing failed, the manifest tag in the destination was not modified; now the tag is pointed to the new manifest, but the manifest has no signatures. (Individual transports could still batch both operations and actually upload atomically in .Commit(), but the docker:// destinations we care about for manifest format autodetection don’t work that way; sadly we have no option but to really upload the manifest.) Signed-off-by: Miloslav Trmač <mitr@redhat.com>
There is no particular reason to set it only when we know that we will be updating the manifest, and because the value is immutable, it is convenient to move it out of the core manifest update path (which will be possibly run several times in the future). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
… to make the high-level structure of copy.Image easier to follow. Also move it to a separate file, it has few outside interactions and every little bit making copy.go more readable helps. Does not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This directly references the fixtures in ../signature/fixtures; arguably this subpackage should have its own fixtures. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
… to make the high-level structure of copy.Image easier to follow. In the future, we will also try calling this several times with different manifest types. Does not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
It is fairly stand-alone, and this shortens copy.go again. Does not change behavior (in fact, does not change the function at all). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…tConversion This is either the original MIME type from the source, or the MIME type we are (in the future, primarily,) converting to. The return value is not currently being used; later it will be used only in an error message, but quite useful in there. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Instead of trusting ImageDestination.SupportedManifestMIMETypes() to be perfectly correct, build a list of all possible MIME types to try. This will be useful for supporting both schema1+schema2 and schema1-only docker/distribution registries, which can’t be (at least in the case of OpenShift) distinguished without actually trying to upload a manifest. This is not used yet, the user will be added in the next commit. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…rder Finally take advantage of the new data computed by determineManifestConversion. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This will be used to trigger the fallback to other manifest schemas; for now this only defines a type and updates comments. Does not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The user relying on this will be added next. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This will allow us to avoid repeated attempts, and an ugly error message describing all of them, if the manifest upload fails for a different reason. Note that this differs from the docker/docker behavior, which falls back from schema2 to schema1 on any kind of failure. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
👍 |
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.
This modifies
copy.Imageto try a sequence of possibly supported manifest MIME types instead of only a single preferred one; notably this allows using thedocker:transport with schema1-only registries, although the mechanism is ultimately fully general (i.e. it can deal with OCI-only registries just as well ifSupportsManifestMIMETypeincludes OCI).See the individual commit messages for detailed description;
NOTE The final commits, starting with “Define a ManifestTypeRejectedError for ImageDestination.PutManifest”, implement the behavior of only trying a sequence of MIME types if the initial manifest is rejected with a recognized "this is an invalid manifest” error (as opposed to anything else, e.g. insufficient permissions or out of space). This is IMHO more sensible, but it does differ from docker/docker does, which AFAICT falls back from schema2 to schema1 on any error.
(WIP because I don’t have the entire set of integration tests written, in particular using non-OpenShift registries; but the primary motivation, support for the OpenShift
acceptschema2configuration option, does work correctly. EDIT Also the error messages might need tweaking.)EDIT Integration tests exist, error messages are acceptable.