Finish daemonReference

Allow either a !NameOnly named reference, or a sha256:hex digest.  Both
forms can be used for an ImageSource; ImageDestination accepts only a
name:tag value.

Because the sha256:hex reference values make it impossible to create
a reasonable policy hierarchy, only support a trivial namespace with
a single per-transport policy.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

docker/daemon/daemon_dest.go

@@ -16,6 +16,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/types"
+	"github.com/docker/docker/reference"
 	"github.com/docker/engine-api/client"
 	"golang.org/x/net/context"
 )
@@ -33,7 +34,13 @@ type daemonImageDestination struct {
 
 // newImageDestination returns a types.ImageDestination for the specified image reference.
 func newImageDestination(systemCtx *types.SystemContext, ref daemonReference) (types.ImageDestination, error) {
-	// FIXME: Do something with ref
+	if ref.ref == nil {
+		return nil, fmt.Errorf("Invalid destination docker-daemon:%s: a destination must be a name:tag", ref.StringWithinTransport())
+	}
+	if _, ok := ref.ref.(reference.NamedTagged); !ok {
+		return nil, fmt.Errorf("Invalid destination docker-daemon:%s: a destination must be a name:tag", ref.StringWithinTransport())
+	}
+
 	c, err := client.NewClient(client.DefaultDockerHost, "1.22", nil, nil) // FIXME: overridable host
 	if err != nil {
 		return nil, fmt.Errorf("Error initializing docker engine client: %v", err)
@@ -176,7 +183,7 @@ func (d *daemonImageDestination) PutManifest(m []byte) error {
 	}
 	items := []manifestItem{{
 		Config:       man.Config.Digest,
-		RepoTags:     []string{string(d.ref)}, // FIXME: Only if ref is a NamedTagged
+		RepoTags:     []string{d.ref.ref.String()}, // newImageDestination ensures that d.ref.ref is a reference.NamedTagged
 		Layers:       layerPaths,
 		Parent:       "",
 		LayerSources: nil,

docker/daemon/daemon_src.go

@@ -52,7 +52,9 @@ func newImageSource(ctx *types.SystemContext, ref daemonReference) (types.ImageS
 	if err != nil {
 		return nil, fmt.Errorf("Error initializing docker engine client: %v", err)
 	}
-	inputStream, err := c.ImageSave(context.TODO(), []string{string(ref)}) // FIXME: ref should be per docker/reference.ParseIDOrReference, and we don't want NameOnly
+	// Per NewReference(), ref.StringWithinTransport() is either an image ID (config digest), or a !reference.NameOnly() reference.
+	// Either way ImageSave should create a tarball with exactly one image.
+	inputStream, err := c.ImageSave(context.TODO(), []string{ref.StringWithinTransport()})
 	if err != nil {
 		return nil, fmt.Errorf("Error loading image from docker engine: %v", err)
 	}

docker/daemon/daemon_transport.go

@@ -1,11 +1,13 @@
 package daemon
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/image"
 	"github.com/containers/image/types"
+	"github.com/docker/distribution/digest"
 )
 
 // Transport is an ImageTransport for images managed by a local Docker daemon.
@@ -28,19 +30,69 @@ func (t daemonTransport) ParseReference(reference string) (types.ImageReference,
 // It is acceptable to allow an invalid value which will never be matched, it can "only" cause user confusion.
 // scope passed to this function will not be "", that value is always allowed.
 func (t daemonTransport) ValidatePolicyConfigurationScope(scope string) error {
-	// FIXME FIXME
-	return nil
+	// See the explanation in daemonReference.PolicyConfigurationIdentity.
+	return errors.New(`docker-daemon: does not support any scopes except the default "" one`)
 }
 
-// daemonReference is an ImageReference for images managed by a local Docker daemon.
-type daemonReference string // FIXME FIXME
+// daemonReference is an ImageReference for images managed by a local Docker daemon
+// Exactly one of id and ref can be set.
+// For daemonImageSource, both id and ref are acceptable, ref must not be a NameOnly (interpreted as all tags in that repository by the daemon)
+// For daemonImageDestination, it must be a ref, which is NamedTagged.
+// (We could, in principle, also allow storing images without tagging them, and the user would have to refer to them using the docker image ID = config digest.
+//  Using the config digest requires the caller to parse the manifest themselves, which is very cumbersome; so, for now, we don’t bother.)
+type daemonReference struct {
+	id  digest.Digest
+	ref reference.Named // !reference.IsNameOnly
+}
 
 // ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an ImageReference.
-func ParseReference(reference string) (types.ImageReference, error) {
-	return daemonReference(reference), nil // FIXME FIXME
+func ParseReference(refString string) (types.ImageReference, error) {
+	// This is intended to be compatible with reference.ParseIDOrReference, but more strict about refusing some of the ambiguous cases.
+	// In particular, this rejects unprefixed digest values (64 hex chars), and sha256 digest prefixes (sha256:fewer-than-64-hex-chars).
+
+	// digest:hexstring is structurally the same as a reponame:tag (meaning docker.io/library/reponame:tag).
+	// reference.ParseIDOrReference interprets such strings as digests.
+	if dgst, err := digest.ParseDigest(refString); err == nil {
+		// The daemon explicitly refuses to tag images with a reponame equal to digest.Canonical - but _only_ this digest name.
+		// Other digest references are ambiguous, so refuse them.
+		if dgst.Algorithm() != digest.Canonical {
+			return nil, fmt.Errorf("Invalid docker-daemon: reference %s: only digest algorithm %s accepted", refString, digest.Canonical)
+		}
+		return NewReference(dgst, nil)
+	}
+
+	ref, err := reference.ParseNamed(refString) // This also rejects unprefixed digest values
+	if err != nil {
+		return nil, err
+	}
+	if ref.Name() == digest.Canonical.String() {
+		return nil, fmt.Errorf("Invalid docker-daemon: reference %s: The %s repository name is reserved for (non-shortened) digest references", refString, digest.Canonical)
+	}
+	return NewReference("", ref)
 }
 
-// FIXME FIXME: NewReference?
+// NewReference returns a docker-daemon reference for either the supplied image ID (config digest) or the supplied reference (which must satisfy !reference.IsNameOnly)
+func NewReference(id digest.Digest, ref reference.Named) (types.ImageReference, error) {
+	if id != "" && ref != nil {
+		return nil, errors.New("docker-daemon: reference must not have an image ID and a reference string specified at the same time")
+	}
+	if ref != nil {
+		if reference.IsNameOnly(ref) {
+			return nil, fmt.Errorf("docker-daemon: reference %s has neither a tag nor a digest", ref.String())
+		}
+		// A github.com/distribution/reference value can have a tag and a digest at the same time!
+		// docker/reference does not handle that, so fail.
+		_, isTagged := ref.(reference.NamedTagged)
+		_, isDigested := ref.(reference.Canonical)
+		if isTagged && isDigested {
+			return nil, fmt.Errorf("docker-daemon: references with both a tag and digest are currently not supported")
+		}
+	}
+	return daemonReference{
+		id:  id,
+		ref: ref,
+	}, nil
+}
 
 func (ref daemonReference) Transport() types.ImageTransport {
 	return Transport
@@ -53,14 +105,21 @@ func (ref daemonReference) Transport() types.ImageTransport {
 // WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix;
 // instead, see transports.ImageName().
 func (ref daemonReference) StringWithinTransport() string {
-	return string(ref) // FIXME FIXME
+	switch {
+	case ref.id != "":
+		return ref.id.String()
+	case ref.ref != nil:
+		return ref.ref.String()
+	default: // Coverage: Should never happen, NewReference above should refuse such values.
+		panic("Internal inconsistency: daemonReference has empty id and nil ref")
+	}
 }
 
 // DockerReference returns a Docker reference associated with this reference
 // (fully explicit, i.e. !reference.IsNameOnly, but reflecting user intent,
 // not e.g. after redirect or alias processing), or nil if unknown/not applicable.
 func (ref daemonReference) DockerReference() reference.Named {
-	return nil // FIXME FIXME
+	return ref.ref // May be nil
 }
 
 // PolicyConfigurationIdentity returns a string representation of the reference, suitable for policy lookup.
@@ -71,7 +130,10 @@ func (ref daemonReference) DockerReference() reference.Named {
 // not required/guaranteed that it will be a valid input to Transport().ParseReference().
 // Returns "" if configuration identities for these references are not supported.
 func (ref daemonReference) PolicyConfigurationIdentity() string {
-	return string(ref) // FIXME FIXME
+	// We must allow referring to images in the daemon by image ID, otherwise untagged images would not be accessible.
+	// But the existence of image IDs means that we can’t truly well namespace the input; the untagged images would have to fall into the default policy,
+	// which can be unexpected.  So, punt.
+	return "" // This still allows using the default "" scope to define a policy for this transport.
 }
 
 // PolicyConfigurationNamespaces returns a list of other policy configuration namespaces to search
@@ -110,5 +172,8 @@ func (ref daemonReference) NewImageDestination(ctx *types.SystemContext) (types.
 
 // DeleteImage deletes the named image from the registry, if supported.
 func (ref daemonReference) DeleteImage(ctx *types.SystemContext) error {
-	return fmt.Errorf("Deleting images not implemented for docker-daemon: images") // FIXME FIXME?
+	// Should this just untag the image? Should this stop running containers?
+	// The semantics is not quite as clear as for remote repositories.
+	// The user can run (docker rmi) directly anyway, so, for now(?), punt instead of trying to guess what the user meant.
+	return fmt.Errorf("Deleting images not implemented for docker-daemon: images")
 }