diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go index f1a17f606..b4af75e1b 100644 --- a/remotes/docker/resolver.go +++ b/remotes/docker/resolver.go @@ -242,10 +242,9 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp } var ( - firstErr error - paths [][]string - dgst = refspec.Digest() - caps = HostCapabilityPull + paths [][]string + dgst = refspec.Digest() + caps = HostCapabilityPull ) if dgst != "" { @@ -276,6 +275,13 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp return "", ocispec.Descriptor{}, err } + var ( + // firstErr is the most relevant error encountered during resolution. + // We use this to determine the error to return, making sure that the + // error created furthest through the resolution process is returned. + firstErr error + firstErrPriority int + ) for _, u := range paths { for _, host := range hosts { ctx := log.WithLogger(ctx, log.G(ctx).WithField("host", host.Host)) @@ -295,9 +301,9 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp if errors.Is(err, ErrInvalidAuthorization) { err = fmt.Errorf("pull access denied, repository does not exist or may require authorization: %w", err) } - // Store the error for referencing later - if firstErr == nil { + if firstErrPriority < 1 { firstErr = err + firstErrPriority = 1 } log.G(ctx).WithError(err).Info("trying next host") continue // try another host @@ -306,14 +312,19 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp if resp.StatusCode > 299 { if resp.StatusCode == http.StatusNotFound { + if firstErrPriority < 2 { + firstErr = fmt.Errorf("%s: %w", ref, errdefs.ErrNotFound) + firstErrPriority = 2 + } log.G(ctx).Info("trying next host - response was http.StatusNotFound") continue } if resp.StatusCode > 399 { - // Set firstErr when encountering the first non-404 status code. - if firstErr == nil { + if firstErrPriority < 3 { firstErr = remoteerrors.NewUnexpectedStatusErr(resp) + firstErrPriority = 3 } + log.G(ctx).Infof("trying next host - response was %s", resp.Status) continue // try another host } return "", ocispec.Descriptor{}, remoteerrors.NewUnexpectedStatusErr(resp) @@ -384,8 +395,9 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp } // Prevent resolving to excessively large manifests if size > MaxManifestSize { - if firstErr == nil { + if firstErrPriority < 4 { firstErr = fmt.Errorf("rejecting %d byte manifest for %s: %w", size, ref, errdefs.ErrNotFound) + firstErrPriority = 4 } continue } @@ -401,10 +413,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp } } - // If above loop terminates without return, then there was an error. - // "firstErr" contains the first non-404 error. That is, "firstErr == nil" - // means that either no registries were given or each registry returned 404. - + // If above loop terminates without return or error, then no registries + // were provided. if firstErr == nil { firstErr = fmt.Errorf("%s: %w", ref, errdefs.ErrNotFound) } diff --git a/remotes/docker/resolver_test.go b/remotes/docker/resolver_test.go index 52df1f0ec..040a2b978 100644 --- a/remotes/docker/resolver_test.go +++ b/remotes/docker/resolver_test.go @@ -32,6 +32,7 @@ import ( "testing" "time" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/remotes" "github.com/containerd/containerd/remotes/docker/auth" digest "github.com/opencontainers/go-digest" @@ -331,6 +332,7 @@ func TestHostTLSFailureFallbackResolver(t *testing.T) { } runBasicTest(t, "testname", sf) + runNotFoundTest(t, "testname", sf) } func TestHTTPFallbackResolver(t *testing.T) { @@ -638,6 +640,28 @@ func runBasicTest(t *testing.T, name string, sf func(h http.Handler) (string, Re } } +func runNotFoundTest(t *testing.T, name string, sf func(h http.Handler) (string, ResolverOptions, func())) { + var ( + ctx = context.Background() + tag = "latest" + r = http.NewServeMux() + ) + + base, ro, close := sf(logHandler{t, r}) + defer close() + + resolver := NewResolver(ro) + image := fmt.Sprintf("%s/%s:%s", base, name, tag) + + _, _, err := resolver.Resolve(ctx, image) + if err == nil { + t.Fatalf("Expected error resolving %s, got nil", image) + } + if !errors.Is(err, errdefs.ErrNotFound) { + t.Fatalf("Expected error resolving %s to be ErrNotFound, got %v", image, err) + } +} + func testFetch(ctx context.Context, f remotes.Fetcher, desc ocispec.Descriptor) error { r, err := f.Fetch(ctx, desc) if err != nil {