Skip to content

Commit f30be44

Browse files
dmcgowank8s-infra-cherrypick-robot
dmcgowan
authored and
k8s-infra-cherrypick-robot
committed
Update fetcher errors to include full registry error
Currently the registry error does not include details or allow the caller access to the original error body. Signed-off-by: Derek McGowan <[email protected]>
1 parent 64ed272 commit f30be44

4 files changed

Lines changed: 36 additions & 22 deletions

File tree

‎core/remotes/docker/errcode.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@ package docker
1818

1919
import (
2020
"encoding/json"
21+
"errors"
2122
"fmt"
23+
"net/http"
2224
"strings"
25+
26+
remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors"
2327
)
2428

2529
// ErrorCoder is the base interface for ErrorCode and Error allowing
@@ -281,3 +285,21 @@ func (errs *Errors) UnmarshalJSON(data []byte) error {
281285
*errs = newErrs
282286
return nil
283287
}
288+
289+
func unexpectedResponseErr(resp *http.Response) (retErr error) {
290+
retErr = remoteerrors.NewUnexpectedStatusErr(resp)
291+
292+
// Decode registry error if provided
293+
if rerr := retErr.(remoteerrors.ErrUnexpectedStatus); len(rerr.Body) > 0 {
294+
var registryErr Errors
295+
if err := json.Unmarshal(rerr.Body, &registryErr); err == nil && registryErr.Len() > 0 {
296+
// Join the unexpected error with the typed errors, when printed it will
297+
// show the unexpected error message and the registry errors. The body
298+
// is always excluded from the unexpected error message. This also allows
299+
// clients to decode into either type.
300+
retErr = errors.Join(rerr, registryErr)
301+
}
302+
}
303+
304+
return
305+
}

‎core/remotes/docker/fetcher_test.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -534,13 +534,11 @@ func TestDockerFetcherOpen(t *testing.T) {
534534
assert.Equal(t, tt.want, got)
535535
assert.Equal(t, 0, tt.retries)
536536
if tt.wantErr {
537-
var expectedError error
537+
expectedError := fmt.Sprintf("unexpected status from GET request to %s/ns: %v %s", s.URL, tt.mockedStatus, http.StatusText(tt.mockedStatus))
538538
if tt.wantServerMessageError {
539-
expectedError = fmt.Errorf("unexpected status code %v/ns: %v %s - Server message: %s", s.URL, tt.mockedStatus, http.StatusText(tt.mockedStatus), tt.mockedErr.Error())
540-
} else if tt.wantPlainError {
541-
expectedError = fmt.Errorf("unexpected status code %v/ns: %v %s", s.URL, tt.mockedStatus, http.StatusText(tt.mockedStatus))
539+
expectedError += "\n" + tt.mockedErr.Error()
542540
}
543-
assert.Equal(t, expectedError.Error(), err.Error())
541+
assert.Equal(t, expectedError, err.Error())
544542

545543
}
546544

‎core/remotes/docker/pusher.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
"github.com/containerd/containerd/v2/core/content"
3737
"github.com/containerd/containerd/v2/core/images"
3838
"github.com/containerd/containerd/v2/core/remotes"
39-
remoteserrors "github.com/containerd/containerd/v2/core/remotes/errors"
4039
)
4140

4241
type dockerPusher struct {
@@ -149,8 +148,8 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
149148
return nil, fmt.Errorf("content %v on remote: %w", desc.Digest, errdefs.ErrAlreadyExists)
150149
}
151150
} else if resp.StatusCode != http.StatusNotFound {
152-
err := remoteserrors.NewUnexpectedStatusErr(resp)
153-
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
151+
err := unexpectedResponseErr(resp)
152+
log.G(ctx).WithError(err).Debug("unexpected response")
154153
resp.Body.Close()
155154
return nil, err
156155
}
@@ -224,8 +223,8 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
224223
})
225224
return nil, fmt.Errorf("content %v on remote: %w", desc.Digest, errdefs.ErrAlreadyExists)
226225
default:
227-
err := remoteserrors.NewUnexpectedStatusErr(resp)
228-
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
226+
err := unexpectedResponseErr(resp)
227+
log.G(ctx).WithError(err).Debug("unexpected response")
229228
return nil, err
230229
}
231230

@@ -299,8 +298,8 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
299298
switch resp.StatusCode {
300299
case http.StatusOK, http.StatusCreated, http.StatusNoContent:
301300
default:
302-
err := remoteserrors.NewUnexpectedStatusErr(resp)
303-
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
301+
err := unexpectedResponseErr(resp)
302+
log.G(ctx).WithError(err).Debug("unexpected response")
304303
pushw.setError(err)
305304
return
306305
}
@@ -513,7 +512,7 @@ func (pw *pushWriter) Commit(ctx context.Context, size int64, expected digest.Di
513512
switch resp.StatusCode {
514513
case http.StatusOK, http.StatusCreated, http.StatusNoContent, http.StatusAccepted:
515514
default:
516-
return remoteserrors.NewUnexpectedStatusErr(resp)
515+
return unexpectedResponseErr(resp)
517516
}
518517

519518
status, err := pw.tracker.GetStatus(pw.ref)

‎core/remotes/docker/resolver.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package docker
1919
import (
2020
"context"
2121
"crypto/tls"
22-
"encoding/json"
2322
"errors"
2423
"fmt"
2524
"io"
@@ -39,7 +38,6 @@ import (
3938

4039
"github.com/containerd/containerd/v2/core/images"
4140
"github.com/containerd/containerd/v2/core/remotes"
42-
remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors"
4341
"github.com/containerd/containerd/v2/core/transfer"
4442
"github.com/containerd/containerd/v2/pkg/reference"
4543
"github.com/containerd/containerd/v2/pkg/tracing"
@@ -332,13 +330,13 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
332330
}
333331
if resp.StatusCode > 399 {
334332
if firstErrPriority < 3 {
335-
firstErr = remoteerrors.NewUnexpectedStatusErr(resp)
333+
firstErr = unexpectedResponseErr(resp)
336334
firstErrPriority = 3
337335
}
338336
log.G(ctx).Infof("%s after status: %s", nextHostOrFail(i), resp.Status)
339337
continue // try another host
340338
}
341-
return "", ocispec.Descriptor{}, remoteerrors.NewUnexpectedStatusErr(resp)
339+
return "", ocispec.Descriptor{}, unexpectedResponseErr(resp)
342340
}
343341
size := resp.ContentLength
344342
contentType := getManifestMediaType(resp)
@@ -653,11 +651,8 @@ func withErrorCheck(r *request, resp *http.Response) error {
653651
if resp.StatusCode == http.StatusNotFound {
654652
return fmt.Errorf("content at %v not found: %w", r.String(), errdefs.ErrNotFound)
655653
}
656-
var registryErr Errors
657-
if err := json.NewDecoder(resp.Body).Decode(&registryErr); err != nil || registryErr.Len() < 1 {
658-
return fmt.Errorf("unexpected status code %v: %v", r.String(), resp.Status)
659-
}
660-
return fmt.Errorf("unexpected status code %v: %s - Server message: %s", r.String(), resp.Status, registryErr.Error())
654+
655+
return unexpectedResponseErr(resp)
661656
}
662657
return nil
663658
}

0 commit comments

Comments
 (0)