From be404d7557e930003ad99a63006c939353c5b46b Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Fri, 24 Jul 2015 16:14:04 -0700 Subject: [PATCH] Make the registry client more tolerant about HTTP status codes Generally, all 2xx and 3xx codes should be treated as success. Signed-off-by: Aaron Lehmann --- registry/client/auth/session.go | 3 +- registry/client/blob_writer.go | 12 ++--- registry/client/errors.go | 6 +++ registry/client/repository.go | 64 +++++++++--------------- registry/client/transport/http_reader.go | 7 +-- 5 files changed, 40 insertions(+), 52 deletions(-) diff --git a/registry/client/auth/session.go b/registry/client/auth/session.go index 27e1d9e35..27a2aa719 100644 --- a/registry/client/auth/session.go +++ b/registry/client/auth/session.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/transport" ) @@ -209,7 +210,7 @@ func (th *tokenHandler) fetchToken(params map[string]string) (token string, err } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { + if !client.SuccessStatus(resp.StatusCode) { return "", fmt.Errorf("token auth attempt for registry: %s request failed with status: %d %s", req.URL, resp.StatusCode, http.StatusText(resp.StatusCode)) } diff --git a/registry/client/blob_writer.go b/registry/client/blob_writer.go index 9ebd41839..5f6f01f7f 100644 --- a/registry/client/blob_writer.go +++ b/registry/client/blob_writer.go @@ -44,7 +44,7 @@ func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) { return 0, err } - if resp.StatusCode != http.StatusAccepted { + if !SuccessStatus(resp.StatusCode) { return 0, hbu.handleErrorResponse(resp) } @@ -79,7 +79,7 @@ func (hbu *httpBlobUpload) Write(p []byte) (n int, err error) { return 0, err } - if resp.StatusCode != http.StatusAccepted { + if !SuccessStatus(resp.StatusCode) { return 0, hbu.handleErrorResponse(resp) } @@ -142,7 +142,7 @@ func (hbu *httpBlobUpload) Commit(ctx context.Context, desc distribution.Descrip } defer resp.Body.Close() - if resp.StatusCode != http.StatusCreated { + if !SuccessStatus(resp.StatusCode) { return distribution.Descriptor{}, hbu.handleErrorResponse(resp) } @@ -160,12 +160,10 @@ func (hbu *httpBlobUpload) Cancel(ctx context.Context) error { } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusNoContent, http.StatusNotFound: + if resp.StatusCode == http.StatusNotFound || SuccessStatus(resp.StatusCode) { return nil - default: - return hbu.handleErrorResponse(resp) } + return hbu.handleErrorResponse(resp) } func (hbu *httpBlobUpload) Close() error { diff --git a/registry/client/errors.go b/registry/client/errors.go index 2c168400a..ebd1c36c4 100644 --- a/registry/client/errors.go +++ b/registry/client/errors.go @@ -61,3 +61,9 @@ func handleErrorResponse(resp *http.Response) error { } return &UnexpectedHTTPStatusError{Status: resp.Status} } + +// SuccessStatus returns true if the argument is a successful HTTP response +// code (in the range 200 - 399 inclusive). +func SuccessStatus(status int) bool { + return status >= 200 && status <= 399 +} diff --git a/registry/client/repository.go b/registry/client/repository.go index 50e7b5ce6..d0079f092 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -70,8 +70,7 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusOK: + if SuccessStatus(resp.StatusCode) { var ctlg struct { Repositories []string `json:"repositories"` } @@ -90,8 +89,7 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri if link == "" { returnErr = io.EOF } - - default: + } else { return 0, handleErrorResponse(resp) } @@ -199,8 +197,7 @@ func (ms *manifests) Tags() ([]string, error) { } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusOK: + if SuccessStatus(resp.StatusCode) { b, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, err @@ -214,11 +211,10 @@ func (ms *manifests) Tags() ([]string, error) { } return tagsResponse.Tags, nil - case http.StatusNotFound: + } else if resp.StatusCode == http.StatusNotFound { return nil, nil - default: - return nil, handleErrorResponse(resp) } + return nil, handleErrorResponse(resp) } func (ms *manifests) Exists(dgst digest.Digest) (bool, error) { @@ -238,14 +234,12 @@ func (ms *manifests) ExistsByTag(tag string) (bool, error) { return false, err } - switch resp.StatusCode { - case http.StatusOK: + if SuccessStatus(resp.StatusCode) { return true, nil - case http.StatusNotFound: + } else if resp.StatusCode == http.StatusNotFound { return false, nil - default: - return false, handleErrorResponse(resp) } + return false, handleErrorResponse(resp) } func (ms *manifests) Get(dgst digest.Digest) (*manifest.SignedManifest, error) { @@ -294,8 +288,9 @@ func (ms *manifests) GetByTag(tag string, options ...distribution.ManifestServic } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusOK: + if resp.StatusCode == http.StatusNotModified { + return nil, nil + } else if SuccessStatus(resp.StatusCode) { var sm manifest.SignedManifest decoder := json.NewDecoder(resp.Body) @@ -303,11 +298,8 @@ func (ms *manifests) GetByTag(tag string, options ...distribution.ManifestServic return nil, err } return &sm, nil - case http.StatusNotModified: - return nil, nil - default: - return nil, handleErrorResponse(resp) } + return nil, handleErrorResponse(resp) } func (ms *manifests) Put(m *manifest.SignedManifest) error { @@ -329,13 +321,11 @@ func (ms *manifests) Put(m *manifest.SignedManifest) error { } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusAccepted: + if SuccessStatus(resp.StatusCode) { // TODO(dmcgowan): make use of digest header return nil - default: - return handleErrorResponse(resp) } + return handleErrorResponse(resp) } func (ms *manifests) Delete(dgst digest.Digest) error { @@ -354,12 +344,10 @@ func (ms *manifests) Delete(dgst digest.Digest) error { } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusAccepted: + if SuccessStatus(resp.StatusCode) { return nil - default: - return handleErrorResponse(resp) } + return handleErrorResponse(resp) } type blobs struct { @@ -461,8 +449,7 @@ func (bs *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusAccepted: + if SuccessStatus(resp.StatusCode) { // TODO(dmcgowan): Check for invalid UUID uuid := resp.Header.Get("Docker-Upload-UUID") location, err := sanitizeLocation(resp.Header.Get("Location"), u) @@ -477,9 +464,8 @@ func (bs *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { startedAt: time.Now(), location: location, }, nil - default: - return nil, handleErrorResponse(resp) } + return nil, handleErrorResponse(resp) } func (bs *blobs) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { @@ -508,8 +494,7 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusOK: + if SuccessStatus(resp.StatusCode) { lengthHeader := resp.Header.Get("Content-Length") length, err := strconv.ParseInt(lengthHeader, 10, 64) if err != nil { @@ -521,11 +506,10 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi Size: length, Digest: dgst, }, nil - case http.StatusNotFound: + } else if resp.StatusCode == http.StatusNotFound { return distribution.Descriptor{}, distribution.ErrBlobUnknown - default: - return distribution.Descriptor{}, handleErrorResponse(resp) } + return distribution.Descriptor{}, handleErrorResponse(resp) } func buildCatalogValues(maxEntries int, last string) url.Values { @@ -559,12 +543,10 @@ func (bs *blobStatter) Clear(ctx context.Context, dgst digest.Digest) error { } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusAccepted: + if SuccessStatus(resp.StatusCode) { return nil - default: - return handleErrorResponse(resp) } + return handleErrorResponse(resp) } func (bs *blobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error { diff --git a/registry/client/transport/http_reader.go b/registry/client/transport/http_reader.go index e351bdfe3..b2e74ddb8 100644 --- a/registry/client/transport/http_reader.go +++ b/registry/client/transport/http_reader.go @@ -154,10 +154,11 @@ func (hrs *httpReadSeeker) reader() (io.Reader, error) { return nil, err } - switch { - case resp.StatusCode == 200: + // Normally would use client.SuccessStatus, but that would be a cyclic + // import + if resp.StatusCode >= 200 && resp.StatusCode <= 399 { hrs.rc = resp.Body - default: + } else { defer resp.Body.Close() return nil, fmt.Errorf("unexpected status resolving reader: %v", resp.Status) }