8000 Implementation of the Manifest Service API refactor. by RichardScothern · Pull Request #1268 · distribution/distribution · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implementation of the Manifest Service API refactor. #1268

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

Merged

Conversation

RichardScothern
Copy link

Add a generic Manifest interface to represent manifests in the registry and
remove references to schema specific manifests.

Add a ManifestBuilder to construct Manifest objects. Concrete manifest builders
will exist for each manifest type and implementations will contain manifest
specific data used to build a manifest.

schema1

Change the way data is stored in schema1.Manifest to enable Payload() to be used
to return complete Manifest JSON from the HTTP handler without knowledge of the
schema1 protocol.

tags

Move tag functionality to a seperate TagService and update ManifestService
to use the new interfaces. Implement a driver based tagService to be backward
compatible with the current tag service.

Add a proxyTagService to enable the registry to get a digest for remote manifests
from a tag.

manifest store

Remove revision store and move all signing functionality into the signed manifeststore.

manifest registration

Add a mechanism to register manifest media types and to allow different manifest
types to be Unmarshalled correctly.

client

Add ManifestServiceOptions to client functions to allow tags to be passed into Put and
Get for building correct registry URLs. Change functional arguments to be an interface type
to allow passing data without mutating shared state.

Signed-off-by: Richard Scothern richard.scothern@docker.com

if err != nil {
return nil, err
panic("digesting canonical")
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of thing is all the more reason to merge #1264 :)

if err := json.Unmarshal(raw, &sm); err != nil {
return nil, err
}
ms.Enumerate(ctx, make([]distribution.Manifest, 1), sm)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this Enumerate call for?

@stevvooe
Copy link
Collaborator

This is a solid crack at the task. Most of the commentary above is clarifications.


func (ms *manifests) Exists(ctx context.Context, dgst digest.Digest) (bool, error) {
// Call by Tag endpoint since the API uses the same
// URL endpoint for tags and digests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comment.

return nil, err
}
// Target returns the target of this signed manifest
func (sm SignedManifest) Target() distribution.Descriptor {
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding back a comment that GitHub thinks is against an outdated diff:

This function doesn't really make sense to me. It's returning a descriptor for the manifest itself as the target. It seems a bit recursive, and inconsistent with what Target will do for a schema2 manifest.

There isn't really a sensible implementation of Target for schema1. In the original proposal thread, I suggested that Target should not be part of the Manifest interface, because not all manifest types would be able to implement it. Looking over my preliminary schema2 code, all uses of Target do a type assertion to *schema2.DeserializedManifest before calling it. How about just removing Target from the interface and removing this implementation?

Adding to that: Target doesn't make sense for manifest lists either. It's very schema2-specific.

Copy link
Author

Choose a reason for hiding this comment

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

Given that it's valid for a minority of manifests, I am OK with removing it.

@aaronlehmann
Copy link
Contributor

LGTM

Please squash commits though.

Add a generic Manifest interface to represent manifests in the registry and
remove references to schema specific manifests.

Add a ManifestBuilder to construct Manifest objects. Concrete manifest builders
will exist for each manifest type and implementations will contain manifest
specific data used to build a manifest.

Remove Signatures() from Repository interface.

Signatures are relevant only to schema1 manifests.  Move access to the signature
store inside the schema1 manifestStore.  Add some API tests to verify
signature roundtripping.

schema1
-------

Change the way data is stored in schema1.Manifest to enable Payload() to be used
to return complete Manifest JSON from the HTTP handler without knowledge of the
schema1 protocol.

tags
----

Move tag functionality to a seperate TagService and update ManifestService
to use the new interfaces.  Implement a driver based tagService to be backward
compatible with the current tag service.

Add a proxyTagService to enable the registry to get a digest for remote manifests
from a tag.

manifest store
--------------

Remove revision store and move all signing functionality into the signed manifeststore.

manifest registration
---------------------

Add a mechanism to register manifest media types and to allow different manifest
types to be Unmarshalled correctly.

client
------

Add ManifestServiceOptions to client functions to allow tags to be passed into Put and
Get for building correct registry URLs.  Change functional arguments to be an interface type
to allow passing data without mutating shared state.

Signed-off-by: Richard Scothern <richard.scothern@gmail.com>

Signed-off-by: Richard Scothern <richard.scothern@docker.com>
@stevvooe
Copy link
Collaborator

LGTM

RichardScothern pushed a commit that referenced this pull request Dec 18, 2015
Implementation of the Manifest Service API refactor.
@RichardScothern RichardScothern merged commit 67d3675 into distribution:master Dec 18, 2015
@RichardScothern RichardScothern deleted the manifest-refactor-impl branch February 5, 2016 18:31
thaJeztah pushed a commit to thaJeztah/distribution that referenced this pull request Apr 22, 2021
…factor-impl

Implementation of the Manifest Service API refactor.
thaJeztah pushed a commit to thaJeztah/distribution that referenced this pull request Jan 19, 2022
…factor-impl

Implementation of the Manifest Service API refactor.
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.

4 participants
0