From b741d2e9b5cf4e52f8da0c94cdc3097616120411 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Wed, 25 Jan 2017 16:54:18 -0800 Subject: [PATCH] Use distribution reference Remove forked reference package. Use normalized named values everywhere and familiar functions to convert back to familiar strings for UX and storage compatibility. Enforce that the source repository in the distribution metadata is always a normalized string, ignore invalid values which are not. Update distribution tests to use normalized values. Signed-off-by: Derek McGowan (github: dmcgowan) --- container_commit.go | 14 ++++++++------ image_create.go | 8 ++++---- image_import.go | 2 +- image_pull.go | 27 +++++++++++++++++++++------ image_pull_test.go | 2 +- image_push.go | 18 ++++++++++-------- image_push_test.go | 2 +- image_tag.go | 17 +++++++++-------- plugin_install.go | 2 +- plugin_upgrade.go | 2 +- 10 files changed, 57 insertions(+), 37 deletions(-) diff --git a/container_commit.go b/container_commit.go index c766d62e40..531d796ee7 100644 --- a/container_commit.go +++ b/container_commit.go @@ -5,9 +5,8 @@ import ( "errors" "net/url" - distreference "github.com/docker/distribution/reference" + "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/reference" "golang.org/x/net/context" ) @@ -15,17 +14,20 @@ import ( func (cli *Client) ContainerCommit(ctx context.Context, container string, options types.ContainerCommitOptions) (types.IDResponse, error) { var repository, tag string if options.Reference != "" { - distributionRef, err := distreference.ParseNamed(options.Reference) + ref, err := reference.ParseNormalizedNamed(options.Reference) if err != nil { return types.IDResponse{}, err } - if _, isCanonical := distributionRef.(distreference.Canonical); isCanonical { + if _, isCanonical := ref.(reference.Canonical); isCanonical { return types.IDResponse{}, errors.New("refusing to create a tag with a digest reference") } + ref = reference.TagNameOnly(ref) - tag = reference.GetTagFromNamedRef(distributionRef) - repository = distributionRef.Name() + if tagged, ok := ref.(reference.Tagged); ok { + tag = tagged.Tag() + } + repository = reference.FamiliarName(ref) } query := url.Values{} diff --git a/image_create.go b/image_create.go index cf023a7186..4436abb0dd 100644 --- a/image_create.go +++ b/image_create.go @@ -6,21 +6,21 @@ import ( "golang.org/x/net/context" + "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/reference" ) // ImageCreate creates a new image based in the parent options. // It returns the JSON content in the response body. func (cli *Client) ImageCreate(ctx context.Context, parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { - repository, tag, err := reference.Parse(parentReference) + ref, err := reference.ParseNormalizedNamed(parentReference) if err != nil { return nil, err } query := url.Values{} - query.Set("fromImage", repository) - query.Set("tag", tag) + query.Set("fromImage", reference.FamiliarName(ref)) + query.Set("tag", getAPITagFromNamedRef(ref)) resp, err := cli.tryImageCreate(ctx, query, options.RegistryAuth) if err != nil { return nil, err diff --git a/image_import.go b/image_import.go index c6f154b249..d7dedd8232 100644 --- a/image_import.go +++ b/image_import.go @@ -15,7 +15,7 @@ import ( func (cli *Client) ImageImport(ctx context.Context, source types.ImageImportSource, ref string, options types.ImageImportOptions) (io.ReadCloser, error) { if ref != "" { //Check if the given image name can be resolved - if _, err := reference.ParseNamed(ref); err != nil { + if _, err := reference.ParseNormalizedNamed(ref); err != nil { return nil, err } } diff --git a/image_pull.go b/image_pull.go index 3bffdb70e8..a72b9bf7fc 100644 --- a/image_pull.go +++ b/image_pull.go @@ -7,8 +7,8 @@ import ( "golang.org/x/net/context" + "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/reference" ) // ImagePull requests the docker host to pull an image from a remote registry. @@ -19,16 +19,16 @@ import ( // FIXME(vdemeester): there is currently used in a few way in docker/docker // - if not in trusted content, ref is used to pass the whole reference, and tag is empty // - if in trusted content, ref is used to pass the reference name, and tag for the digest -func (cli *Client) ImagePull(ctx context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error) { - repository, tag, err := reference.Parse(ref) +func (cli *Client) ImagePull(ctx context.Context, refStr string, options types.ImagePullOptions) (io.ReadCloser, error) { + ref, err := reference.ParseNormalizedNamed(refStr) if err != nil { return nil, err } query := url.Values{} - query.Set("fromImage", repository) - if tag != "" && !options.All { - query.Set("tag", tag) + query.Set("fromImage", reference.FamiliarName(ref)) + if !options.All { + query.Set("tag", getAPITagFromNamedRef(ref)) } resp, err := cli.tryImageCreate(ctx, query, options.RegistryAuth) @@ -44,3 +44,18 @@ func (cli *Client) ImagePull(ctx context.Context, ref string, options types.Imag } return resp.body, nil } + +// getAPITagFromNamedRef returns a tag from the specified reference. +// This function is necessary as long as the docker "server" api expects +// digests to be sent as tags and makes a distinction between the name +// and tag/digest part of a reference. +func getAPITagFromNamedRef(ref reference.Named) string { + if digested, ok := ref.(reference.Digested); ok { + return digested.Digest().String() + } + ref = reference.TagNameOnly(ref) + if tagged, ok := ref.(reference.Tagged); ok { + return tagged.Tag() + } + return "" +} diff --git a/image_pull_test.go b/image_pull_test.go index fe6bafed97..ab49d2d349 100644 --- a/image_pull_test.go +++ b/image_pull_test.go @@ -21,7 +21,7 @@ func TestImagePullReferenceParseError(t *testing.T) { } // An empty reference is an invalid reference _, err := client.ImagePull(context.Background(), "", types.ImagePullOptions{}) - if err == nil || err.Error() != "repository name must have at least one component" { + if err == nil || !strings.Contains(err.Error(), "invalid reference format") { t.Fatalf("expected an error, got %v", err) } } diff --git a/image_push.go b/image_push.go index 8e73d28f56..410d2fb91d 100644 --- a/image_push.go +++ b/image_push.go @@ -8,7 +8,7 @@ import ( "golang.org/x/net/context" - distreference "github.com/docker/distribution/reference" + "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" ) @@ -16,31 +16,33 @@ import ( // It executes the privileged function if the operation is unauthorized // and it tries one more time. // It's up to the caller to handle the io.ReadCloser and close it properly. -func (cli *Client) ImagePush(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { - distributionRef, err := distreference.ParseNamed(ref) +func (cli *Client) ImagePush(ctx context.Context, image string, options types.ImagePushOptions) (io.ReadCloser, error) { + ref, err := reference.ParseNormalizedNamed(image) if err != nil { return nil, err } - if _, isCanonical := distributionRef.(distreference.Canonical); isCanonical { + if _, isCanonical := ref.(reference.Canonical); isCanonical { return nil, errors.New("cannot push a digest reference") } - var tag = "" - if nameTaggedRef, isNamedTagged := distributionRef.(distreference.NamedTagged); isNamedTagged { + tag := "" + name := reference.FamiliarName(ref) + + if nameTaggedRef, isNamedTagged := ref.(reference.NamedTagged); isNamedTagged { tag = nameTaggedRef.Tag() } query := url.Values{} query.Set("tag", tag) - resp, err := cli.tryImagePush(ctx, distributionRef.Name(), query, options.RegistryAuth) + resp, err := cli.tryImagePush(ctx, name, query, options.RegistryAuth) if resp.statusCode == http.StatusUnauthorized && options.PrivilegeFunc != nil { newAuthHeader, privilegeErr := options.PrivilegeFunc() if privilegeErr != nil { return nil, privilegeErr } - resp, err = cli.tryImagePush(ctx, distributionRef.Name(), query, newAuthHeader) + resp, err = cli.tryImagePush(ctx, name, query, newAuthHeader) } if err != nil { return nil, err diff --git a/image_push_test.go b/image_push_test.go index b52da8b8dc..f93debf5bb 100644 --- a/image_push_test.go +++ b/image_push_test.go @@ -21,7 +21,7 @@ func TestImagePushReferenceError(t *testing.T) { } // An empty reference is an invalid reference _, err := client.ImagePush(context.Background(), "", types.ImagePushOptions{}) - if err == nil || err.Error() != "repository name must have at least one component" { + if err == nil || !strings.Contains(err.Error(), "invalid reference format") { t.Fatalf("expected an error, got %v", err) } // An canonical reference cannot be pushed diff --git a/image_tag.go b/image_tag.go index dbcd078e1c..35abe332bf 100644 --- a/image_tag.go +++ b/image_tag.go @@ -3,32 +3,33 @@ package client import ( "net/url" - distreference "github.com/docker/distribution/reference" - "github.com/docker/docker/api/types/reference" + "github.com/docker/distribution/reference" "github.com/pkg/errors" "golang.org/x/net/context" ) // ImageTag tags an image in the docker host func (cli *Client) ImageTag(ctx context.Context, source, target string) error { - if _, err := distreference.ParseNamed(source); err != nil { + if _, err := reference.ParseNormalizedNamed(source); err != nil { return errors.Wrapf(err, "Error parsing reference: %q is not a valid repository/tag", source) } - distributionRef, err := distreference.ParseNamed(target) + ref, err := reference.ParseNormalizedNamed(target) if err != nil { return errors.Wrapf(err, "Error parsing reference: %q is not a valid repository/tag", target) } - if _, isCanonical := distributionRef.(distreference.Canonical); isCanonical { + if _, isCanonical := ref.(reference.Canonical); isCanonical { return errors.New("refusing to create a tag with a digest reference") } - tag := reference.GetTagFromNamedRef(distributionRef) + ref = reference.TagNameOnly(ref) query := url.Values{} - query.Set("repo", distributionRef.Name()) - query.Set("tag", tag) + query.Set("repo", reference.FamiliarName(ref)) + if tagged, ok := ref.(reference.Tagged); ok { + query.Set("tag", tagged.Tag()) + } resp, err := cli.post(ctx, "/images/"+source+"/tag", query, nil, nil) ensureReaderClosed(resp) diff --git a/plugin_install.go b/plugin_install.go index 3217c4cf39..33876cc101 100644 --- a/plugin_install.go +++ b/plugin_install.go @@ -15,7 +15,7 @@ import ( // PluginInstall installs a plugin func (cli *Client) PluginInstall(ctx context.Context, name string, options types.PluginInstallOptions) (rc io.ReadCloser, err error) { query := url.Values{} - if _, err := reference.ParseNamed(options.RemoteRef); err != nil { + if _, err := reference.ParseNormalizedNamed(options.RemoteRef); err != nil { return nil, errors.Wrap(err, "invalid remote reference") } query.Set("remote", options.RemoteRef) diff --git a/plugin_upgrade.go b/plugin_upgrade.go index 95a4356b97..24293c5073 100644 --- a/plugin_upgrade.go +++ b/plugin_upgrade.go @@ -14,7 +14,7 @@ import ( // PluginUpgrade upgrades a plugin func (cli *Client) PluginUpgrade(ctx context.Context, name string, options types.PluginInstallOptions) (rc io.ReadCloser, err error) { query := url.Values{} - if _, err := reference.ParseNamed(options.RemoteRef); err != nil { + if _, err := reference.ParseNormalizedNamed(options.RemoteRef); err != nil { return nil, errors.Wrap(err, "invalid remote reference") } query.Set("remote", options.RemoteRef)