Merge pull request #58 from mtrmac/api-general

General image API cleanups

cmd/skopeo/copy.go

@@ -54,7 +54,7 @@ func copyHandler(context *cli.Context) {
 	}
 	signBy := context.String("sign-by")
 
-	manifest, _, err := src.GetManifest()
+	manifest, err := src.GetManifest()
 	if err != nil {
 		logrus.Fatalf("Error reading manifest: %s", err.Error())
 	}

cmd/skopeo/inspect.go

@@ -3,11 +3,27 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"time"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/codegangsta/cli"
+	"github.com/projectatomic/skopeo/dockerutils"
 )
 
+// inspectOutput is the output format of (skopeo inspect), primarily so that we can format it with a simple json.MarshalIndent.
+type inspectOutput struct {
+	Name          string
+	Tag           string
+	Digest        string
+	RepoTags      []string
+	Created       time.Time
+	DockerVersion string
+	Labels        map[string]string
+	Architecture  string
+	Os            string
+	Layers        []string
+}
+
 var inspectCmd = cli.Command{
 	Name:  "inspect",
 	Usage: "inspect images on a registry",
@@ -22,19 +38,39 @@ var inspectCmd = cli.Command{
 		if err != nil {
 			logrus.Fatal(err)
 		}
+		rawManifest, err := img.Manifest()
+		if err != nil {
+			logrus.Fatal(err)
+		}
 		if c.Bool("raw") {
-			b, err := img.Manifest()
-			if err != nil {
-				logrus.Fatal(err)
-			}
-			fmt.Println(string(b))
+			fmt.Println(string(rawManifest))
 			return
 		}
 		imgInspect, err := img.Inspect()
 		if err != nil {
 			logrus.Fatal(err)
 		}
-		out, err := json.MarshalIndent(imgInspect, "", "    ")
+		manifestDigest, err := dockerutils.ManifestDigest(rawManifest)
+		if err != nil {
+			logrus.Fatalf("Error computing manifest digest: %s", err.Error())
+		}
+		repoTags, err := img.GetRepositoryTags()
+		if err != nil {
+			logrus.Fatalf("Error determining repository tags: %s", err.Error())
+		}
+		outputData := inspectOutput{
+			Name:          imgInspect.Name,
+			Tag:           imgInspect.Tag,
+			Digest:        manifestDigest,
+			RepoTags:      repoTags,
+			Created:       imgInspect.Created,
+			DockerVersion: imgInspect.DockerVersion,
+			Labels:        imgInspect.Labels,
+			Architecture:  imgInspect.Architecture,
+			Os:            imgInspect.Os,
+			Layers:        imgInspect.Layers,
+		}
+		out, err := json.MarshalIndent(outputData, "", "    ")
 		if err != nil {
 			logrus.Fatal(err)
 		}

directory/directory.go

@@ -84,13 +84,8 @@ func (s *dirImageSource) IntendedDockerReference() string {
 	return ""
 }
 
-func (s *dirImageSource) GetManifest() ([]byte, string, error) {
-	manifest, err := ioutil.ReadFile(manifestPath(s.dir))
-	if err != nil {
-		return nil, "", err
-	}
-
-	return manifest, "", nil // FIXME? unverifiedCanonicalDigest value - really primarily used by dockerImage
+func (s *dirImageSource) GetManifest() ([]byte, error) {
+	return ioutil.ReadFile(manifestPath(s.dir))
 }
 
 func (s *dirImageSource) GetLayer(digest string) (io.ReadCloser, error) {

docker/docker_image.go

@@ -20,8 +20,7 @@ var (
 
 type dockerImage struct {
 	src              *dockerImageSource
-	digest           string
-	rawManifest      []byte
+	cachedManifest   []byte   // Private cache for Manifest(); nil if not yet known.
 	cachedSignatures [][]byte // Private cache for Signatures(); nil if not yet known.
 }
 
@@ -44,10 +43,14 @@ func (i *dockerImage) IntendedDockerReference() string {
 
 // Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.
 func (i *dockerImage) Manifest() ([]byte, error) {
-	if err := i.retrieveRawManifest(); err != nil {
-		return nil, err
+	if i.cachedManifest == nil {
+		m, err := i.src.GetManifest()
+		if err != nil {
+			return nil, err
+		}
+		i.cachedManifest = m
 	}
-	return i.rawManifest, nil
+	return i.cachedManifest, nil
 }
 
 // Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
@@ -62,7 +65,7 @@ func (i *dockerImage) Signatures() ([][]byte, error) {
 	return i.cachedSignatures, nil
 }
 
-func (i *dockerImage) Inspect() (types.ImageManifest, error) {
+func (i *dockerImage) Inspect() (*types.ImageInspectInfo, error) {
 	// TODO(runcom): unused version param for now, default to docker v2-1
 	m, err := i.getSchema1Manifest()
 	if err != nil {
@@ -72,18 +75,24 @@ func (i *dockerImage) Inspect() (types.ImageManifest, error) {
 	if !ok {
 		return nil, fmt.Errorf("error retrivieng manifest schema1")
 	}
-	tags, err := i.getTags()
-	if err != nil {
+	v1 := &v1Image{}
+	if err := json.Unmarshal([]byte(ms1.History[0].V1Compatibility), v1); err != nil {
 		return nil, err
 	}
-	imgManifest, err := makeImageManifest(i.src.ref.FullName(), ms1, i.digest, tags)
-	if err != nil {
-		return nil, err
-	}
-	return imgManifest, nil
+	return &types.ImageInspectInfo{
+		Name:          i.src.ref.FullName(),
+		Tag:           ms1.Tag,
+		DockerVersion: v1.DockerVersion,
+		Created:       v1.Created,
+		Labels:        v1.Config.Labels,
+		Architecture:  v1.Architecture,
+		Os:            v1.OS,
+		Layers:        ms1.GetLayers(),
+	}, nil
 }
 
-func (i *dockerImage) getTags() ([]string, error) {
+// GetRepositoryTags list all tags available in the repository. Note that this has no connection with the tag(s) used for this specific image, if any.
+func (i *dockerImage) GetRepositoryTags() ([]string, error) {
 	// FIXME? Breaking the abstraction.
 	url := fmt.Sprintf(tagsURL, i.src.ref.RemoteName())
 	res, err := i.src.c.makeRequest("GET", url, nil, nil)
@@ -122,25 +131,6 @@ type v1Image struct {
 	OS string `json:"os,omitempty"`
 }
 
-func makeImageManifest(name string, m *manifestSchema1, dgst string, tagList []string) (types.ImageManifest, error) {
-	v1 := &v1Image{}
-	if err := json.Unmarshal([]byte(m.History[0].V1Compatibility), v1); err != nil {
-		return nil, err
-	}
-	return &types.DockerImageManifest{
-		Name:          name,
-		Tag:           m.Tag,
-		Digest:        dgst,
-		RepoTags:      tagList,
-		DockerVersion: v1.DockerVersion,
-		Created:       v1.Created,
-		Labels:        v1.Config.Labels,
-		Architecture:  v1.Architecture,
-		Os:            v1.OS,
-		Layers:        m.GetLayers(),
-	}, nil
-}
-
 // TODO(runcom)
 func (i *dockerImage) DockerTar() ([]byte, error) {
 	return nil, nil
@@ -181,25 +171,13 @@ func sanitize(s string) string {
 	return strings.Replace(s, "/", "-", -1)
 }
 
-func (i *dockerImage) retrieveRawManifest() error {
-	if i.rawManifest != nil {
-		return nil
-	}
-	manblob, unverifiedCanonicalDigest, err := i.src.GetManifest()
-	if err != nil {
-		return err
-	}
-	i.rawManifest = manblob
-	i.digest = unverifiedCanonicalDigest
-	return nil
-}
-
 func (i *dockerImage) getSchema1Manifest() (manifest, error) {
-	if err := i.retrieveRawManifest(); err != nil {
+	manblob, err := i.Manifest()
+	if err != nil {
 		return nil, err
 	}
 	mschema1 := &manifestSchema1{}
-	if err := json.Unmarshal(i.rawManifest, mschema1); err != nil {
+	if err := json.Unmarshal(manblob, mschema1); err != nil {
 		return nil, err
 	}
 	if err := fixManifestLayers(mschema1); err != nil {

docker/docker_image_src.go

@@ -55,23 +55,24 @@ func (s *dockerImageSource) IntendedDockerReference() string {
 	return fmt.Sprintf("%s:%s", s.ref.Name(), s.tag)
 }
 
-func (s *dockerImageSource) GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) {
+func (s *dockerImageSource) GetManifest() ([]byte, 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
 	// TODO(runcom) NO, switch on the resulter manifest like Docker is doing
 	res, err := s.c.makeRequest("GET", url, nil, nil)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	defer res.Body.Close()
 	manblob, err := ioutil.ReadAll(res.Body)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	if res.StatusCode != http.StatusOK {
-		return nil, "", errFetchManifest{res.StatusCode, manblob}
+		return nil, errFetchManifest{res.StatusCode, manblob}
 	}
-	return manblob, res.Header.Get("Docker-Content-Digest"), nil
+	// We might validate manblob against the Docker-Content-Digest header here to protect against transport errors.
+	return manblob, nil
 }
 
 func (s *dockerImageSource) GetLayer(digest string) (io.ReadCloser, error) {

openshift/openshift.go

@@ -193,9 +193,9 @@ func (s *openshiftImageSource) IntendedDockerReference() string {
 	return s.client.canonicalDockerReference()
 }
 
-func (s *openshiftImageSource) GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) {
+func (s *openshiftImageSource) GetManifest() ([]byte, error) {
 	if err := s.ensureImageIsResolved(); err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	return s.docker.GetManifest()
 }

types/types.go

@@ -29,13 +29,17 @@ type Repository interface {
 }
 
 // ImageSource is a service, possibly remote (= slow), to download components of a single image.
+// This is primarily useful for copying images around; for examining their properties, Image (below)
+// is usually more useful.
 type ImageSource interface {
 	// IntendedDockerReference 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.
 	IntendedDockerReference() string
 	// GetManifest returns the image's manifest.  It may use a remote (= slow) service.
-	GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error)
+	// FIXME? This should also return a MIME type if known, to differentiate between schema versions.
+	GetManifest() ([]byte, error)
+	// Note: Calling GetLayer() may have ordering dependencies WRT other methods of this type. FIXME: How does this work with (docker save) on stdin?
 	GetLayer(digest string) (io.ReadCloser, error)
 	// GetSignatures returns the image's signatures.  It may use a remote (= slow) service.
 	GetSignatures() ([][]byte, error)
@@ -45,12 +49,14 @@ type ImageSource interface {
 type ImageDestination interface {
 	// CanonicalDockerReference returns the full, unambiguous, Docker reference for this image (even if the user referred to the image using some shorthand notation).
 	CanonicalDockerReference() (string, error)
+	// FIXME? This should also receive a MIME type if known, to differentiate between schema versions.
 	PutManifest([]byte) error
+	// Note: Calling PutLayer() and other methods may have ordering dependencies WRT other methods of this type. FIXME: Figure out and document.
 	PutLayer(digest string, stream io.Reader) error
 	PutSignatures(signatures [][]byte) error
 }
 
-// Image is a Docker image in a repository.
+// Image is the primary API for inspecting properties of images.
 type Image interface {
 	// ref to repository?
 	// IntendedDockerReference returns the full, unambiguous, Docker reference for this image, _as specified by the user_
@@ -58,27 +64,23 @@ type Image interface {
 	// May be "" if unknown.
 	IntendedDockerReference() string
 	// Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need.
+	// FIXME? This should also return a MIME type if known, to differentiate between schema versions.
 	Manifest() ([]byte, error)
 	// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
 	Signatures() ([][]byte, error)
 	Layers(layers ...string) error // configure download directory? Call it DownloadLayers?
-	Inspect() (ImageManifest, error)
+	Inspect() (*ImageInspectInfo, error)
 	DockerTar() ([]byte, error) // ??? also, configure output directory
+	// GetRepositoryTags list all tags available in the repository. Note that this has no connection with the tag(s) used for this specific image, if any.
+	// Eventually we should move this away from the generic Image interface, and move it into a Docker-specific case within the (skopeo inspect) command,
+	// see https://github.com/projectatomic/skopeo/pull/58#discussion_r63411838 .
+	GetRepositoryTags() ([]string, error)
 }
 
-// ImageManifest is the interesting subset of metadata about an Image.
-// TODO(runcom)
-type ImageManifest interface {
-	String() string
-}
-
-// DockerImageManifest is a set of metadata describing Docker images and their manifest.json files.
-// Note that this is not exactly manifest.json, e.g. some fields have been added.
-type DockerImageManifest struct {
+// ImageInspectInfo is a set of metadata describing Docker images, primarily their manifest and configuration.
+type ImageInspectInfo struct {
 	Name          string
 	Tag           string
-	Digest        string
-	RepoTags      []string
 	Created       time.Time
 	DockerVersion string
 	Labels        map[string]string
@@ -87,6 +89,6 @@ type DockerImageManifest struct {
 	Layers        []string
 }
 
-func (m *DockerImageManifest) String() string {
+func (m *ImageInspectInfo) String() string {
 	return fmt.Sprintf("%s:%s", m.Name, m.Tag)
 }