Merge pull request #4523 from errordeveloper/master

Log unexpected responses

remotes/docker/auth/fetch.go

@@ -19,15 +19,13 @@ package auth
 import (
 	"context"
 	"encoding/json"
-	"fmt"
-	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
 	"github.com/containerd/containerd/log"
+	remoteserrors "github.com/containerd/containerd/remotes/errors"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context/ctxhttp"
 )
@@ -38,25 +36,6 @@ var (
 	ErrNoToken = errors.New("authorization server did not include a token in the response")
 )
 
-// ErrUnexpectedStatus is returned if a token request returned with unexpected HTTP status
-type ErrUnexpectedStatus struct {
-	Status     string
-	StatusCode int
-	Body       []byte
-}
-
-func (e ErrUnexpectedStatus) Error() string {
-	return fmt.Sprintf("unexpected status: %s", e.Status)
-}
-
-func newUnexpectedStatusErr(resp *http.Response) error {
-	var b []byte
-	if resp.Body != nil {
-		b, _ = ioutil.ReadAll(io.LimitReader(resp.Body, 64000)) // 64KB
-	}
-	return ErrUnexpectedStatus{Status: resp.Status, StatusCode: resp.StatusCode, Body: b}
-}
-
 // GenerateTokenOptions generates options for fetching a token based on a challenge
 func GenerateTokenOptions(ctx context.Context, host, username, secret string, c Challenge) (TokenOptions, error) {
 	realm, ok := c.Parameters["realm"]
@@ -140,7 +119,7 @@ func FetchTokenWithOAuth(ctx context.Context, client *http.Client, headers http.
 	defer resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
-		return nil, errors.WithStack(newUnexpectedStatusErr(resp))
+		return nil, errors.WithStack(remoteserrors.NewUnexpectedStatusErr(resp))
 	}
 
 	decoder := json.NewDecoder(resp.Body)
@@ -202,7 +181,7 @@ func FetchToken(ctx context.Context, client *http.Client, headers http.Header, t
 	defer resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
-		return nil, errors.WithStack(newUnexpectedStatusErr(resp))
+		return nil, errors.WithStack(remoteserrors.NewUnexpectedStatusErr(resp))
 	}
 
 	decoder := json.NewDecoder(resp.Body)

remotes/docker/authorizer.go

@@ -27,6 +27,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/remotes/docker/auth"
+	remoteerrors "github.com/containerd/containerd/remotes/errors"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -278,7 +279,7 @@ func (ah *authHandler) doBearerAuth(ctx context.Context) (token string, err erro
 		// TODO: Allow setting client_id
 		resp, err := auth.FetchTokenWithOAuth(ctx, ah.client, ah.header, "containerd-client", to)
 		if err != nil {
-			var errStatus auth.ErrUnexpectedStatus
+			var errStatus remoteerrors.ErrUnexpectedStatus
 			if errors.As(err, &errStatus) {
 				// Registries without support for POST may return 404 for POST /v2/token.
 				// As of September 2017, GCR is known to return 404.

remotes/docker/pusher.go

@@ -30,6 +30,7 @@ import (
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/remotes"
+	remoteserrors "github.com/containerd/containerd/remotes/errors"
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -112,8 +113,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 				return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest)
 			}
 		} else if resp.StatusCode != http.StatusNotFound {
-			// TODO: log error
-			return nil, errors.Errorf("unexpected response: %s", resp.Status)
+			err := remoteserrors.NewUnexpectedStatusErr(resp)
+			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
+			return nil, err
 		}
 	}
 
@@ -166,8 +168,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 			})
 			return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest)
 		default:
-			// TODO: log error
-			return nil, errors.Errorf("unexpected response: %s", resp.Status)
+			err := remoteserrors.NewUnexpectedStatusErr(resp)
+			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
+			return nil, err
 		}
 
 		var (
@@ -244,8 +247,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 		switch resp.StatusCode {
 		case http.StatusOK, http.StatusCreated, http.StatusNoContent:
 		default:
-			// TODO: log error
-			pr.CloseWithError(errors.Errorf("unexpected response: %s", resp.Status))
+			err := remoteserrors.NewUnexpectedStatusErr(resp)
+			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
+			pr.CloseWithError(err)
 		}
 		respC <- resp
 	}()

remotes/errors/errors.go

@@ -0,0 +1,46 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package errors
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"net/http"
+)
+
+var _ error = ErrUnexpectedStatus{}
+
+// ErrUnexpectedStatus is returned if a registry API request returned with unexpected HTTP status
+type ErrUnexpectedStatus struct {
+	Status     string
+	StatusCode int
+	Body       []byte
+}
+
+func (e ErrUnexpectedStatus) Error() string {
+	return fmt.Sprintf("unexpected status: %s", e.Status)
+}
+
+// NewUnexpectedStatusErr creates an ErrUnexpectedStatus from HTTP response
+func NewUnexpectedStatusErr(resp *http.Response) error {
+	var b []byte
+	if resp.Body != nil {
+		b, _ = ioutil.ReadAll(io.LimitReader(resp.Body, 64000)) // 64KB
+	}
+	return ErrUnexpectedStatus{Status: resp.Status, StatusCode: resp.StatusCode, Body: b}
+}