Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
dda360d
API separation: Add an 'X' to all public names from c/i/docker/daemon…
mtrmac Jan 20, 2017
2db870f
Duplication: Have TWO distreference.Named values in namedRef
mtrmac Jan 20, 2017
2c1ea37
Transition to .upstream: Use in taggedRef and canonicalRef
mtrmac Jan 20, 2017
465998c
Transition to .upstream: Use instead of splitHostname
mtrmac Jan 20, 2017
e8533be
Transition to .upstream: Use distreference.Familiar* instead of our n…
mtrmac Jan 20, 2017
5edbd7f
Simplification: drop namedRef.our
mtrmac Jan 20, 2017
5240b7c
Simplification: Make namedRef implement distreference.Named
mtrmac Jan 20, 2017
df65181
API transition: Drop XNamed.XFullName
mtrmac Jan 20, 2017
2f8c595
API transition: Drop XNamed.XHostname
mtrmac Jan 20, 2017
8cde554
API transition: Drop XNamed.XRemoteName
mtrmac Jan 20, 2017
2b4f09f
API transition: Drop XNamed.XName
mtrmac Jan 20, 2017
7abfa98
API transition: Drop XNamed.XString
mtrmac Jan 20, 2017
dfe2faf
API transition: Drop reference.XNamed
mtrmac Jan 20, 2017
c2360fc
Duplication: Have both a namedRef and NamedTagged in taggedRef
mtrmac Jan 20, 2017
09779a3
Implementation transition: Use the NamedTagged in XTag instead of nam…
mtrmac Jan 20, 2017
0268d90
API transition: Drop XNamedTagged.XTag
mtrmac Jan 20, 2017
751e8a5
Implementation collapse: Drop taggedRef
mtrmac Jan 20, 2017
777b215
API transition: Drop reference.XNamedTagged
mtrmac Jan 20, 2017
49dea97
API transition: Drop XWithTag
mtrmac Jan 20, 2017
11e43ce
Duplication: Have both a namedRef and Canonical in canonicalRef
mtrmac Jan 20, 2017
76aef09
Implementation transition: Use the Canonical in XDigest instead of na…
mtrmac Jan 20, 2017
00b598c
API transition: Drop XCanonical.XDigest
mtrmac Jan 20, 2017
c91c7c2
Implementation collapse: Drop canonicalRef
mtrmac Jan 20, 2017
cc0f48a
API transition: Drop reference.XCanonical
mtrmac Jan 20, 2017
b55e51d
Implementation transition: Use distreference.IsNameOnly in XIsNameOnly
mtrmac Jan 20, 2017
c2d8ac2
API transition: Drop reference.XIsNameOnly
mtrmac Jan 20, 2017
b330f50
Implementation transition: Use distreference.TagNameOnly in XWithDefa…
mtrmac Jan 20, 2017
05f35b9
API transition: Drop reference.XWithDefaultTag
mtrmac Feb 1, 2017
7c038c2
Implementation collapse: Drop namedRef
mtrmac Jan 20, 2017
32d33ac
API transition: Drop reference.XWithName
mtrmac Jan 20, 2017
5a9a27a
Do not normalize the input string twice in reference.XParseNamed
mtrmac Jan 20, 2017
e928b40
Drop unused constants from docker/reference.
mtrmac Jan 20, 2017
184b810
BEHAVIOR CHANGE: Do not re-construct the reference in XParseNamed
mtrmac Jan 20, 2017
a81649c
API transition: Drop reference.XParseNamed
mtrmac Jan 20, 2017
2f1d5df
Implementation transition: Base XParseIDOrReference on distreference.…
mtrmac Jan 20, 2017
6e4f15a
API transition: Drop reference.XParseIDOrReference
mtrmac Jan 20, 2017
74c24ed
Finally, get rid of the last remains of c/i/docker/reference.
mtrmac Jan 20, 2017
ecdd233
Copy github.com/docker/distribution/reference to docker/reference
mtrmac Feb 1, 2017
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
2 changes: 1 addition & 1 deletion docker/daemon/daemon_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func (d *daemonImageDestination) PutManifest(m []byte) error {
// a hostname-qualified reference.
// See https://github.com/containers/image/issues/72 for a more detailed
// analysis and explanation.
refString := fmt.Sprintf("%s:%s", d.namedTaggedRef.FullName(), d.namedTaggedRef.Tag())
refString := fmt.Sprintf("%s:%s", d.namedTaggedRef.Name(), d.namedTaggedRef.Tag())

items := []manifestItem{{
Config: man.Config.Digest.String(),
Expand Down
15 changes: 8 additions & 7 deletions docker/daemon/daemon_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ type daemonReference struct {

// ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an ImageReference.
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.
// This is intended to be compatible with reference.ParseAnyReference, 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.
// reference.ParseAnyReference interprets such strings as digests.
if dgst, err := digest.Parse(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.
Expand All @@ -60,11 +60,11 @@ func ParseReference(refString string) (types.ImageReference, error) {
return NewReference(dgst, nil)
}

ref, err := reference.ParseNamed(refString) // This also rejects unprefixed digest values
ref, err := reference.ParseNormalizedNamed(refString) // This also rejects unprefixed digest values
if err != nil {
return nil, err
}
if ref.Name() == digest.Canonical.String() {
if reference.FamiliarName(ref) == digest.Canonical.String() {
return nil, errors.Errorf("Invalid docker-daemon: reference %s: The %s repository name is reserved for (non-shortened) digest references", refString, digest.Canonical)
}
return NewReference("", ref)
Expand All @@ -77,10 +77,11 @@ func NewReference(id digest.Digest, ref reference.Named) (types.ImageReference,
}
if ref != nil {
if reference.IsNameOnly(ref) {
return nil, errors.Errorf("docker-daemon: reference %s has neither a tag nor a digest", ref.String())
return nil, errors.Errorf("docker-daemon: reference %s has neither a tag nor a digest", reference.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.
// Most versions of docker/reference do not handle that (ignoring the tag), so reject such input.
// This MAY be accepted in the future.
_, isTagged := ref.(reference.NamedTagged)
_, isDigested := ref.(reference.Canonical)
if isTagged && isDigested {
Expand Down Expand Up @@ -108,7 +109,7 @@ func (ref daemonReference) StringWithinTransport() string {
case ref.id != "":
return ref.id.String()
case ref.ref != nil:
return ref.ref.String()
return reference.FamiliarString(ref.ref)
default: // Coverage: Should never happen, NewReference above should refuse such values.
panic("Internal inconsistency: daemonReference has empty id and nil ref")
}
Expand Down
56 changes: 24 additions & 32 deletions docker/daemon/daemon_transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,12 @@ 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
// Most versions of docker/reference do not handle that (ignoring the tag), so we reject such input.
{"busybox:latest@" + 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 == "" {
Expand All @@ -67,43 +64,37 @@ func testParseReference(t *testing.T, fn func(string) (types.ImageReference, err
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)
// If we don't reject the input, the interpretation must be consistent with reference.ParseAnyReference
dockerRef, err := reference.ParseAnyReference(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)
_, ok := dockerRef.(reference.Digested)
require.True(t, ok, c.input)
assert.Equal(t, c.expectedID, dockerRef.String(), 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)
_, ok := dockerRef.(reference.Named)
require.True(t, ok, 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
{"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) {
Expand All @@ -119,7 +110,7 @@ func TestNewReference(t *testing.T) {

// Named references
for _, c := range validNamedReferenceTestCases {
parsed, err := reference.ParseNamed(c.input)
parsed, err := reference.ParseNormalizedNamed(c.input)
require.NoError(t, err)
ref, err := NewReference("", parsed)
require.NoError(t, err, c.input)
Expand All @@ -131,24 +122,25 @@ func TestNewReference(t *testing.T) {
}

// Both an ID and a named reference provided
parsed, err := reference.ParseNamed("busybox:latest")
parsed, err := reference.ParseNormalizedNamed("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")
parsed, err = reference.ParseNormalizedNamed("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)
parsed, err = reference.ParseNormalizedNamed("busybox:notlatest@" + sha256digest)
require.NoError(t, err)
refDigested, ok := parsed.(reference.Canonical)
_, ok = parsed.(reference.Canonical)
require.True(t, ok)
tagDigestRef := refWithTagAndDigest{refDigested}
_, err = NewReference("", tagDigestRef)
_, ok = parsed.(reference.NamedTagged)
require.True(t, ok)
_, err = NewReference("", parsed)
assert.Error(t, err)
}

Expand Down
7 changes: 4 additions & 3 deletions docker/docker_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"time"

"github.com/Sirupsen/logrus"
"github.com/containers/image/docker/reference"
"github.com/containers/image/types"
"github.com/containers/storage/pkg/homedir"
"github.com/docker/go-connections/sockets"
Expand Down Expand Up @@ -164,11 +165,11 @@ func hasFile(files []os.FileInfo, name string) bool {
// newDockerClient returns a new dockerClient instance for refHostname (a host a specified in the Docker image reference, not canonicalized to dockerRegistry)
// “write” specifies whether the client will be used for "write" access (in particular passed to lookaside.go:toplevelFromSection)
func newDockerClient(ctx *types.SystemContext, ref dockerReference, write bool, actions string) (*dockerClient, error) {
registry := ref.ref.Hostname()
registry := reference.Domain(ref.ref)
if registry == dockerHostname {
registry = dockerRegistry
}
username, password, err := getAuth(ctx, ref.ref.Hostname())
username, password, err := getAuth(ctx, reference.Domain(ref.ref))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -202,7 +203,7 @@ func newDockerClient(ctx *types.SystemContext, ref dockerReference, write bool,
signatureBase: sigBase,
scope: authScope{
actions: actions,
remoteName: ref.ref.RemoteName(),
remoteName: reference.Path(ref.ref),
},
}, nil
}
Expand Down
5 changes: 3 additions & 2 deletions docker/docker_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/http"

"github.com/containers/image/docker/reference"
"github.com/containers/image/image"
"github.com/containers/image/types"
"github.com/pkg/errors"
Expand Down Expand Up @@ -34,12 +35,12 @@ func newImage(ctx *types.SystemContext, ref dockerReference) (types.Image, error

// SourceRefFullName returns a fully expanded name for the repository this image is in.
func (i *Image) SourceRefFullName() string {
return i.src.ref.ref.FullName()
return i.src.ref.ref.Name()
}

// 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 *Image) GetRepositoryTags() ([]string, error) {
url := fmt.Sprintf(tagsURL, i.src.ref.ref.RemoteName())
url := fmt.Sprintf(tagsURL, reference.Path(i.src.ref.ref))
res, err := i.src.c.makeRequest("GET", url, nil, nil)
if err != nil {
return nil, err
Expand Down
19 changes: 10 additions & 9 deletions docker/docker_image_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"path/filepath"

"github.com/Sirupsen/logrus"
"github.com/containers/image/docker/reference"
"github.com/containers/image/manifest"
"github.com/containers/image/types"
"github.com/opencontainers/go-digest"
Expand Down Expand Up @@ -98,7 +99,7 @@ func (c *sizeCounter) Write(p []byte) (n int, err error) {
// If stream.Read() at any time, ESPECIALLY at end of input, returns an error, PutBlob MUST 1) fail, and 2) delete any data stored so far.
func (d *dockerImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobInfo) (types.BlobInfo, error) {
if inputInfo.Digest.String() != "" {
checkURL := fmt.Sprintf(blobsURL, d.ref.ref.RemoteName(), inputInfo.Digest.String())
checkURL := fmt.Sprintf(blobsURL, reference.Path(d.ref.ref), inputInfo.Digest.String())

logrus.Debugf("Checking %s", checkURL)
res, err := d.c.makeRequest("HEAD", checkURL, nil, nil)
Expand All @@ -112,17 +113,17 @@ func (d *dockerImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobI
return types.BlobInfo{Digest: inputInfo.Digest, Size: getBlobSize(res)}, nil
case http.StatusUnauthorized:
logrus.Debugf("... not authorized")
return types.BlobInfo{}, errors.Errorf("not authorized to read from destination repository %s", d.ref.ref.RemoteName())
return types.BlobInfo{}, errors.Errorf("not authorized to read from destination repository %s", reference.Path(d.ref.ref))
case http.StatusNotFound:
// noop
default:
return types.BlobInfo{}, errors.Errorf("failed to read from destination repository %s: %v", d.ref.ref.RemoteName(), http.StatusText(res.StatusCode))
return types.BlobInfo{}, errors.Errorf("failed to read from destination repository %s: %v", reference.Path(d.ref.ref), http.StatusText(res.StatusCode))
}
logrus.Debugf("... failed, status %d", res.StatusCode)
}

// FIXME? Chunked upload, progress reporting, etc.
uploadURL := fmt.Sprintf(blobUploadURL, d.ref.ref.RemoteName())
uploadURL := fmt.Sprintf(blobUploadURL, reference.Path(d.ref.ref))
logrus.Debugf("Uploading %s", uploadURL)
res, err := d.c.makeRequest("POST", uploadURL, nil, nil)
if err != nil {
Expand Down Expand Up @@ -178,7 +179,7 @@ func (d *dockerImageDestination) HasBlob(info types.BlobInfo) (bool, int64, erro
if info.Digest == "" {
return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
}
checkURL := fmt.Sprintf(blobsURL, d.ref.ref.RemoteName(), info.Digest.String())
checkURL := fmt.Sprintf(blobsURL, reference.Path(d.ref.ref), info.Digest.String())

logrus.Debugf("Checking %s", checkURL)
res, err := d.c.makeRequest("HEAD", checkURL, nil, nil)
Expand All @@ -192,12 +193,12 @@ func (d *dockerImageDestination) HasBlob(info types.BlobInfo) (bool, int64, erro
return true, getBlobSize(res), nil
case http.StatusUnauthorized:
logrus.Debugf("... not authorized")
return false, -1, errors.Errorf("not authorized to read from destination repository %s", d.ref.ref.RemoteName())
return false, -1, errors.Errorf("not authorized to read from destination repository %s", reference.Path(d.ref.ref))
case http.StatusNotFound:
logrus.Debugf("... not present")
return false, -1, types.ErrBlobNotFound
default:
logrus.Errorf("failed to read from destination repository %s: %v", d.ref.ref.RemoteName(), http.StatusText(res.StatusCode))
logrus.Errorf("failed to read from destination repository %s: %v", reference.Path(d.ref.ref), http.StatusText(res.StatusCode))
}
logrus.Debugf("... failed, status %d, ignoring", res.StatusCode)
return false, -1, types.ErrBlobNotFound
Expand All @@ -214,11 +215,11 @@ func (d *dockerImageDestination) PutManifest(m []byte) error {
}
d.manifestDigest = digest

reference, err := d.ref.tagOrDigest()
refTail, err := d.ref.tagOrDigest()
if err != nil {
return err
}
url := fmt.Sprintf(manifestURL, d.ref.ref.RemoteName(), reference)
url := fmt.Sprintf(manifestURL, reference.Path(d.ref.ref), refTail)

headers := map[string][]string{}
mimeType := manifest.GuessMIMEType(m)
Expand Down
11 changes: 6 additions & 5 deletions docker/docker_image_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strconv"

"github.com/Sirupsen/logrus"
"github.com/containers/image/docker/reference"
"github.com/containers/image/manifest"
"github.com/containers/image/types"
"github.com/docker/distribution/registry/client"
Expand Down Expand Up @@ -91,7 +92,7 @@ func (s *dockerImageSource) GetManifest() ([]byte, string, error) {
}

func (s *dockerImageSource) fetchManifest(tagOrDigest string) ([]byte, string, error) {
url := fmt.Sprintf(manifestURL, s.ref.ref.RemoteName(), tagOrDigest)
url := fmt.Sprintf(manifestURL, reference.Path(s.ref.ref), tagOrDigest)
headers := make(map[string][]string)
headers["Accept"] = s.requestedManifestMIMETypes
res, err := s.c.makeRequest("GET", url, headers, nil)
Expand Down Expand Up @@ -177,7 +178,7 @@ func (s *dockerImageSource) GetBlob(info types.BlobInfo) (io.ReadCloser, int64,
return s.getExternalBlob(info.URLs)
}

url := fmt.Sprintf(blobsURL, s.ref.ref.RemoteName(), info.Digest.String())
url := fmt.Sprintf(blobsURL, reference.Path(s.ref.ref), info.Digest.String())
logrus.Debugf("Downloading %s", url)
res, err := s.c.makeRequest("GET", url, nil, nil)
if err != nil {
Expand Down Expand Up @@ -271,11 +272,11 @@ func deleteImage(ctx *types.SystemContext, ref dockerReference) error {
headers := make(map[string][]string)
headers["Accept"] = []string{manifest.DockerV2Schema2MediaType}

reference, err := ref.tagOrDigest()
refTail, err := ref.tagOrDigest()
if err != nil {
return err
}
getURL := fmt.Sprintf(manifestURL, ref.ref.RemoteName(), reference)
getURL := fmt.Sprintf(manifestURL, reference.Path(ref.ref), refTail)
get, err := c.makeRequest("GET", getURL, headers, nil)
if err != nil {
return err
Expand All @@ -294,7 +295,7 @@ func deleteImage(ctx *types.SystemContext, ref dockerReference) error {
}

digest := get.Header.Get("Docker-Content-Digest")
deleteURL := fmt.Sprintf(manifestURL, ref.ref.RemoteName(), digest)
deleteURL := fmt.Sprintf(manifestURL, reference.Path(ref.ref), digest)

// When retrieving the digest from a registry >= 2.3 use the following header:
// "Accept": "application/vnd.docker.distribution.manifest.v2+json"
Expand Down
Loading