Skip to content

API for signatures #57

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.

Sign up for GitHub

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
runcom merged 2 commits into from
May 16, 2016
Merged

API for signatures #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
feb9de4
Add GetManifest and GetSignatures to types.Image
mtrmac May 16, 2016
e4913bd
Add GetIntendedDockerReference to types.Image and types.ImageSource
mtrmac May 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions cmd/skopeo/inspect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ var inspectCmd = cli.Command{
logrus.Fatal(err)
}
if c.Bool("raw") {
// TODO(runcom): hardcoded schema 2 version 1
b, err := img.RawManifest("2-1")
b, err := img.GetManifest()
if err != nil {
logrus.Fatal(err)
}
Expand Down
7 changes: 7 additions & 0 deletions directory/directory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ func NewDirImageSource(dir string) types.ImageSource {
return &dirImageSource{dir}
}

// GetIntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (s *dirImageSource) GetIntendedDockerReference() string {
return ""
}

func (s *dirImageSource) GetManifest() ([]byte, string, error) {
manifest, err := ioutil.ReadFile(manifestPath(s.dir))
if err != nil {
Expand Down
30 changes: 25 additions & 5 deletions docker/docker_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ var (
)

type dockerImage struct {
src *dockerImageSource
digest string
rawManifest []byte
src *dockerImageSource
digest string
rawManifest []byte
cachedSignatures [][]byte // Private cache for GetSignatures; nil if not yet known.
}

// NewDockerImage returns a new Image interface type after setting up
Expand All @@ -34,14 +35,33 @@ func NewDockerImage(img, certPath string, tlsVerify bool) (types.Image, error) {
return &dockerImage{src: s}, nil
}

func (i *dockerImage) RawManifest(version string) ([]byte, error) {
// TODO(runcom): unused version param for now, default to docker v2-1
// GetIntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (i *dockerImage) GetIntendedDockerReference() string {
return i.src.GetIntendedDockerReference()
}

// GetManifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.
func (i *dockerImage) GetManifest() ([]byte, error) {
if err := i.retrieveRawManifest(); err != nil {
return nil, err
}
return i.rawManifest, nil
}

// GetSignatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *dockerImage) GetSignatures() ([][]byte, error) {
if i.cachedSignatures == nil {
sigs, err := i.src.GetSignatures()
if err != nil {
return nil, err
}
i.cachedSignatures = sigs
}
return i.cachedSignatures, nil
}

func (i *dockerImage) Manifest() (types.ImageManifest, error) {
// TODO(runcom): unused version param for now, default to docker v2-1
m, err := i.getSchema1Manifest()
Expand Down
7 changes: 7 additions & 0 deletions docker/docker_image_src.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ func NewDockerImageSource(img, certPath string, tlsVerify bool) (types.ImageSour
return newDockerImageSource(img, certPath, tlsVerify)
}

// GetIntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (s *dockerImageSource) GetIntendedDockerReference() string {
return fmt.Sprintf("%s:%s", s.ref.Name(), s.tag)
}

func (s *dockerImageSource) GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) {
url := fmt.Sprintf(manifestURL, s.ref.RemoteName(), s.tag)
// TODO(runcom) set manifest version header! schema1 for now - then schema2 etc etc and v1
Expand Down
16 changes: 15 additions & 1 deletion openshift/openshift.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ func (c *openshiftClient) doRequest(method, path string, requestBody []byte) ([]
return body, nil
}

// canonicalDockerReference returns a canonical reference we use for signing OpenShift images.
// FIXME: This is, strictly speaking, a namespace conflict with images placed in a Docker registry running on the same host.
// Do we need to do something else, perhaps disambiguate (port number?) or namespace Docker and OpenShift separately?
func (c *openshiftClient) canonicalDockerReference() string {
return fmt.Sprintf("%s/%s/%s:%s", c.baseURL.Host, c.namespace, c.stream, c.tag)
}

// convertDockerImageReference takes an image API DockerImageReference value and returns a reference we can actually use;
// currently OpenShift stores the cluster-internal service IPs here, which are unusable from the outside.
func (c *openshiftClient) convertDockerImageReference(ref string) (string, error) {
Expand Down Expand Up @@ -179,6 +186,13 @@ func NewOpenshiftImageSource(imageName, certPath string, tlsVerify bool) (types.
}, nil
}

// GetIntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
func (s *openshiftImageSource) GetIntendedDockerReference() string {
return s.client.canonicalDockerReference()
}

func (s *openshiftImageSource) GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) {
if err := s.ensureImageIsResolved(); err != nil {
return nil, "", err
Expand Down Expand Up @@ -270,7 +284,7 @@ func NewOpenshiftImageDestination(imageName, certPath string, tlsVerify bool) (t
}

func (d *openshiftImageDestination) CanonicalDockerReference() (string, error) {
return fmt.Sprintf("%s/%s/%s:%s", d.client.baseURL.Host, d.client.namespace, d.client.stream, d.client.tag), nil
return d.client.canonicalDockerReference(), nil
}

func (d *openshiftImageDestination) PutManifest(manifest []byte) error {
Expand Down
13 changes: 12 additions & 1 deletion types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ type Repository interface {

// ImageSource is a service, possibly remote (= slow), to download components of a single image.
type ImageSource interface {
// GetIntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
GetIntendedDockerReference() string
GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error)
GetLayer(digest string) (io.ReadCloser, error)
GetSignatures() ([][]byte, error)
Expand All @@ -47,9 +51,16 @@ type ImageDestination interface {
// Image is a Docker image in a repository.
type Image interface {
// ref to repository?
// GetIntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
// May be "" if unknown.
GetIntendedDockerReference() string
// GetManifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.
GetManifest() ([]byte, error)
// GetSignatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
GetSignatures() ([][]byte, error)
Copy link
Member

@runcom runcom May 16, 2016

Choose a reason for hiding this comment

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

nit, but I don't like Get(er) or setter in Go - calling image.Signatures is pretty straightforward and clean WDYT?

Copy link
Member

@runcom runcom May 16, 2016

Choose a reason for hiding this comment

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

alright, I've seen the comment in commit message - yet I prefer not to use Get|Set - is there anything specific that needs this Get|Set prefix?

Copy link
Member

@runcom runcom May 16, 2016

Choose a reason for hiding this comment

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

This is valid also for Put (which recall me of HTTP for instance) - is it ok to use something like UploadManifest? SendManifest? I like it much more

Copy link
Contributor Author

@mtrmac mtrmac May 16, 2016

Choose a reason for hiding this comment

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

For now the immediate barrier is that there already is a Manifest method. But #58 proposes renaming it to Inspect anyway…

Dropping the Get prefix is perfectly fine with me.

As for Put, Upload/Send are a bit weird with dirImageDestination; and generally ImageDestination may have ordering dependencies, e.g. see how openshiftImageDestination in https://github.com/mtrmac/skopeo/tree/openshift-annotation-sigs only stores signatures locally in PutSignatures and uploads them only in PutManifest. Upload/Send could be misleading in such a case — but honestly the whole ImageSource/ImageDestination will need to be rethought if we ever support non-seekable tarball streams.

Copy link
Member

@runcom runcom May 16, 2016

Choose a reason for hiding this comment

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

For now the immediate barrier is that there already is a Manifest method. But #58 proposes renaming it to Inspect anyway…

this is indeed awesome and what I like

Copy link
Member

@runcom runcom May 16, 2016

Choose a reason for hiding this comment

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

I'm fine merging this as-is given we could remove at least Get (#57 (comment))

Copy link
Member

@runcom runcom May 16, 2016

Choose a reason for hiding this comment

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

but honestly the whole ImageSource/ImageDestination will need to be rethought if we ever support non-seekable tarball streams.

agree, maybe they could be embedded directly in images and are available only as a mean of developing new internal source|destination exposed by types.Image directy

Copy link
Contributor Author

@mtrmac mtrmac May 16, 2016

Choose a reason for hiding this comment

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

I do think that the source/destination distinction is useful; but for sources, this PR already goes a long way towards most users using a types.Image and not caring about ImageSource at all (basically unless you need layer contents, in which case tarball streams and ordering etc. are really important, Image gives you the same information and the caching and parsing makes it the preferred interface.).

Layers(layers ...string) error // configure download directory? Call it DownloadLayers?
Manifest() (ImageManifest, error)
RawManifest(version string) ([]byte, error)
DockerTar() ([]byte, error) // ??? also, configure output directory
}

Expand Down