alexvanin/distribution
1
0
Fork 0
forked from TrueCloudLab/distribution
Code Pull requests Activity

registry/client: combine SuccessStatus and HandleErrorResponse

Browse source 
The SuccessStatus acted on the response's status code, and was used to return
early, before checking the same status code with HandleErrorResponse.

This patch combines both functions into a HandleHTTPResponseError, which
returns an error for "non-success" status-codes, which simplifies handling
of responses, and makes some logic slightly more idiomatic.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2023-09-08 14:33:46 +02:00
parent 285b601af9
commit c8ba5d7081
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
5 changed files with 170 additions and 150 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
registry/client/auth/session.go
View file

@ -360,8 +360,7 @@ func (th *tokenHandler) fetchTokenWithOAuth(ctx context.Context, realm *url.URL,
}
defer resp.Body.Close()
if !client.SuccessStatus(resp.StatusCode) {
err := client.HandleErrorResponse(resp)
if err := client.HandleHTTPResponseError(resp); err != nil {
return "", time.Time{}, err
}
@ -443,8 +442,7 @@ func (th *tokenHandler) fetchTokenWithBasicAuth(ctx context.Context, realm *url.
}
defer resp.Body.Close()
if !client.SuccessStatus(resp.StatusCode) {
err := client.HandleErrorResponse(resp)
if err := client.HandleHTTPResponseError(resp); err != nil {
return "", time.Time{}, err
}

16
registry/client/blob_writer.go
View file

@ -33,7 +33,7 @@ func (hbu *httpBlobUpload) handleErrorResponse(resp *http.Response) error {
if resp.StatusCode == http.StatusNotFound {
return distribution.ErrBlobUploadUnknown
}
return HandleErrorResponse(resp)
return HandleHTTPResponseError(resp)
}
func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) {
@ -51,8 +51,8 @@ func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) {
}
defer resp.Body.Close()
if !SuccessStatus(resp.StatusCode) {
return 0, hbu.handleErrorResponse(resp)
if err := hbu.handleErrorResponse(resp); err != nil {
return 0, err
}
hbu.uuid = resp.Header.Get("Docker-Upload-UUID")
@ -87,8 +87,8 @@ func (hbu *httpBlobUpload) Write(p []byte) (n int, err error) {
}
defer resp.Body.Close()
if !SuccessStatus(resp.StatusCode) {
return 0, hbu.handleErrorResponse(resp)
if err := hbu.handleErrorResponse(resp); err != nil {
return 0, err
}
hbu.uuid = resp.Header.Get("Docker-Upload-UUID")
@ -137,8 +137,8 @@ func (hbu *httpBlobUpload) Commit(ctx context.Context, desc distribution.Descrip
}
defer resp.Body.Close()
if !SuccessStatus(resp.StatusCode) {
return distribution.Descriptor{}, hbu.handleErrorResponse(resp)
if err := hbu.handleErrorResponse(resp); err != nil {
return distribution.Descriptor{}, err
}
return hbu.statter.Stat(ctx, desc.Digest)
@ -155,7 +155,7 @@ func (hbu *httpBlobUpload) Cancel(ctx context.Context) error {
}
defer resp.Body.Close()
if resp.StatusCode == http.StatusNotFound || SuccessStatus(resp.StatusCode) {
if resp.StatusCode == http.StatusNotFound {
return nil
}
return hbu.handleErrorResponse(resp)

30
registry/client/errors.go
View file

@ -116,11 +116,16 @@ 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 {
// HandleHTTPResponseError returns error parsed from HTTP response, if any.
// It returns nil if no error occurred (HTTP status 200-399), or an error
// for unsuccessful HTTP response codes (in the range 400 - 499 inclusive).
// If possible, it returns a typed error, but an UnexpectedHTTPStatusError
// is returned for response code outside the expected range (HTTP status < 200
// and > 500).
func HandleHTTPResponseError(resp *http.Response) error {
if resp.StatusCode >= 200 && resp.StatusCode <= 399 {
return nil
}
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
@ -153,8 +158,23 @@ func HandleErrorResponse(resp *http.Response) error {
return &UnexpectedHTTPStatusError{Status: resp.Status}
}
// 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.
//
// Deprecated: use [HandleHTTPResponseError] and check the error.
func HandleErrorResponse(resp *http.Response) error {
if resp.StatusCode >= 200 && resp.StatusCode <= 399 {
return &UnexpectedHTTPStatusError{Status: resp.Status}
}
return HandleHTTPResponseError(resp)
}
// SuccessStatus returns true if the argument is a successful HTTP response
// code (in the range 200 - 399 inclusive).
//
// Deprecated: use [HandleHTTPResponseError] and check the error.
func SuccessStatus(status int) bool {
return status >= 200 && status <= 399
}

43
registry/client/errors_test.go
View file

@ -14,7 +14,18 @@ type nopCloser struct {
func (nopCloser) Close() error { return nil }
func TestHandleErrorResponse401ValidBody(t *testing.T) {
func TestHandleHTTPResponseError200ValidBody(t *testing.T) {
response := &http.Response{
Status: "200 OK",
StatusCode: 200,
}
err := HandleHTTPResponseError(response)
if err != nil {
t.Errorf("Expected no error, got: %v", err)
}
}
func TestHandleHTTPResponseError401ValidBody(t *testing.T) {
json := `{"errors":[{"code":"UNAUTHORIZED","message":"action requires authentication"}]}`
response := &http.Response{
Status: "401 Unauthorized",
@ -22,7 +33,7 @@ func TestHandleErrorResponse401ValidBody(t *testing.T) {
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := "unauthorized: action requires authentication"
if !strings.Contains(err.Error(), expectedMsg) {
@ -30,7 +41,7 @@ func TestHandleErrorResponse401ValidBody(t *testing.T) {
}
}
func TestHandleErrorResponse401WithInvalidBody(t *testing.T) {
func TestHandleHTTPResponseError401WithInvalidBody(t *testing.T) {
json := "{invalid json}"
response := &http.Response{
Status: "401 Unauthorized",
@ -38,7 +49,7 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) {
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := "unauthorized: authentication required"
if !strings.Contains(err.Error(), expectedMsg) {
@ -46,7 +57,7 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) {
}
}
func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) {
func TestHandleHTTPResponseErrorExpectedStatusCode400ValidBody(t *testing.T) {
json := `{"errors":[{"code":"DIGEST_INVALID","message":"provided digest does not match"}]}`
response := &http.Response{
Status: "400 Bad Request",
@ -54,7 +65,7 @@ func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) {
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := "digest invalid: provided digest does not match"
if !strings.Contains(err.Error(), expectedMsg) {
@ -62,7 +73,7 @@ func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) {
}
}
func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
func TestHandleHTTPResponseErrorExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
json := `{"randomkey": "randomvalue"}`
response := &http.Response{
Status: "404 Not Found",
@ -70,7 +81,7 @@ func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := `error parsing HTTP 404 response body: no error details found in HTTP response body: "{\"randomkey\": \"randomvalue\"}"`
if !strings.Contains(err.Error(), expectedMsg) {
@ -78,7 +89,7 @@ func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
}
}
func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
func TestHandleHTTPResponseErrorExpectedStatusCode404InvalidBody(t *testing.T) {
json := "{invalid json}"
response := &http.Response{
Status: "404 Not Found",
@ -86,7 +97,7 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := "error parsing HTTP 404 response body: invalid character 'i' looking for beginning of object key string: \"{invalid json}\""
if !strings.Contains(err.Error(), expectedMsg) {
@ -94,14 +105,14 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
}
}
func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
func TestHandleHTTPResponseErrorUnexpectedStatusCode501(t *testing.T) {
response := &http.Response{
Status: "501 Not Implemented",
StatusCode: 501,
Body: nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")},
Header: http.Header{"Content-Type": []string{"application/json"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := "received unexpected HTTP status: 501 Not Implemented"
if !strings.Contains(err.Error(), expectedMsg) {
@ -109,7 +120,7 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
}
}
func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) {
func TestHandleHTTPResponseErrorInsufficientPrivileges403(t *testing.T) {
json := `{"details":"requesting higher privileges than access token allows"}`
response := &http.Response{
Status: "403 Forbidden",
@ -117,7 +128,7 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) {
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
expectedMsg := "denied: requesting higher privileges than access token allows"
if !strings.Contains(err.Error(), expectedMsg) {
@ -125,14 +136,14 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) {
}
}
func TestHandleErrorResponseNonJson(t *testing.T) {
func TestHandleHTTPResponseErrorNonJson(t *testing.T) {
msg := `{"details":"requesting higher privileges than access token allows"}`
response := &http.Response{
Status: "403 Forbidden",
StatusCode: 403,
Body: nopCloser{bytes.NewBufferString(msg)},
}
err := HandleErrorResponse(response)
err := HandleHTTPResponseError(response)
if !strings.Contains(err.Error(), msg) {
t.Errorf("Expected %q, got: %q", msg, err.Error())

225
registry/client/repository.go
View file

@ -89,8 +89,6 @@ type registry struct {
// of the slice, starting at the value provided in 'last'. The number of entries will be returned along with io.EOF if there
// are no more entries
func (r *registry) Repositories(ctx context.Context, entries []string, last string) (int, error) {
var numFilled int
var returnErr error
values := buildCatalogValues(len(entries), last)
u, err := r.ub.BuildCatalogURL(values)
@ -108,28 +106,27 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
var ctlg struct {
Repositories []string `json:"repositories"`
}
decoder := json.NewDecoder(resp.Body)
if err := decoder.Decode(&ctlg); err != nil {
return 0, err
}
copy(entries, ctlg.Repositories)
numFilled = len(ctlg.Repositories)
link := resp.Header.Get("Link")
if link == "" {
returnErr = io.EOF
}
} else {
return 0, HandleErrorResponse(resp)
if err := HandleHTTPResponseError(resp); err != nil {
return 0, err
}
return numFilled, returnErr
var ctlg struct {
Repositories []string `json:"repositories"`
}
decoder := json.NewDecoder(resp.Body)
if err := decoder.Decode(&ctlg); err != nil {
return 0, err
}
copy(entries, ctlg.Repositories)
numFilled := len(ctlg.Repositories)
if resp.Header.Get("Link") == "" {
return numFilled, io.EOF
}
return numFilled, nil
}
// NewRepository creates a new Repository for the given repository name and base URL.
@ -200,18 +197,17 @@ type tags struct {
// All returns all tags
func (t *tags) All(ctx context.Context) ([]string, error) {
var tags []string
listURLStr, err := t.ub.BuildTagsURL(t.name)
if err != nil {
return tags, err
return nil, err
}
listURL, err := url.Parse(listURLStr)
if err != nil {
return tags, err
return nil, err
}
var allTags []string
for {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, listURL.String(), nil)
if err != nil {
@ -219,36 +215,36 @@ func (t *tags) All(ctx context.Context) ([]string, error) {
}
resp, err := t.client.Do(req)
if err != nil {
return tags, err
return allTags, err
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
b, err := io.ReadAll(resp.Body)
if err := HandleHTTPResponseError(resp); err != nil {
return allTags, err
}
b, err := io.ReadAll(resp.Body)
if err != nil {
return allTags, err
}
tagsResponse := struct {
Tags []string `json:"tags"`
}{}
if err := json.Unmarshal(b, &tagsResponse); err != nil {
return allTags, err
}
allTags = append(allTags, tagsResponse.Tags...)
if link := resp.Header.Get("Link"); link != "" {
firsLink, _, _ := strings.Cut(link, ";")
linkURL, err := url.Parse(strings.Trim(firsLink, "<>"))
if err != nil {
return tags, err
return allTags, err
}
tagsResponse := struct {
Tags []string `json:"tags"`
}{}
if err := json.Unmarshal(b, &tagsResponse); err != nil {
return tags, err
}
tags = append(tags, tagsResponse.Tags...)
if link := resp.Header.Get("Link"); link != "" {
firsLink, _, _ := strings.Cut(link, ";")
linkURL, err := url.Parse(strings.Trim(firsLink, "<>"))
if err != nil {
return tags, err
}
listURL = listURL.ResolveReference(linkURL)
} else {
return tags, nil
}
listURL = listURL.ResolveReference(linkURL)
} else {
return tags, HandleErrorResponse(resp)
return allTags, nil
}
}
}
@ -345,7 +341,7 @@ func (t *tags) Get(ctx context.Context, tag string) (distribution.Descriptor, er
if resp.StatusCode >= 200 && resp.StatusCode < 400 {
return descriptorFromResponse(resp)
}
return distribution.Descriptor{}, HandleErrorResponse(resp)
return distribution.Descriptor{}, HandleHTTPResponseError(resp)
}
}
@ -378,10 +374,7 @@ func (t *tags) Untag(ctx context.Context, tag string) error {
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
return nil
}
return HandleErrorResponse(resp)
return HandleHTTPResponseError(resp)
}
type manifests struct {
@ -411,12 +404,14 @@ func (ms *manifests) Exists(ctx context.Context, dgst digest.Digest) (bool, erro
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
return true, nil
} else if resp.StatusCode == http.StatusNotFound {
if resp.StatusCode == http.StatusNotFound {
return false, nil
}
return false, HandleErrorResponse(resp)
if err := HandleHTTPResponseError(resp); err != nil {
return false, err
}
return true, nil
}
// AddEtagToTag allows a client to supply an eTag to Get which will be
@ -517,25 +512,27 @@ func (ms *manifests) Get(ctx context.Context, dgst digest.Digest, options ...dis
defer resp.Body.Close()
if resp.StatusCode == http.StatusNotModified {
return nil, distribution.ErrManifestNotModified
} else if SuccessStatus(resp.StatusCode) {
if contentDgst != nil {
dgst, err := digest.Parse(resp.Header.Get("Docker-Content-Digest"))
if err == nil {
*contentDgst = dgst
}
}
mt := resp.Header.Get("Content-Type")
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
m, _, err := distribution.UnmarshalManifest(mt, body)
if err != nil {
return nil, err
}
return m, nil
}
return nil, HandleErrorResponse(resp)
if err := HandleHTTPResponseError(resp); err != nil {
return nil, err
}
if contentDgst != nil {
dgst, err := digest.Parse(resp.Header.Get("Docker-Content-Digest"))
if err == nil {
*contentDgst = dgst
}
}
mt := resp.Header.Get("Content-Type")
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
m, _, err := distribution.UnmarshalManifest(mt, body)
if err != nil {
return nil, err
}
return m, nil
}
// Put puts a manifest. A tag can be specified using an options parameter which uses some shared state to hold the
@ -594,17 +591,16 @@ func (ms *manifests) Put(ctx context.Context, m distribution.Manifest, options .
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
dgstHeader := resp.Header.Get("Docker-Content-Digest")
dgst, err := digest.Parse(dgstHeader)
if err != nil {
return "", err
}
return dgst, nil
if err := HandleHTTPResponseError(resp); err != nil {
return "", err
}
return "", HandleErrorResponse(resp)
dgst, err := digest.Parse(resp.Header.Get("Docker-Content-Digest"))
if err != nil {
return "", err
}
return dgst, nil
}
func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error {
@ -627,10 +623,7 @@ func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error {
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
return nil
}
return HandleErrorResponse(resp)
return HandleHTTPResponseError(resp)
}
// todo(richardscothern): Restore interface and implementation with merge of #1050
@ -689,7 +682,7 @@ func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekClose
if resp.StatusCode == http.StatusNotFound {
return distribution.ErrBlobUnknown
}
return HandleErrorResponse(resp)
return HandleHTTPResponseError(resp)
}), nil
}
@ -732,13 +725,11 @@ func (bs *blobs) Put(ctx context.Context, mediaType string, p []byte) (distribut
return distribution.Descriptor{}, fmt.Errorf("short copy: wrote %d of %d", n, len(p))
}
desc := distribution.Descriptor{
return writer.Commit(ctx, distribution.Descriptor{
MediaType: mediaType,
Size: int64(len(p)),
Digest: dgstr.Digest(),
}
return writer.Commit(ctx, desc)
})
}
type optionFunc func(interface{}) error
@ -827,7 +818,7 @@ func (bs *blobs) Create(ctx context.Context, options ...distribution.BlobCreateO
location: location,
}, nil
default:
return nil, HandleErrorResponse(resp)
return nil, HandleHTTPResponseError(resp)
}
}
@ -877,26 +868,29 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
lengthHeader := resp.Header.Get("Content-Length")
if lengthHeader == "" {
return distribution.Descriptor{}, fmt.Errorf("missing content-length header for request: %s", u)
}
length, err := strconv.ParseInt(lengthHeader, 10, 64)
if err != nil {
return distribution.Descriptor{}, fmt.Errorf("error parsing content-length: %v", err)
}
return distribution.Descriptor{
MediaType: resp.Header.Get("Content-Type"),
Size: length,
Digest: dgst,
}, nil
} else if resp.StatusCode == http.StatusNotFound {
if resp.StatusCode == http.StatusNotFound {
return distribution.Descriptor{}, distribution.ErrBlobUnknown
}
return distribution.Descriptor{}, HandleErrorResponse(resp)
if err := HandleHTTPResponseError(resp); err != nil {
return distribution.Descriptor{}, err
}
lengthHeader := resp.Header.Get("Content-Length")
if lengthHeader == "" {
return distribution.Descriptor{}, fmt.Errorf("missing content-length header for request: %s", u)
}
length, err := strconv.ParseInt(lengthHeader, 10, 64)
if err != nil {
return distribution.Descriptor{}, fmt.Errorf("error parsing content-length: %v", err)
}
return distribution.Descriptor{
MediaType: resp.Header.Get("Content-Type"),
Size: length,
Digest: dgst,
}, nil
}
func buildCatalogValues(maxEntries int, last string) url.Values {
@ -934,10 +928,7 @@ func (bs *blobStatter) Clear(ctx context.Context, dgst digest.Digest) error {
}
defer resp.Body.Close()
if SuccessStatus(resp.StatusCode) {
return nil
}
return HandleErrorResponse(resp)
return HandleHTTPResponseError(resp)
}
func (bs *blobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error {