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>
This commit is contained in:
Miloslav Trmač 2016-11-19 03:03:40 +01:00
parent 29838ec474
commit d742d1caa4
5 changed files with 326 additions and 15 deletions

View File

@ -14,6 +14,7 @@ import (
"time"
"github.com/Sirupsen/logrus"
"github.com/containers/image/docker/reference"
"github.com/containers/image/manifest"
"github.com/containers/image/types"
"github.com/docker/engine-api/client"
@ -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,

View File

@ -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)
}

View File

@ -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 dont 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)
}
// FIXME FIXME: NewReference?
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)
}
// 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 cant 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")
}

View File

@ -0,0 +1,236 @@
package daemon
import (
"testing"
"github.com/containers/image/docker/reference"
"github.com/containers/image/types"
"github.com/docker/distribution/digest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
sha256digestHex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
sha256digest = "sha256:" + sha256digestHex
)
func TestTransportName(t *testing.T) {
assert.Equal(t, "docker-daemon", Transport.Name())
}
func TestTransportParseReference(t *testing.T) {
testParseReference(t, Transport.ParseReference)
}
func TestTransportValidatePolicyConfigurationScope(t *testing.T) {
for _, scope := range []string{ // A semi-representative assortment of values; everything is rejected.
sha256digestHex,
sha256digest,
"docker.io/library/busybox:latest",
"docker.io",
"",
} {
err := Transport.ValidatePolicyConfigurationScope(scope)
assert.Error(t, err, scope)
}
}
func TestParseReference(t *testing.T) {
testParseReference(t, ParseReference)
}
// testParseReference is a test shared for Transport.ParseReference and ParseReference.
func testParseReference(t *testing.T, fn func(string) (types.ImageReference, error)) {
for _, c := range []struct{ input, expectedID, expectedRef string }{
{sha256digest, sha256digest, ""}, // Valid digest format
{"sha512:" + sha256digestHex + sha256digestHex, "", ""}, // Non-digest.Canonical digest
{"sha256:ab", "", ""}, // Invalid digest value (too short)
{sha256digest + "ab", "", ""}, // Invalid digest value (too long)
{"sha256:XX23456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", "", ""}, // Invalid digest value
{"UPPERCASEISINVALID", "", ""}, // Invalid reference input
{"busybox", "", ""}, // Missing tag or digest
{"busybox:latest", "", "busybox:latest"}, // Explicit tag
{"busybox@" + sha256digest, "", "busybox@" + sha256digest}, // Explicit digest
// A github.com/distribution/reference value can have a tag and a digest at the same time!
// github.com/docker/reference handles that by dropping the tag. That is not obviously the
// right thing to do, but it is at least reasonable, so test that we keep behaving reasonably.
// This test case should not be construed to make this an API promise.
// FIXME? Instead work extra hard to reject such input?
{"busybox:latest@" + sha256digest, "", "busybox@" + sha256digest}, // Both tag and digest
{"docker.io/library/busybox:latest", "", "busybox:latest"}, // All implied values explicitly specified
} {
ref, err := fn(c.input)
if c.expectedID == "" && c.expectedRef == "" {
assert.Error(t, err, c.input)
} else {
require.NoError(t, err, c.input)
daemonRef, ok := ref.(daemonReference)
require.True(t, ok, c.input)
// If we don't reject the input, the interpretation must be consistent for reference.ParseIDOrReference
dockerID, dockerRef, err := reference.ParseIDOrReference(c.input)
require.NoError(t, err, c.input)
if c.expectedRef == "" {
assert.Equal(t, c.expectedID, daemonRef.id.String(), c.input)
assert.Nil(t, daemonRef.ref, c.input)
assert.Equal(t, c.expectedID, dockerID.String(), c.input)
assert.Nil(t, dockerRef, c.input)
} else {
assert.Equal(t, "", daemonRef.id.String(), c.input)
require.NotNil(t, daemonRef.ref, c.input)
assert.Equal(t, c.expectedRef, daemonRef.ref.String(), c.input)
assert.Equal(t, "", dockerID.String(), c.input)
require.NotNil(t, dockerRef, c.input)
assert.Equal(t, c.expectedRef, dockerRef.String(), c.input)
}
}
}
}
// refWithTagAndDigest is a reference.NamedTagged and reference.Canonical at the same time.
type refWithTagAndDigest struct{ reference.Canonical }
func (ref refWithTagAndDigest) Tag() string {
return "notLatest"
}
// A common list of reference formats to test for the various ImageReference methods.
// (For IDs it is much simpler, we simply use them unmodified)
var validNamedReferenceTestCases = []struct{ input, dockerRef, stringWithinTransport string }{
{"busybox:notlatest", "busybox:notlatest", "busybox:notlatest"}, // Explicit tag
{"busybox" + sha256digest, "busybox" + sha256digest, "busybox" + sha256digest}, // Explicit digest
{"docker.io/library/busybox:latest", "busybox:latest", "busybox:latest"}, // All implied values explicitly specified
{"example.com/ns/foo:bar", "example.com/ns/foo:bar", "example.com/ns/foo:bar"}, // All values explicitly specified
}
func TestNewReference(t *testing.T) {
// An ID reference.
id, err := digest.ParseDigest(sha256digest)
require.NoError(t, err)
ref, err := NewReference(id, nil)
require.NoError(t, err)
daemonRef, ok := ref.(daemonReference)
require.True(t, ok)
assert.Equal(t, id, daemonRef.id)
assert.Nil(t, daemonRef.ref)
// Named references
for _, c := range validNamedReferenceTestCases {
parsed, err := reference.ParseNamed(c.input)
require.NoError(t, err)
ref, err := NewReference("", parsed)
require.NoError(t, err, c.input)
daemonRef, ok := ref.(daemonReference)
require.True(t, ok, c.input)
assert.Equal(t, "", daemonRef.id.String())
require.NotNil(t, daemonRef.ref)
assert.Equal(t, c.dockerRef, daemonRef.ref.String(), c.input)
}
// Both an ID and a named reference provided
parsed, err := reference.ParseNamed("busybox:latest")
require.NoError(t, err)
_, err = NewReference(id, parsed)
assert.Error(t, err)
// A reference with neither a tag nor digest
parsed, err = reference.ParseNamed("busybox")
require.NoError(t, err)
_, err = NewReference("", parsed)
assert.Error(t, err)
// A github.com/distribution/reference value can have a tag and a digest at the same time!
parsed, err = reference.ParseNamed("busybox@" + sha256digest)
require.NoError(t, err)
refDigested, ok := parsed.(reference.Canonical)
require.True(t, ok)
tagDigestRef := refWithTagAndDigest{refDigested}
_, err = NewReference("", tagDigestRef)
assert.Error(t, err)
}
func TestReferenceTransport(t *testing.T) {
ref, err := ParseReference(sha256digest)
require.NoError(t, err)
assert.Equal(t, Transport, ref.Transport())
ref, err = ParseReference("busybox:latest")
require.NoError(t, err)
assert.Equal(t, Transport, ref.Transport())
}
func TestReferenceStringWithinTransport(t *testing.T) {
ref, err := ParseReference(sha256digest)
require.NoError(t, err)
assert.Equal(t, sha256digest, ref.StringWithinTransport())
for _, c := range validNamedReferenceTestCases {
ref, err := ParseReference(c.input)
require.NoError(t, err, c.input)
stringRef := ref.StringWithinTransport()
assert.Equal(t, c.stringWithinTransport, stringRef, c.input)
// Do one more round to verify that the output can be parsed, to an equal value.
ref2, err := Transport.ParseReference(stringRef)
require.NoError(t, err, c.input)
stringRef2 := ref2.StringWithinTransport()
assert.Equal(t, stringRef, stringRef2, c.input)
}
}
func TestReferenceDockerReference(t *testing.T) {
ref, err := ParseReference(sha256digest)
require.NoError(t, err)
assert.Nil(t, ref.DockerReference())
for _, c := range validNamedReferenceTestCases {
ref, err := ParseReference(c.input)
require.NoError(t, err, c.input)
dockerRef := ref.DockerReference()
require.NotNil(t, dockerRef, c.input)
assert.Equal(t, c.dockerRef, dockerRef.String(), c.input)
}
}
func TestReferencePolicyConfigurationIdentity(t *testing.T) {
ref, err := ParseReference(sha256digest)
require.NoError(t, err)
assert.Equal(t, "", ref.PolicyConfigurationIdentity())
for _, c := range validNamedReferenceTestCases {
ref, err := ParseReference(c.input)
require.NoError(t, err, c.input)
assert.Equal(t, "", ref.PolicyConfigurationIdentity(), c.input)
}
}
func TestReferencePolicyConfigurationNamespaces(t *testing.T) {
ref, err := ParseReference(sha256digest)
require.NoError(t, err)
assert.Empty(t, ref.PolicyConfigurationNamespaces())
for _, c := range validNamedReferenceTestCases {
ref, err := ParseReference(c.input)
require.NoError(t, err, c.input)
assert.Empty(t, ref.PolicyConfigurationNamespaces(), c.input)
}
}
// daemonReference.NewImage, daemonReference.NewImageSource, openshiftReference.NewImageDestination
// untested because just creating the objects immediately connects to the daemon.
func TestReferenceDeleteImage(t *testing.T) {
ref, err := ParseReference(sha256digest)
require.NoError(t, err)
err = ref.DeleteImage(nil)
assert.Error(t, err)
for _, c := range validNamedReferenceTestCases {
ref, err := ParseReference(c.input)
require.NoError(t, err, c.input)
err = ref.DeleteImage(nil)
assert.Error(t, err, c.input)
}
}

View File

@ -32,7 +32,8 @@ func TestImageNameHandling(t *testing.T) {
{"dir", "/etc", "/etc"},
{"docker", "//busybox", "//busybox:latest"},
{"docker", "//busybox:notlatest", "//busybox:notlatest"}, // This also tests handling of multiple ":" characters
{"docker-daemon", "FIXME FIXME", "FIXME FIXME"},
{"docker-daemon", "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"},
{"docker-daemon", "busybox:latest", "busybox:latest"},
{"oci", "/etc:sometag", "/etc:sometag"},
// "atomic" not tested here because it depends on per-user configuration for the default cluster.
} {