Skip to content

Commit a5c6381

Browse files
estespestesp
authored
Merge pull request #4523 from errordeveloper/master
Log unexpected responses
2 parents 4339431 + 2de5506 commit a5c6381

4 files changed

Lines changed: 61 additions & 31 deletions

File tree

‎remotes/docker/auth/fetch.go‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,13 @@ package auth
1919
import (
2020
"context"
2121
"encoding/json"
22-
"fmt"
23-
"io"
24-
"io/ioutil"
2522
"net/http"
2623
"net/url"
2724
"strings"
2825
"time"
2926

3027
"github.com/containerd/containerd/log"
28+
remoteserrors "github.com/containerd/containerd/remotes/errors"
3129
"github.com/pkg/errors"
3230
"golang.org/x/net/context/ctxhttp"
3331
)
@@ -38,25 +36,6 @@ var (
3836
ErrNoToken = errors.New("authorization server did not include a token in the response")
3937
)
4038

41-
// ErrUnexpectedStatus is returned if a token request returned with unexpected HTTP status
42-
type ErrUnexpectedStatus struct {
43-
Status string
44-
StatusCode int
45-
Body []byte
46-
}
47-
48-
func (e ErrUnexpectedStatus) Error() string {
49-
return fmt.Sprintf("unexpected status: %s", e.Status)
50-
}
51-
52-
func newUnexpectedStatusErr(resp *http.Response) error {
53-
var b []byte
54-
if resp.Body != nil {
55-
b, _ = ioutil.ReadAll(io.LimitReader(resp.Body, 64000)) // 64KB
56-
}
57-
return ErrUnexpectedStatus{Status: resp.Status, StatusCode: resp.StatusCode, Body: b}
58-
}
59-
6039
// GenerateTokenOptions generates options for fetching a token based on a challenge
6140
func GenerateTokenOptions(ctx context.Context, host, username, secret string, c Challenge) (TokenOptions, error) {
6241
realm, ok := c.Parameters["realm"]
@@ -140,7 +119,7 @@ func FetchTokenWithOAuth(ctx context.Context, client *http.Client, headers http.
140119
defer resp.Body.Close()
141120

142121
if resp.StatusCode < 200 || resp.StatusCode >= 400 {
143-
return nil, errors.WithStack(newUnexpectedStatusErr(resp))
122+
return nil, errors.WithStack(remoteserrors.NewUnexpectedStatusErr(resp))
144123
}
145124

146125
decoder := json.NewDecoder(resp.Body)
@@ -202,7 +181,7 @@ func FetchToken(ctx context.Context, client *http.Client, headers http.Header, t
202181
defer resp.Body.Close()
203182

204183
if resp.StatusCode < 200 || resp.StatusCode >= 400 {
205-
return nil, errors.WithStack(newUnexpectedStatusErr(resp))
184+
return nil, errors.WithStack(remoteserrors.NewUnexpectedStatusErr(resp))
206185
}
207186

208187
decoder := json.NewDecoder(resp.Body)

‎remotes/docker/authorizer.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/containerd/containerd/errdefs"
2828
"github.com/containerd/containerd/log"
2929
"github.com/containerd/containerd/remotes/docker/auth"
30+
remoteerrors "github.com/containerd/containerd/remotes/errors"
3031
"github.com/pkg/errors"
3132
"github.com/sirupsen/logrus"
3233
)
@@ -278,7 +279,7 @@ func (ah *authHandler) doBearerAuth(ctx context.Context) (token string, err erro
278279
// TODO: Allow setting client_id
279280
resp, err := auth.FetchTokenWithOAuth(ctx, ah.client, ah.header, "containerd-client", to)
280281
if err != nil {
281-
var errStatus auth.ErrUnexpectedStatus
282+
var errStatus remoteerrors.ErrUnexpectedStatus
282283
if errors.As(err, &errStatus) {
283284
// Registries without support for POST may return 404 for POST /v2/token.
284285
// As of September 2017, GCR is known to return 404.

‎remotes/docker/pusher.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/containerd/containerd/images"
3131
"github.com/containerd/containerd/log"
3232
"github.com/containerd/containerd/remotes"
33+
remoteserrors "github.com/containerd/containerd/remotes/errors"
3334
digest "github.com/opencontainers/go-digest"
3435
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
3536
"github.com/pkg/errors"
@@ -112,8 +113,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
112113
return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest)
113114
}
114115
} else if resp.StatusCode != http.StatusNotFound {
115-
// TODO: log error
116-
return nil, errors.Errorf("unexpected response: %s", resp.Status)
116+
err := remoteserrors.NewUnexpectedStatusErr(resp)
117+
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
118+
return nil, err
117119
}
118120
}
119121

@@ -166,8 +168,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
166168
})
167169
return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest)
168170
default:
169-
// TODO: log error
170-
return nil, errors.Errorf("unexpected response: %s", resp.Status)
171+
err := remoteserrors.NewUnexpectedStatusErr(resp)
172+
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
173+
return nil, err
171174
}
172175

173176
var (
@@ -244,8 +247,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
244247
switch resp.StatusCode {
245248
case http.StatusOK, http.StatusCreated, http.StatusNoContent:
246249
default:
247-
// TODO: log error
248-
pr.CloseWithError(errors.Errorf("unexpected response: %s", resp.Status))
250+
err := remoteserrors.NewUnexpectedStatusErr(resp)
251+
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
252+
pr.CloseWithError(err)
249253
}
250254
respC <- resp
251255
}()

‎remotes/errors/errors.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package errors
18+
19+
import (
20+
"fmt"
21+
"io"
22+
"io/ioutil"
23+
"net/http"
24+
)
25+
26+
var _ error = ErrUnexpectedStatus{}
27+
28+
// ErrUnexpectedStatus is returned if a registry API request returned with unexpected HTTP status
29+
type ErrUnexpectedStatus struct {
30+
Status string
31+
StatusCode int
32+
Body []byte
33+
}
34+
35+
func (e ErrUnexpectedStatus) Error() string {
36+
return fmt.Sprintf("unexpected status: %s", e.Status)
37+
}
38+
39+
// NewUnexpectedStatusErr creates an ErrUnexpectedStatus from HTTP response
40+
func NewUnexpectedStatusErr(resp *http.Response) error {
41+
var b []byte
42+
if resp.Body != nil {
43+
b, _ = ioutil.ReadAll(io.LimitReader(resp.Body, 64000)) // 64KB
44+
}
45+
return ErrUnexpectedStatus{Status: resp.Status, StatusCode: resp.StatusCode, Body: b}
46+
}

0 commit comments

Comments
 (0)