containers
/
image
mirror of https://github.com/containers/image.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

API transition: Drop XNamed.XString 

Instead call distreference.FamiliarString() for SOME uses, generally for
error messages and StringWithinTransport().

In signature/policy_reference_match.go and signature/docker.go, where we
care about equality but not exactly about the kind of normalization, call
XNamed.String() instead, with the same rationale as the earlier
Name/FamiliarName choice.

In copy.Image, when creating a singature, use .String() (i.e. the fully
explicit form), for that extra bit of safety.

In tests, generally use the simpler .String() and modify expected
results, instead of calling FamilarString().

XNamed is now equivalent to distreference.Named, all the extra methods
have went away.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2017-01-20 05:42:56 +01:00
parent 2b4f09f141
commit 7abfa9812f
16 changed files with 52 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
copy/copy.go
View File

@ -210,7 +210,7 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe
}
writeReport("Signing manifest\n")
newSig, err := signature.SignDockerManifest(manifest, dockerReference.XString(), mech, options.SignBy)
newSig, err := signature.SignDockerManifest(manifest, dockerReference.String(), mech, options.SignBy)
if err != nil {
return errors.Wrap(err, "Error creating signature")
}

4
docker/daemon/daemon_transport.go
View File

@ -78,7 +78,7 @@ func NewReference(id digest.Digest, ref reference.XNamed) (types.ImageReference,
}
if ref != nil {
if reference.XIsNameOnly(ref) {
return nil, errors.Errorf("docker-daemon: reference %s has neither a tag nor a digest", ref.XString())
return nil, errors.Errorf("docker-daemon: reference %s has neither a tag nor a digest", distreference.FamiliarString(ref))
}
// 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.
@ -109,7 +109,7 @@ func (ref daemonReference) StringWithinTransport() string {
case ref.id != "":
return ref.id.String()
case ref.ref != nil:
return ref.ref.XString()
return distreference.FamiliarString(ref.ref)
default: // Coverage: Should never happen, NewReference above should refuse such values.
panic("Internal inconsistency: daemonReference has empty id and nil ref")
}

24
docker/daemon/daemon_transport_test.go
View File

@ -50,15 +50,15 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err
{"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
{"busybox:latest", "", "docker.io/library/busybox:latest"}, // Explicit tag
{"busybox@" + sha256digest, "", "docker.io/library/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
{"busybox:latest@" + sha256digest, "", "docker.io/library/busybox@" + sha256digest}, // Both tag and digest
{"docker.io/library/busybox:latest", "", "docker.io/library/busybox:latest"}, // All implied values explicitly specified
} {
ref, err := fn(c.input)
if c.expectedID == "" && c.expectedRef == "" {
@ -80,11 +80,11 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err
} else {
assert.Equal(t, "", daemonRef.id.String(), c.input)
require.NotNil(t, daemonRef.ref, c.input)
assert.Equal(t, c.expectedRef, daemonRef.ref.XString(), 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.XString(), c.input)
assert.Equal(t, c.expectedRef, dockerRef.String(), c.input)
}
}
}
@ -100,10 +100,10 @@ func (ref refWithTagAndDigest) XTag() string {
// 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
{"busybox:notlatest", "docker.io/library/busybox:notlatest", "busybox:notlatest"}, // Explicit tag
{"busybox" + sha256digest, "docker.io/library/busybox" + sha256digest, "busybox" + sha256digest}, // Explicit digest
{"docker.io/library/busybox:latest", "docker.io/library/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) {
@ -127,7 +127,7 @@ func TestNewReference(t *testing.T) {
require.True(t, ok, c.input)
assert.Equal(t, "", daemonRef.id.String())
require.NotNil(t, daemonRef.ref)
assert.Equal(t, c.dockerRef, daemonRef.ref.XString(), c.input)
assert.Equal(t, c.dockerRef, daemonRef.ref.String(), c.input)
}
// Both an ID and a named reference provided
@ -190,7 +190,7 @@ func TestReferenceDockerReference(t *testing.T) {
require.NoError(t, err, c.input)
dockerRef := ref.DockerReference()
require.NotNil(t, dockerRef, c.input)
assert.Equal(t, c.dockerRef, dockerRef.XString(), c.input)
assert.Equal(t, c.dockerRef, dockerRef.String(), c.input)
}
}

7
docker/docker_transport.go
View File

@ -7,6 +7,7 @@ import (
"github.com/containers/image/docker/policyconfiguration"
"github.com/containers/image/docker/reference"
"github.com/containers/image/types"
distreference "github.com/docker/distribution/reference"
"github.com/pkg/errors"
)
@ -56,7 +57,7 @@ func ParseReference(refString string) (types.ImageReference, error) {
// NewReference returns a Docker reference for a named reference. The reference must satisfy !reference.XIsNameOnly().
func NewReference(ref reference.XNamed) (types.ImageReference, error) {
if reference.XIsNameOnly(ref) {
return nil, errors.Errorf("Docker reference %s has neither a tag nor a digest", ref.XString())
return nil, errors.Errorf("Docker reference %s has neither a tag nor a digest", distreference.FamiliarString(ref))
}
// 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.
@ -82,7 +83,7 @@ func (ref dockerReference) Transport() types.ImageTransport {
// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
// WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
func (ref dockerReference) StringWithinTransport() string {
return "//" + ref.ref.XString()
return "//" + distreference.FamiliarString(ref.ref)
}
// DockerReference returns a Docker reference associated with this reference
@ -152,5 +153,5 @@ func (ref dockerReference) tagOrDigest() (string, error) {
return ref.XTag(), nil
}
// This should not happen, NewReference above refuses reference.XIsNameOnly values.
return "", errors.Errorf("Internal inconsistency: Reference %s unexpectedly has neither a digest nor a tag", ref.ref.XString())
return "", errors.Errorf("Internal inconsistency: Reference %s unexpectedly has neither a digest nor a tag", distreference.FamiliarString(ref.ref))
}

28
docker/docker_transport_test.go
View File

@ -42,18 +42,18 @@ func TestParseReference(t *testing.T) {
// 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, expected string }{
{"busybox", ""}, // Missing // prefix
{"//busybox:notlatest", "busybox:notlatest"}, // Explicit tag
{"//busybox" + sha256digest, "busybox" + sha256digest}, // Explicit digest
{"//busybox", "busybox:latest"}, // Default tag
{"busybox", ""}, // Missing // prefix
{"//busybox:notlatest", "docker.io/library/busybox:notlatest"}, // Explicit tag
{"//busybox" + sha256digest, "docker.io/library/busybox" + sha256digest}, // Explicit digest
{"//busybox", "docker.io/library/busybox:latest"}, // Default tag
// 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
{"//UPPERCASEISINVALID", ""}, // Invalid input
{"//busybox:latest" + sha256digest, "docker.io/library/busybox" + sha256digest}, // Both tag and digest
{"//docker.io/library/busybox:latest", "docker.io/library/busybox:latest"}, // All implied values explicitly specified
{"//UPPERCASEISINVALID", ""}, // Invalid input
} {
ref, err := fn(c.input)
if c.expected == "" {
@ -62,7 +62,7 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err
require.NoError(t, err, c.input)
dockerRef, ok := ref.(dockerReference)
require.True(t, ok, c.input)
assert.Equal(t, c.expected, dockerRef.ref.XString(), c.input)
assert.Equal(t, c.expected, dockerRef.ref.String(), c.input)
}
}
}
@ -76,10 +76,10 @@ func (ref refWithTagAndDigest) XTag() string {
// A common list of reference formats to test for the various ImageReference methods.
var validReferenceTestCases = []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
{"busybox:notlatest", "docker.io/library/busybox:notlatest", "//busybox:notlatest"}, // Explicit tag
{"busybox" + sha256digest, "docker.io/library/busybox" + sha256digest, "//busybox" + sha256digest}, // Explicit digest
{"docker.io/library/busybox:latest", "docker.io/library/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) {
@ -90,7 +90,7 @@ func TestNewReference(t *testing.T) {
require.NoError(t, err, c.input)
dockerRef, ok := ref.(dockerReference)
require.True(t, ok, c.input)
assert.Equal(t, c.dockerRef, dockerRef.ref.XString(), c.input)
assert.Equal(t, c.dockerRef, dockerRef.ref.String(), c.input)
}
// Neither a tag nor digest
@ -135,7 +135,7 @@ func TestReferenceDockerReference(t *testing.T) {
require.NoError(t, err, c.input)
dockerRef := ref.DockerReference()
require.NotNil(t, dockerRef, c.input)
assert.Equal(t, c.dockerRef, dockerRef.XString(), c.input)
assert.Equal(t, c.dockerRef, dockerRef.String(), c.input)
}
}

2
docker/lookaside.go
View File

@ -66,7 +66,7 @@ func configuredSignatureStorageBase(ctx *types.SystemContext, ref dockerReferenc
// FIXME? Restrict to explicitly supported schemes?
repo := ref.ref.Name() // Note that this is without a tag or digest.
if path.Clean(repo) != repo { // Coverage: This should not be reachable because /./ and /../ components are not valid in docker references
return nil, errors.Errorf("Unexpected path elements in Docker reference %s for signature storage", ref.ref.XString())
return nil, errors.Errorf("Unexpected path elements in Docker reference %s for signature storage", ref.ref.String())
}
url.Path = url.Path + "/" + repo
return url, nil

5
docker/policyconfiguration/naming.go
View File

@ -6,6 +6,7 @@ import (
"github.com/pkg/errors"
"github.com/containers/image/docker/reference"
distreference "github.com/docker/distribution/reference"
)
// DockerReferenceIdentity returns a string representation of the reference, suitable for policy lookup,
@ -17,9 +18,9 @@ func DockerReferenceIdentity(ref reference.XNamed) (string, error) {
digested, isDigested := ref.(reference.XCanonical)
switch {
case isTagged && isDigested: // This should not happen, docker/reference.XParseNamed drops the tag.
return "", errors.Errorf("Unexpected Docker reference %s with both a name and a digest", ref.XString())
return "", errors.Errorf("Unexpected Docker reference %s with both a name and a digest", distreference.FamiliarString(ref))
case !isTagged && !isDigested: // This should not happen, the caller is expected to ensure !reference.XIsNameOnly()
return "", errors.Errorf("Internal inconsistency: Docker reference %s with neither a tag nor a digest", ref.XString())
return "", errors.Errorf("Internal inconsistency: Docker reference %s with neither a tag nor a digest", distreference.FamiliarString(ref))
case isTagged:
res = res + ":" + tagged.XTag()
case isDigested:

5
docker/reference/reference.go
View File

@ -26,8 +26,6 @@ const (
// XNamed is an object with a full name
type XNamed interface {
distreference.Named
// XString returns full reference, like "ubuntu@sha256:abcdef..."
XString() string
}
// XNamedTagged is an object including a name and tag.
@ -126,9 +124,6 @@ func (r *namedRef) Familiar() distreference.Named {
return r.Named.(drPRIVATEInterfaces).Familiar()
}
func (r *namedRef) XString() string {
return distreference.FamiliarString(r)
}
func (r *taggedRef) XTag() string {
return r.namedRef.Named.(distreference.NamedTagged).Tag()
}

2
docker/reference/reference_test.go
View File

@ -255,7 +255,7 @@ func TestParseReferenceWithTagAndDigest(t *testing.T) {
if _, isCanonical := ref.(XCanonical); !isCanonical {
t.Fatalf("Reference from %q should not support digest", ref)
}
if expected, actual := "busybox@sha256:86e0e091d0da6bde2456dbb48306f3956bbeb2eae1b5b9a43045843f69fe4aaa", ref.XString(); actual != expected {
if expected, actual := "busybox@sha256:86e0e091d0da6bde2456dbb48306f3956bbeb2eae1b5b9a43045843f69fe4aaa", distreference.FamiliarString(ref); actual != expected {
t.Fatalf("Invalid parsed reference for %q: expected %q, got %q", ref, expected, actual)
}
}

4
openshift/openshift_transport.go
View File

@ -68,7 +68,7 @@ func NewReference(dockerRef reference.XNamedTagged) (types.ImageReference, error
r := strings.SplitN(distreference.Path(dockerRef), "/", 3)
if len(r) != 2 {
return nil, errors.Errorf("invalid image reference: %s, expected format: 'hostname/namespace/stream:tag'",
dockerRef.XString())
distreference.FamiliarString(dockerRef))
}
return openshiftReference{
namespace: r[0],
@ -87,7 +87,7 @@ func (ref openshiftReference) Transport() types.ImageTransport {
// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa.
// WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix.
func (ref openshiftReference) StringWithinTransport() string {
return ref.dockerReference.XString()
return distreference.FamiliarString(ref.dockerReference)
}
// DockerReference returns a Docker reference associated with this reference

2
openshift/openshift_transport_test.go
View File

@ -80,7 +80,7 @@ func TestReferenceDockerReference(t *testing.T) {
require.NoError(t, err)
dockerRef := ref.DockerReference()
require.NotNil(t, dockerRef)
assert.Equal(t, "registry.example.com:8443/ns/stream:notlatest", dockerRef.XString())
assert.Equal(t, "registry.example.com:8443/ns/stream:notlatest", dockerRef.String())
}
func TestReferenceTransport(t *testing.T) {

2
signature/docker.go
View File

@ -41,7 +41,7 @@ func VerifyDockerManifestSignature(unverifiedSignature, unverifiedManifest []byt
if err != nil {
return InvalidSignatureError{msg: fmt.Sprintf("Invalid docker reference %s in signature", signedDockerReference)}
}
if signedRef.XString() != expectedRef.XString() {
if signedRef.String() != expectedRef.String() {
return InvalidSignatureError{msg: fmt.Sprintf("Docker reference %s does not match %s",
signedDockerReference, expectedDockerReference)}
}

6
signature/policy_reference_match.go
View File

@ -33,7 +33,7 @@ func (prm *prmMatchExact) matchesDockerReference(image types.UnparsedImage, sign
if reference.XIsNameOnly(intended) || reference.XIsNameOnly(signature) {
return false
}
return signature.XString() == intended.XString()
return signature.String() == intended.String()
}
func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {
@ -48,7 +48,7 @@ func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image types.Unparse
}
switch intended.(type) {
case reference.XNamedTagged: // Includes the case when intended has both a tag and a digest.
return signature.XString() == intended.XString()
return signature.String() == intended.String()
case reference.XCanonical:
// We don’t actually compare the manifest digest against the signature here; that happens prSignedBy.in UnparsedImage.Manifest.
// Becase UnparsedImage.Manifest verifies the intended.Digest() against the manifest, and prSignedBy verifies the signature digest against the manifest,
@ -89,7 +89,7 @@ func (prm *prmExactReference) matchesDockerReference(image types.UnparsedImage,
if reference.XIsNameOnly(intended) || reference.XIsNameOnly(signature) {
return false
}
return signature.XString() == intended.XString()
return signature.String() == intended.String()
}
func (prm *prmExactRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool {

10
signature/policy_reference_match_test.go
View File

@ -6,7 +6,7 @@ import (
"github.com/containers/image/docker/reference"
"github.com/containers/image/types"
distreference "github.com/docker/distribution/reference"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -30,8 +30,8 @@ func TestParseImageAndDockerReference(t *testing.T) {
require.NoError(t, err)
r1, r2, err := parseImageAndDockerReference(refImageMock{ref}, ok2)
require.NoError(t, err)
assert.Equal(t, ok1, r1.XString())
assert.Equal(t, ok2, r2.XString())
assert.Equal(t, ok1, distreference.FamiliarString(r1))
assert.Equal(t, ok2, distreference.FamiliarString(r2))
// Unidentified images are rejected.
_, _, err = parseImageAndDockerReference(refImageMock{nil}, ok2)
@ -307,8 +307,8 @@ func TestParseDockerReferences(t *testing.T) {
// Success
r1, r2, err := parseDockerReferences(ok1, ok2)
require.NoError(t, err)
assert.Equal(t, ok1, r1.XString())
assert.Equal(t, ok2, r2.XString())
assert.Equal(t, ok1, distreference.FamiliarString(r1))
assert.Equal(t, ok2, distreference.FamiliarString(r2))
// Failures
for _, refs := range [][]string{

2
storage/storage_reference_test.go
View File

@ -23,7 +23,7 @@ func TestStorageReferenceDockerReference(t *testing.T) {
require.NoError(t, err)
dr := ref.DockerReference()
require.NotNil(t, dr)
assert.Equal(t, "busybox:latest", dr.XString())
assert.Equal(t, "docker.io/library/busybox:latest", dr.String())
ref, err = Transport.ParseReference("@" + sha256digestHex)
require.NoError(t, err)

2
storage/storage_transport_test.go
View File

@ -57,7 +57,7 @@ func TestTransportParseStoreReference(t *testing.T) {
dockerRef, err := reference.XParseNamed(c.expectedRef)
require.NoError(t, err)
require.NotNil(t, storageRef.name, c.input)
assert.Equal(t, dockerRef.XString(), storageRef.name.XString())
assert.Equal(t, dockerRef.String(), storageRef.name.String())
}
}
}