From 60ecc132c538129e7093dd38987291651e418035 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Wed, 9 Nov 2016 20:36:17 -0800 Subject: [PATCH 1/3] Update distribution vendor Pull in client changes to handle oauth errors Signed-off-by: Derek McGowan (github: dmcgowan) --- vendor.conf | 2 +- .../distribution/reference/reference.go | 30 ++++ .../registry/api/v2/headerparser.go | 161 ++++++++++++++++++ .../distribution/registry/api/v2/urls.go | 67 +++++++- .../client/auth/{ => challenge}/addr.go | 2 +- .../auth/{ => challenge}/authchallenge.go | 20 +-- .../registry/client/auth/session.go | 11 +- .../distribution/registry/client/errors.go | 42 ++++- 8 files changed, 311 insertions(+), 24 deletions(-) create mode 100644 vendor/github.com/docker/distribution/registry/api/v2/headerparser.go rename vendor/github.com/docker/distribution/registry/client/auth/{ => challenge}/addr.go (97%) rename vendor/github.com/docker/distribution/registry/client/auth/{ => challenge}/authchallenge.go (91%) diff --git a/vendor.conf b/vendor.conf index f0a4edba85..45f467b036 100644 --- a/vendor.conf +++ b/vendor.conf @@ -44,7 +44,7 @@ github.com/boltdb/bolt fff57c100f4dea1905678da7e90d92429dff2904 github.com/miekg/dns 75e6e86cc601825c5dbcd4e0c209eab180997cd7 # get graph and distribution packages -github.com/docker/distribution c04791f441f98bcf073353d7317db83663cf3ea2 +github.com/docker/distribution 8016d2d8903e378edacac11e4d809efbc987ad61 github.com/vbatts/tar-split v0.10.1 # get go-zfs packages diff --git a/vendor/github.com/docker/distribution/reference/reference.go b/vendor/github.com/docker/distribution/reference/reference.go index dc3af1670b..02786628e8 100644 --- a/vendor/github.com/docker/distribution/reference/reference.go +++ b/vendor/github.com/docker/distribution/reference/reference.go @@ -24,6 +24,7 @@ package reference import ( "errors" "fmt" + "path" "strings" "github.com/docker/distribution/digest" @@ -218,6 +219,13 @@ func WithTag(name Named, tag string) (NamedTagged, error) { if !anchoredTagRegexp.MatchString(tag) { return nil, ErrTagInvalidFormat } + if canonical, ok := name.(Canonical); ok { + return reference{ + name: name.Name(), + tag: tag, + digest: canonical.Digest(), + }, nil + } return taggedReference{ name: name.Name(), tag: tag, @@ -230,12 +238,34 @@ func WithDigest(name Named, digest digest.Digest) (Canonical, error) { if !anchoredDigestRegexp.MatchString(digest.String()) { return nil, ErrDigestInvalidFormat } + if tagged, ok := name.(Tagged); ok { + return reference{ + name: name.Name(), + tag: tagged.Tag(), + digest: digest, + }, nil + } return canonicalReference{ name: name.Name(), digest: digest, }, nil } +// Match reports whether ref matches the specified pattern. +// See https://godoc.org/path#Match for supported patterns. +func Match(pattern string, ref Reference) (bool, error) { + matched, err := path.Match(pattern, ref.String()) + if namedRef, isNamed := ref.(Named); isNamed && !matched { + matched, _ = path.Match(pattern, namedRef.Name()) + } + return matched, err +} + +// TrimNamed removes any tag or digest from the named reference. +func TrimNamed(ref Named) Named { + return repository(ref.Name()) +} + func getBestReferenceType(ref reference) Reference { if ref.name == "" { // Allow digest only references diff --git a/vendor/github.com/docker/distribution/registry/api/v2/headerparser.go b/vendor/github.com/docker/distribution/registry/api/v2/headerparser.go new file mode 100644 index 0000000000..9bc41a3a64 --- /dev/null +++ b/vendor/github.com/docker/distribution/registry/api/v2/headerparser.go @@ -0,0 +1,161 @@ +package v2 + +import ( + "fmt" + "regexp" + "strings" + "unicode" +) + +var ( + // according to rfc7230 + reToken = regexp.MustCompile(`^[^"(),/:;<=>?@[\]{}[:space:][:cntrl:]]+`) + reQuotedValue = regexp.MustCompile(`^[^\\"]+`) + reEscapedCharacter = regexp.MustCompile(`^[[:blank:][:graph:]]`) +) + +// parseForwardedHeader is a benevolent parser of Forwarded header defined in rfc7239. The header contains +// a comma-separated list of forwarding key-value pairs. Each list element is set by single proxy. The +// function parses only the first element of the list, which is set by the very first proxy. It returns a map +// of corresponding key-value pairs and an unparsed slice of the input string. +// +// Examples of Forwarded header values: +// +// 1. Forwarded: For=192.0.2.43; Proto=https,For="[2001:db8:cafe::17]",For=unknown +// 2. Forwarded: for="192.0.2.43:443"; host="registry.example.org", for="10.10.05.40:80" +// +// The first will be parsed into {"for": "192.0.2.43", "proto": "https"} while the second into +// {"for": "192.0.2.43:443", "host": "registry.example.org"}. +func parseForwardedHeader(forwarded string) (map[string]string, string, error) { + // Following are states of forwarded header parser. Any state could transition to a failure. + const ( + // terminating state; can transition to Parameter + stateElement = iota + // terminating state; can transition to KeyValueDelimiter + stateParameter + // can transition to Value + stateKeyValueDelimiter + // can transition to one of { QuotedValue, PairEnd } + stateValue + // can transition to one of { EscapedCharacter, PairEnd } + stateQuotedValue + // can transition to one of { QuotedValue } + stateEscapedCharacter + // terminating state; can transition to one of { Parameter, Element } + statePairEnd + ) + + var ( + parameter string + value string + parse = forwarded[:] + res = map[string]string{} + state = stateElement + ) + +Loop: + for { + // skip spaces unless in quoted value + if state != stateQuotedValue && state != stateEscapedCharacter { + parse = strings.TrimLeftFunc(parse, unicode.IsSpace) + } + + if len(parse) == 0 { + if state != stateElement && state != statePairEnd && state != stateParameter { + return nil, parse, fmt.Errorf("unexpected end of input") + } + // terminating + break + } + + switch state { + // terminate at list element delimiter + case stateElement: + if parse[0] == ',' { + parse = parse[1:] + break Loop + } + state = stateParameter + + // parse parameter (the key of key-value pair) + case stateParameter: + match := reToken.FindString(parse) + if len(match) == 0 { + return nil, parse, fmt.Errorf("failed to parse token at position %d", len(forwarded)-len(parse)) + } + parameter = strings.ToLower(match) + parse = parse[len(match):] + state = stateKeyValueDelimiter + + // parse '=' + case stateKeyValueDelimiter: + if parse[0] != '=' { + return nil, parse, fmt.Errorf("expected '=', not '%c' at position %d", parse[0], len(forwarded)-len(parse)) + } + parse = parse[1:] + state = stateValue + + // parse value or quoted value + case stateValue: + if parse[0] == '"' { + parse = parse[1:] + state = stateQuotedValue + } else { + value = reToken.FindString(parse) + if len(value) == 0 { + return nil, parse, fmt.Errorf("failed to parse value at position %d", len(forwarded)-len(parse)) + } + if _, exists := res[parameter]; exists { + return nil, parse, fmt.Errorf("duplicate parameter %q at position %d", parameter, len(forwarded)-len(parse)) + } + res[parameter] = value + parse = parse[len(value):] + value = "" + state = statePairEnd + } + + // parse a part of quoted value until the first backslash + case stateQuotedValue: + match := reQuotedValue.FindString(parse) + value += match + parse = parse[len(match):] + switch { + case len(parse) == 0: + return nil, parse, fmt.Errorf("unterminated quoted string") + case parse[0] == '"': + res[parameter] = value + value = "" + parse = parse[1:] + state = statePairEnd + case parse[0] == '\\': + parse = parse[1:] + state = stateEscapedCharacter + } + + // parse escaped character in a quoted string, ignore the backslash + // transition back to QuotedValue state + case stateEscapedCharacter: + c := reEscapedCharacter.FindString(parse) + if len(c) == 0 { + return nil, parse, fmt.Errorf("invalid escape sequence at position %d", len(forwarded)-len(parse)-1) + } + value += c + parse = parse[1:] + state = stateQuotedValue + + // expect either a new key-value pair, new list or end of input + case statePairEnd: + switch parse[0] { + case ';': + parse = parse[1:] + state = stateParameter + case ',': + state = stateElement + default: + return nil, parse, fmt.Errorf("expected ',' or ';', not %c at position %d", parse[0], len(forwarded)-len(parse)) + } + } + } + + return res, parse, nil +} diff --git a/vendor/github.com/docker/distribution/registry/api/v2/urls.go b/vendor/github.com/docker/distribution/registry/api/v2/urls.go index a959aaa897..e2e242eab0 100644 --- a/vendor/github.com/docker/distribution/registry/api/v2/urls.go +++ b/vendor/github.com/docker/distribution/registry/api/v2/urls.go @@ -1,8 +1,10 @@ package v2 import ( + "net" "net/http" "net/url" + "strconv" "strings" "github.com/docker/distribution/reference" @@ -49,10 +51,14 @@ func NewURLBuilderFromRequest(r *http.Request, relative bool) *URLBuilder { var scheme string forwardedProto := r.Header.Get("X-Forwarded-Proto") + // TODO: log the error + forwardedHeader, _, _ := parseForwardedHeader(r.Header.Get("Forwarded")) switch { case len(forwardedProto) > 0: scheme = forwardedProto + case len(forwardedHeader["proto"]) > 0: + scheme = forwardedHeader["proto"] case r.TLS != nil: scheme = "https" case len(r.URL.Scheme) > 0: @@ -62,14 +68,46 @@ func NewURLBuilderFromRequest(r *http.Request, relative bool) *URLBuilder { } host := r.Host - forwardedHost := r.Header.Get("X-Forwarded-Host") - if len(forwardedHost) > 0 { + + if forwardedHost := r.Header.Get("X-Forwarded-Host"); len(forwardedHost) > 0 { // According to the Apache mod_proxy docs, X-Forwarded-Host can be a // comma-separated list of hosts, to which each proxy appends the // requested host. We want to grab the first from this comma-separated // list. hosts := strings.SplitN(forwardedHost, ",", 2) host = strings.TrimSpace(hosts[0]) + } else if addr, exists := forwardedHeader["for"]; exists { + host = addr + } else if h, exists := forwardedHeader["host"]; exists { + host = h + } + + portLessHost, port := host, "" + if !isIPv6Address(portLessHost) { + // with go 1.6, this would treat the last part of IPv6 address as a port + portLessHost, port, _ = net.SplitHostPort(host) + } + if forwardedPort := r.Header.Get("X-Forwarded-Port"); len(port) == 0 && len(forwardedPort) > 0 { + ports := strings.SplitN(forwardedPort, ",", 2) + forwardedPort = strings.TrimSpace(ports[0]) + if _, err := strconv.ParseInt(forwardedPort, 10, 32); err == nil { + port = forwardedPort + } + } + + if len(portLessHost) > 0 { + host = portLessHost + } + if len(port) > 0 { + // remove enclosing brackets of ipv6 address otherwise they will be duplicated + if len(host) > 1 && host[0] == '[' && host[len(host)-1] == ']' { + host = host[1 : len(host)-1] + } + // JoinHostPort properly encloses ipv6 addresses in square brackets + host = net.JoinHostPort(host, port) + } else if isIPv6Address(host) && host[0] != '[' { + // ipv6 needs to be enclosed in square brackets in urls + host = "[" + host + "]" } basePath := routeDescriptorsMap[RouteNameBase].Path @@ -249,3 +287,28 @@ func appendValues(u string, values ...url.Values) string { return appendValuesURL(up, values...).String() } + +// isIPv6Address returns true if given string is a valid IPv6 address. No port is allowed. The address may be +// enclosed in square brackets. +func isIPv6Address(host string) bool { + if len(host) > 1 && host[0] == '[' && host[len(host)-1] == ']' { + host = host[1 : len(host)-1] + } + // The IPv6 scoped addressing zone identifier starts after the last percent sign. + if i := strings.LastIndexByte(host, '%'); i > 0 { + host = host[:i] + } + ip := net.ParseIP(host) + if ip == nil { + return false + } + if ip.To16() == nil { + return false + } + if ip.To4() == nil { + return true + } + // dot can be present in ipv4-mapped address, it needs to come after a colon though + i := strings.IndexAny(host, ":.") + return i >= 0 && host[i] == ':' +} diff --git a/vendor/github.com/docker/distribution/registry/client/auth/addr.go b/vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go similarity index 97% rename from vendor/github.com/docker/distribution/registry/client/auth/addr.go rename to vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go index 6e77752886..2c3ebe1653 100644 --- a/vendor/github.com/docker/distribution/registry/client/auth/addr.go +++ b/vendor/github.com/docker/distribution/registry/client/auth/challenge/addr.go @@ -1,4 +1,4 @@ -package auth +package challenge import ( "net/url" diff --git a/vendor/github.com/docker/distribution/registry/client/auth/authchallenge.go b/vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go similarity index 91% rename from vendor/github.com/docker/distribution/registry/client/auth/authchallenge.go rename to vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go index 69d9d6fe01..c9bdfc355b 100644 --- a/vendor/github.com/docker/distribution/registry/client/auth/authchallenge.go +++ b/vendor/github.com/docker/distribution/registry/client/auth/challenge/authchallenge.go @@ -1,4 +1,4 @@ -package auth +package challenge import ( "fmt" @@ -18,12 +18,12 @@ type Challenge struct { Parameters map[string]string } -// ChallengeManager manages the challenges for endpoints. +// Manager manages the challenges for endpoints. // The challenges are pulled out of HTTP responses. Only // responses which expect challenges should be added to // the manager, since a non-unauthorized request will be // viewed as not requiring challenges. -type ChallengeManager interface { +type Manager interface { // GetChallenges returns the challenges for the given // endpoint URL. GetChallenges(endpoint url.URL) ([]Challenge, error) @@ -37,19 +37,19 @@ type ChallengeManager interface { AddResponse(resp *http.Response) error } -// NewSimpleChallengeManager returns an instance of -// ChallengeManger which only maps endpoints to challenges +// NewSimpleManager returns an instance of +// Manger which only maps endpoints to challenges // based on the responses which have been added the // manager. The simple manager will make no attempt to // perform requests on the endpoints or cache the responses // to a backend. -func NewSimpleChallengeManager() ChallengeManager { - return &simpleChallengeManager{ +func NewSimpleManager() Manager { + return &simpleManager{ Challanges: make(map[string][]Challenge), } } -type simpleChallengeManager struct { +type simpleManager struct { sync.RWMutex Challanges map[string][]Challenge } @@ -59,7 +59,7 @@ func normalizeURL(endpoint *url.URL) { endpoint.Host = canonicalAddr(endpoint) } -func (m *simpleChallengeManager) GetChallenges(endpoint url.URL) ([]Challenge, error) { +func (m *simpleManager) GetChallenges(endpoint url.URL) ([]Challenge, error) { normalizeURL(&endpoint) m.RLock() @@ -68,7 +68,7 @@ func (m *simpleChallengeManager) GetChallenges(endpoint url.URL) ([]Challenge, e return challenges, nil } -func (m *simpleChallengeManager) AddResponse(resp *http.Response) error { +func (m *simpleManager) AddResponse(resp *http.Response) error { challenges := ResponseChallenges(resp) if resp.Request == nil { return fmt.Errorf("missing request reference") diff --git a/vendor/github.com/docker/distribution/registry/client/auth/session.go b/vendor/github.com/docker/distribution/registry/client/auth/session.go index d03d8ff0ed..ffc3384b19 100644 --- a/vendor/github.com/docker/distribution/registry/client/auth/session.go +++ b/vendor/github.com/docker/distribution/registry/client/auth/session.go @@ -12,6 +12,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/distribution/registry/client" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" ) @@ -58,7 +59,7 @@ type CredentialStore interface { // schemes. The handlers are tried in order, the higher priority authentication // methods should be first. The challengeMap holds a list of challenges for // a given root API endpoint (for example "https://registry-1.docker.io/v2/"). -func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler) transport.RequestModifier { +func NewAuthorizer(manager challenge.Manager, handlers ...AuthenticationHandler) transport.RequestModifier { return &endpointAuthorizer{ challenges: manager, handlers: handlers, @@ -66,7 +67,7 @@ func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler) } type endpointAuthorizer struct { - challenges ChallengeManager + challenges challenge.Manager handlers []AuthenticationHandler transport http.RoundTripper } @@ -94,11 +95,11 @@ func (ea *endpointAuthorizer) ModifyRequest(req *http.Request) error { if len(challenges) > 0 { for _, handler := range ea.handlers { - for _, challenge := range challenges { - if challenge.Scheme != handler.Scheme() { + for _, c := range challenges { + if c.Scheme != handler.Scheme() { continue } - if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil { + if err := handler.AuthorizeRequest(req, c.Parameters); err != nil { return err } } diff --git a/vendor/github.com/docker/distribution/registry/client/errors.go b/vendor/github.com/docker/distribution/registry/client/errors.go index f73e3c2301..52d49d5d29 100644 --- a/vendor/github.com/docker/distribution/registry/client/errors.go +++ b/vendor/github.com/docker/distribution/registry/client/errors.go @@ -9,6 +9,7 @@ import ( "net/http" "github.com/docker/distribution/registry/api/errcode" + "github.com/docker/distribution/registry/client/auth/challenge" ) // ErrNoErrorsInBody is returned when an HTTP response body parses to an empty @@ -82,21 +83,52 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error { return errors } +func makeErrorList(err error) []error { + if errL, ok := err.(errcode.Errors); ok { + return []error(errL) + } + return []error{err} +} + +func mergeErrors(err1, err2 error) error { + return errcode.Errors(append(makeErrorList(err1), makeErrorList(err2)...)) +} + // HandleErrorResponse returns error parsed from HTTP response for an // unsuccessful HTTP response code (in the range 400 - 499 inclusive). An // UnexpectedHTTPStatusError returned for response code outside of expected // range. func HandleErrorResponse(resp *http.Response) error { - if resp.StatusCode == 401 { + if resp.StatusCode >= 400 && resp.StatusCode < 500 { + // Check for OAuth errors within the `WWW-Authenticate` header first + // See https://tools.ietf.org/html/rfc6750#section-3 + for _, c := range challenge.ResponseChallenges(resp) { + if c.Scheme == "bearer" { + var err errcode.Error + // codes defined at https://tools.ietf.org/html/rfc6750#section-3.1 + switch c.Parameters["error"] { + case "invalid_token": + err.Code = errcode.ErrorCodeUnauthorized + case "insufficient_scope": + err.Code = errcode.ErrorCodeDenied + default: + continue + } + if description := c.Parameters["error_description"]; description != "" { + err.Message = description + } else { + err.Message = err.Code.Message() + } + + return mergeErrors(err, parseHTTPErrorResponse(resp.StatusCode, resp.Body)) + } + } err := parseHTTPErrorResponse(resp.StatusCode, resp.Body) - if uErr, ok := err.(*UnexpectedHTTPResponseError); ok { + if uErr, ok := err.(*UnexpectedHTTPResponseError); ok && resp.StatusCode == 401 { return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response) } return err } - if resp.StatusCode >= 400 && resp.StatusCode < 500 { - return parseHTTPErrorResponse(resp.StatusCode, resp.Body) - } return &UnexpectedHTTPStatusError{Status: resp.Status} } From c85eb008416f352327b67dce351101591cd5f781 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 10 Nov 2016 15:59:02 -0800 Subject: [PATCH 2/3] Update for distribution vendor Handle updates to reference package. Updates for refactoring of challenge manager. Signed-off-by: Derek McGowan (github: dmcgowan) --- cli/command/image/trust.go | 9 +++++---- daemon/image_pull.go | 2 +- distribution/pull.go | 2 +- distribution/pull_v2.go | 2 +- distribution/push_v2.go | 2 +- migrate/v1/migratev1.go | 2 +- reference/reference.go | 5 +++++ registry/auth.go | 5 +++-- 8 files changed, 18 insertions(+), 11 deletions(-) diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index b8de6a5245..d1106b532e 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -20,6 +20,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/distribution/digest" "github.com/docker/distribution/registry/client/auth" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" "github.com/docker/docker/api/types" registrytypes "github.com/docker/docker/api/types/registry" @@ -291,7 +292,7 @@ func trustedPull(ctx context.Context, cli *command.DockerCli, repoInfo *registry } fmt.Fprintf(cli.Out(), "Pull (%d of %d): %s%s@%s\n", i+1, len(refs), repoInfo.Name(), displayTag, r.digest) - ref, err := reference.WithDigest(repoInfo, r.digest) + ref, err := reference.WithDigest(reference.TrimNamed(repoInfo), r.digest) if err != nil { return err } @@ -305,7 +306,7 @@ func trustedPull(ctx context.Context, cli *command.DockerCli, repoInfo *registry if err != nil { return err } - trustedRef, err := reference.WithDigest(repoInfo, r.digest) + trustedRef, err := reference.WithDigest(reference.TrimNamed(repoInfo), r.digest) if err != nil { return err } @@ -434,7 +435,7 @@ func GetNotaryRepository(streams command.Streams, repoInfo *registry.RepositoryI return nil, err } - challengeManager := auth.NewSimpleChallengeManager() + challengeManager := challenge.NewSimpleManager() resp, err := pingClient.Do(req) if err != nil { @@ -523,7 +524,7 @@ func TrustedReference(ctx context.Context, cli *command.DockerCli, ref reference } - return reference.WithDigest(ref, r.digest) + return reference.WithDigest(reference.TrimNamed(ref), r.digest) } func convertTarget(t client.Target) (target, error) { diff --git a/daemon/image_pull.go b/daemon/image_pull.go index f0b8f55701..7dc1a2d0d9 100644 --- a/daemon/image_pull.go +++ b/daemon/image_pull.go @@ -33,7 +33,7 @@ func (daemon *Daemon) PullImage(ctx context.Context, image, tag string, metaHead var dgst digest.Digest dgst, err = digest.ParseDigest(tag) if err == nil { - ref, err = reference.WithDigest(ref, dgst) + ref, err = reference.WithDigest(reference.TrimNamed(ref), dgst) } else { ref, err = reference.WithTag(ref, tag) } diff --git a/distribution/pull.go b/distribution/pull.go index ea630f03bc..4fab438800 100644 --- a/distribution/pull.go +++ b/distribution/pull.go @@ -206,7 +206,7 @@ func ValidateRepoName(name string) error { } func addDigestReference(store reference.Store, ref reference.Named, dgst digest.Digest, id digest.Digest) error { - dgstRef, err := reference.WithDigest(ref, dgst) + dgstRef, err := reference.WithDigest(reference.TrimNamed(ref), dgst) if err != nil { return err } diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index 3a2015a41e..d62200ddaa 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -671,7 +671,7 @@ func (p *v2Puller) pullManifestList(ctx context.Context, ref reference.Named, mf return "", "", err } - manifestRef, err := reference.WithDigest(ref, manifestDigest) + manifestRef, err := reference.WithDigest(reference.TrimNamed(ref), manifestDigest) if err != nil { return "", "", err } diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 2362f1f752..f2a1f02714 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -331,7 +331,7 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress. continue } - canonicalRef, err := distreference.WithDigest(remoteRef, mountCandidate.Digest) + canonicalRef, err := distreference.WithDigest(distreference.TrimNamed(remoteRef), mountCandidate.Digest) if err != nil { logrus.Errorf("failed to make canonical reference: %v", err) continue diff --git a/migrate/v1/migratev1.go b/migrate/v1/migratev1.go index 90bf627dfe..bc42dd2ca4 100644 --- a/migrate/v1/migratev1.go +++ b/migrate/v1/migratev1.go @@ -331,7 +331,7 @@ func migrateRefs(root, driverName string, rs refAdder, mappings map[string]image continue } if dgst, err := digest.ParseDigest(tag); err == nil { - canonical, err := reference.WithDigest(ref, dgst) + canonical, err := reference.WithDigest(reference.TrimNamed(ref), dgst) if err != nil { logrus.Errorf("migrate tags: invalid digest %q, %q", dgst, err) continue diff --git a/reference/reference.go b/reference/reference.go index b22961b39d..996fc50704 100644 --- a/reference/reference.go +++ b/reference/reference.go @@ -70,6 +70,11 @@ func ParseNamed(s string) (Named, error) { return r, nil } +// TrimNamed removes any tag or digest from the named reference +func TrimNamed(ref Named) Named { + return &namedRef{distreference.TrimNamed(ref)} +} + // WithName returns a named object representing the given string. If the input // is invalid ErrReferenceInvalidFormat will be returned. func WithName(name string) (Named, error) { diff --git a/registry/auth.go b/registry/auth.go index 0bf0450b2c..5da3563f9b 100644 --- a/registry/auth.go +++ b/registry/auth.go @@ -10,6 +10,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/distribution/registry/client/auth" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" "github.com/docker/docker/api/types" registrytypes "github.com/docker/docker/api/types/registry" @@ -255,7 +256,7 @@ func (err PingResponseError) Error() string { // challenge manager for the supported authentication types and // whether v2 was confirmed by the response. If a response is received but // cannot be interpreted a PingResponseError will be returned. -func PingV2Registry(endpoint *url.URL, transport http.RoundTripper) (auth.ChallengeManager, bool, error) { +func PingV2Registry(endpoint *url.URL, transport http.RoundTripper) (challenge.Manager, bool, error) { var ( foundV2 = false v2Version = auth.APIVersion{ @@ -291,7 +292,7 @@ func PingV2Registry(endpoint *url.URL, transport http.RoundTripper) (auth.Challe } } - challengeManager := auth.NewSimpleChallengeManager() + challengeManager := challenge.NewSimpleManager() if err := challengeManager.AddResponse(resp); err != nil { return nil, foundV2, PingResponseError{ Err: err, From 19a93a6e3d4213c56583bb0c843cf9e33d379752 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 10 Nov 2016 15:14:33 -0800 Subject: [PATCH 3/3] Update pull error handling Translate pull errors to provide a more consistent and user friendly error message. Signed-off-by: Derek McGowan (github: dmcgowan) --- distribution/errors.go | 34 ++++++++++++++++++++ distribution/pull.go | 4 +-- integration-cli/docker_cli_by_digest_test.go | 2 +- integration-cli/docker_cli_pull_test.go | 23 +++++-------- 4 files changed, 45 insertions(+), 18 deletions(-) diff --git a/distribution/errors.go b/distribution/errors.go index 1e7630e380..ed8c3c0534 100644 --- a/distribution/errors.go +++ b/distribution/errors.go @@ -5,11 +5,14 @@ import ( "strings" "syscall" + "github.com/Sirupsen/logrus" "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth" "github.com/docker/docker/distribution/xfer" + "github.com/docker/docker/reference" + "github.com/pkg/errors" ) // ErrNoSupport is an error type used for errors indicating that an operation @@ -56,6 +59,37 @@ func shouldV2Fallback(err errcode.Error) bool { return false } +func translatePullError(err error, ref reference.Named) error { + switch v := err.(type) { + case errcode.Errors: + if len(v) != 0 { + for _, extra := range v[1:] { + logrus.Infof("Ignoring extra error returned from registry: %v", extra) + } + return translatePullError(v[0], ref) + } + case errcode.Error: + var newErr error + switch v.Code { + case errcode.ErrorCodeDenied: + // ErrorCodeDenied is used when access to the repository was denied + newErr = errors.Errorf("repository %s not found: does not exist or no read access", ref.Name()) + case v2.ErrorCodeManifestUnknown: + newErr = errors.Errorf("manifest for %s not found", ref.String()) + case v2.ErrorCodeNameUnknown: + newErr = errors.Errorf("repository %s not found", ref.Name()) + } + if newErr != nil { + logrus.Infof("Translating %q to %q", err, newErr) + return newErr + } + case xfer.DoNotRetry: + return translatePullError(v.Err, ref) + } + + return err +} + // continueOnError returns true if we should fallback to the next endpoint // as a result of this error. func continueOnError(err error) bool { diff --git a/distribution/pull.go b/distribution/pull.go index 4fab438800..5307d4ccc9 100644 --- a/distribution/pull.go +++ b/distribution/pull.go @@ -168,7 +168,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo continue } logrus.Errorf("Not continuing with pull after error: %v", err) - return err + return translatePullError(err, ref) } imagePullConfig.ImageEventLogger(ref.String(), repoInfo.Name(), "pull") @@ -179,7 +179,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo lastErr = fmt.Errorf("no endpoints found for %s", ref.String()) } - return lastErr + return translatePullError(lastErr, ref) } // writeStatus writes a status message to out. If layersDownloaded is true, the diff --git a/integration-cli/docker_cli_by_digest_test.go b/integration-cli/docker_cli_by_digest_test.go index a04b04998d..c2d85461a8 100644 --- a/integration-cli/docker_cli_by_digest_test.go +++ b/integration-cli/docker_cli_by_digest_test.go @@ -114,7 +114,7 @@ func testPullByDigestNoFallback(c *check.C) { imageReference := fmt.Sprintf("%s@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", repoName) out, _, err := dockerCmdWithError("pull", imageReference) c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status and correct error message when pulling non-existing image")) - c.Assert(out, checker.Contains, "manifest unknown", check.Commentf("expected non-zero exit status and correct error message when pulling non-existing image")) + c.Assert(out, checker.Contains, fmt.Sprintf("manifest for %s not found", imageReference), check.Commentf("expected non-zero exit status and correct error message when pulling non-existing image")) } func (s *DockerRegistrySuite) TestPullByDigestNoFallback(c *check.C) { diff --git a/integration-cli/docker_cli_pull_test.go b/integration-cli/docker_cli_pull_test.go index 8a86cd39f6..c89adae0c6 100644 --- a/integration-cli/docker_cli_pull_test.go +++ b/integration-cli/docker_cli_pull_test.go @@ -48,12 +48,12 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) { } entries := []entry{ - {"library/asdfasdf", "asdfasdf", "foobar"}, - {"library/asdfasdf", "library/asdfasdf", "foobar"}, - {"library/asdfasdf", "asdfasdf", ""}, - {"library/asdfasdf", "asdfasdf", "latest"}, - {"library/asdfasdf", "library/asdfasdf", ""}, - {"library/asdfasdf", "library/asdfasdf", "latest"}, + {"asdfasdf", "asdfasdf", "foobar"}, + {"asdfasdf", "library/asdfasdf", "foobar"}, + {"asdfasdf", "asdfasdf", ""}, + {"asdfasdf", "asdfasdf", "latest"}, + {"asdfasdf", "library/asdfasdf", ""}, + {"asdfasdf", "library/asdfasdf", "latest"}, } // The option field indicates "-a" or not. @@ -98,18 +98,11 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) { for record := range recordChan { if len(record.option) == 0 { c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out)) - // Hub returns 401 rather than 404 for nonexistent repos over - // the v2 protocol - but we should end up falling back to v1, - // which does return a 404. - tag := record.e.tag - if tag == "" { - tag = "latest" - } - c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s:%s not found", record.e.repo, tag), check.Commentf("expected image not found error messages")) + c.Assert(record.out, checker.Contains, fmt.Sprintf("repository %s not found: does not exist or no read access", record.e.repo), check.Commentf("expected image not found error messages")) } else { // pull -a on a nonexistent registry should fall back as well c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out)) - c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.repo), check.Commentf("expected image not found error messages")) + c.Assert(record.out, checker.Contains, fmt.Sprintf("repository %s not found", record.e.repo), check.Commentf("expected image not found error messages")) c.Assert(record.out, checker.Not(checker.Contains), "unauthorized", check.Commentf(`message should not contain "unauthorized"`)) } }