From 8aacbb3ba90fbfe1b7abf39dfc541925b7043052 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Jan 2024 13:20:35 +0100 Subject: [PATCH] api: fix "GET /distribution" endpoint ignoring mirrors If the daemon is configured to use a mirror for the default (Docker Hub) registry, the endpoint did not fall back to querying the upstream if the mirror did not contain the given reference. If the daemon is configured to use a mirror for the default (Docker Hub) registry, did not fall back to querying the upstream if the mirror did not contain the given reference. For pull-through registry-mirrors, this was not a problem, as in that case the registry would forward the request, but for other mirrors, no fallback would happen. This was inconsistent with how "pulling" images handled this situation; when pulling images, both the mirror and upstream would be tried. This problem was caused by the logic used in GetRepository, which had an optimization to only return the first registry it was successfully able to configure (and connect to), with the assumption that the mirror either contained all images used, or to be configured as a pull-through mirror. This patch: - Introduces a GetRepositories method, which returns all candidates (both mirror(s) and upstream). - Updates the endpoint to try all Before this patch: # the daemon is configured to use a mirror for Docker Hub cat /etc/docker/daemon.json { "registry-mirrors": ["http://localhost:5000"]} # start the mirror (empty registry, not configured as pull-through mirror) docker run -d --name registry -p 127.0.0.1:5000:5000 registry:2 # querying the endpoint fails, because the image-manifest is not found in the mirror: curl -s --unix-socket /var/run/docker.sock http://localhost/v1.43/distribution/docker.io/library/hello-world:latest/json { "message": "manifest unknown: manifest unknown" } With this patch applied: # the daemon is configured to use a mirror for Docker Hub cat /etc/docker/daemon.json { "registry-mirrors": ["http://localhost:5000"]} # start the mirror (empty registry, not configured as pull-through mirror) docker run -d --name registry -p 127.0.0.1:5000:5000 registry:2 # querying the endpoint succeeds (manifest is fetched from the upstream Docker Hub registry): curl -s --unix-socket /var/run/docker.sock http://localhost/v1.43/distribution/docker.io/library/hello-world:latest/json | jq . { "Descriptor": { "mediaType": "application/vnd.oci.image.index.v1+json", "digest": "sha256:1b9844d846ce3a6a6af7013e999a373112c3c0450aca49e155ae444526a2c45e", "size": 3849 }, "Platforms": [ { "architecture": "amd64", "os": "linux" } ] } Signed-off-by: Sebastiaan van Stijn --- api/server/router/distribution/backend.go | 2 +- .../distribution/distribution_routes.go | 49 ++++++++++++++----- daemon/cluster/executor/backend.go | 1 + daemon/daemon.go | 16 ++++++ distribution/repository.go | 41 ++++++++++++++-- 5 files changed, 93 insertions(+), 16 deletions(-) diff --git a/api/server/router/distribution/backend.go b/api/server/router/distribution/backend.go index c28f548093842..1c582389a99b4 100644 --- a/api/server/router/distribution/backend.go +++ b/api/server/router/distribution/backend.go @@ -11,5 +11,5 @@ import ( // Backend is all the methods that need to be implemented // to provide image specific functionality. type Backend interface { - GetRepository(context.Context, reference.Named, *registry.AuthConfig) (distribution.Repository, error) + GetRepositories(context.Context, reference.Named, *registry.AuthConfig) ([]distribution.Repository, error) } diff --git a/api/server/router/distribution/distribution_routes.go b/api/server/router/distribution/distribution_routes.go index 66dd053290dfd..9fb0f47a92ec8 100644 --- a/api/server/router/distribution/distribution_routes.go +++ b/api/server/router/distribution/distribution_routes.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/distribution/reference" + "github.com/docker/distribution" "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" @@ -42,24 +43,50 @@ func (s *distributionRouter) getDistributionInfo(ctx context.Context, w http.Res // For a search it is not an error if no auth was given. Ignore invalid // AuthConfig to increase compatibility with the existing API. authConfig, _ := registry.DecodeAuthConfig(r.Header.Get(registry.AuthHeader)) - distrepo, err := s.backend.GetRepository(ctx, namedRef, authConfig) + repos, err := s.backend.GetRepositories(ctx, namedRef, authConfig) if err != nil { return err } - blobsrvc := distrepo.Blobs(ctx) + // Fetch the manifest; if a mirror is configured, try the mirror first, + // but continue with upstream on failure. + // + // FIXME(thaJeztah): construct "repositories" on-demand; + // GetRepositories() will attempt to connect to all endpoints (registries), + // but we may only need the first one if it contains the manifest we're + // looking for, or if the configured mirror is a pull-through mirror. + // + // Logic for this could be implemented similar to "distribution.Pull()", + // which uses the "pullEndpoints" utility to iterate over the list + // of endpoints; + // + // - https://github.com/moby/moby/blob/12c7411b6b7314bef130cd59f1c7384a7db06d0b/distribution/pull.go#L17-L31 + // - https://github.com/moby/moby/blob/12c7411b6b7314bef130cd59f1c7384a7db06d0b/distribution/pull.go#L76-L152 + var lastErr error + for _, repo := range repos { + distributionInspect, err := s.fetchManifest(ctx, repo, namedRef) + if err != nil { + lastErr = err + continue + } + return httputils.WriteJSON(w, http.StatusOK, distributionInspect) + } + return lastErr +} + +func (s *distributionRouter) fetchManifest(ctx context.Context, distrepo distribution.Repository, namedRef reference.Named) (registry.DistributionInspect, error) { var distributionInspect registry.DistributionInspect if canonicalRef, ok := namedRef.(reference.Canonical); !ok { namedRef = reference.TagNameOnly(namedRef) taggedRef, ok := namedRef.(reference.NamedTagged) if !ok { - return errdefs.InvalidParameter(errors.Errorf("image reference not tagged: %s", image)) + return registry.DistributionInspect{}, errdefs.InvalidParameter(errors.Errorf("image reference not tagged: %s", namedRef)) } descriptor, err := distrepo.Tags(ctx).Get(ctx, taggedRef.Tag()) if err != nil { - return err + return registry.DistributionInspect{}, err } distributionInspect.Descriptor = ocispec.Descriptor{ MediaType: descriptor.MediaType, @@ -76,7 +103,7 @@ func (s *distributionRouter) getDistributionInfo(ctx context.Context, w http.Res // we have a digest, so we can retrieve the manifest mnfstsrvc, err := distrepo.Manifests(ctx) if err != nil { - return err + return registry.DistributionInspect{}, err } mnfst, err := mnfstsrvc.Get(ctx, distributionInspect.Descriptor.Digest) if err != nil { @@ -88,14 +115,14 @@ func (s *distributionRouter) getDistributionInfo(ctx context.Context, w http.Res reference.ErrNameEmpty, reference.ErrNameTooLong, reference.ErrNameNotCanonical: - return errdefs.InvalidParameter(err) + return registry.DistributionInspect{}, errdefs.InvalidParameter(err) } - return err + return registry.DistributionInspect{}, err } mediaType, payload, err := mnfst.Payload() if err != nil { - return err + return registry.DistributionInspect{}, err } // update MediaType because registry might return something incorrect distributionInspect.Descriptor.MediaType = mediaType @@ -116,7 +143,8 @@ func (s *distributionRouter) getDistributionInfo(ctx context.Context, w http.Res }) } case *schema2.DeserializedManifest: - configJSON, err := blobsrvc.Get(ctx, mnfstObj.Config.Digest) + blobStore := distrepo.Blobs(ctx) + configJSON, err := blobStore.Get(ctx, mnfstObj.Config.Digest) var platform ocispec.Platform if err == nil { err := json.Unmarshal(configJSON, &platform) @@ -131,6 +159,5 @@ func (s *distributionRouter) getDistributionInfo(ctx context.Context, w http.Res } distributionInspect.Platforms = append(distributionInspect.Platforms, platform) } - - return httputils.WriteJSON(w, http.StatusOK, distributionInspect) + return distributionInspect, nil } diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index 4cd9a2391dd4e..60267953c34be 100644 --- a/daemon/cluster/executor/backend.go +++ b/daemon/cluster/executor/backend.go @@ -78,5 +78,6 @@ type VolumeBackend interface { type ImageBackend interface { PullImage(ctx context.Context, ref reference.Named, platform *ocispec.Platform, metaHeaders map[string][]string, authConfig *registry.AuthConfig, outStream io.Writer) error GetRepository(context.Context, reference.Named, *registry.AuthConfig) (distribution.Repository, error) + GetRepositories(context.Context, reference.Named, *registry.AuthConfig) ([]distribution.Repository, error) GetImage(ctx context.Context, refOrID string, options opts.GetImageOpts) (*image.Image, error) } diff --git a/daemon/daemon.go b/daemon/daemon.go index 397d28e1aa1cc..73f7d0db6758c 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1595,3 +1595,19 @@ func (i *imageBackend) GetRepository(ctx context.Context, ref reference.Named, a }, }) } + +// GetRepositories returns a list of repositories configured for the given +// reference. Multiple repositories can be returned if the reference is for +// the default (Docker Hub) registry and a mirror is configured, but it omits +// registries that were not reachable (pinging the /v2/ endpoint failed). +// +// It returns an error if it was unable to reach any of the registries for +// the given reference, or if the provided reference is invalid. +func (i *imageBackend) GetRepositories(ctx context.Context, ref reference.Named, authConfig *registrytypes.AuthConfig) ([]dist.Repository, error) { + return distribution.GetRepositories(ctx, ref, &distribution.ImagePullConfig{ + Config: distribution.Config{ + AuthConfig: authConfig, + RegistryService: i.registryService, + }, + }) +} diff --git a/distribution/repository.go b/distribution/repository.go index 5b83197521c34..c5229f7c8d63a 100644 --- a/distribution/repository.go +++ b/distribution/repository.go @@ -3,6 +3,7 @@ package distribution import ( "context" + "github.com/containerd/log" "github.com/distribution/reference" "github.com/docker/distribution" "github.com/docker/docker/errdefs" @@ -10,6 +11,25 @@ import ( // GetRepository returns a repository from the registry. func GetRepository(ctx context.Context, ref reference.Named, config *ImagePullConfig) (repository distribution.Repository, lastError error) { + repos, err := getRepositories(ctx, ref, config, true) + if len(repos) == 0 { + return nil, err + } + return repos[0], nil +} + +// GetRepositories returns a list of repositories configured for the given +// reference. Multiple repositories can be returned if the reference is for +// the default (Docker Hub) registry and a mirror is configured, but it omits +// registries that were not reachable (pinging the /v2/ endpoint failed). +// +// It returns an error if it was unable to reach any of the registries for +// the given reference, or if the provided reference is invalid. +func GetRepositories(ctx context.Context, ref reference.Named, config *ImagePullConfig) ([]distribution.Repository, error) { + return getRepositories(ctx, ref, config, false) +} + +func getRepositories(ctx context.Context, ref reference.Named, config *ImagePullConfig, firstOnly bool) ([]distribution.Repository, error) { repoInfo, err := config.RegistryService.ResolveRepository(ref) if err != nil { return nil, errdefs.InvalidParameter(err) @@ -24,11 +44,24 @@ func GetRepository(ctx context.Context, ref reference.Named, config *ImagePullCo return nil, err } + var ( + repositories []distribution.Repository + lastError error + ) for _, endpoint := range endpoints { - repository, lastError = newRepository(ctx, repoInfo, endpoint, nil, config.AuthConfig, "pull") - if lastError == nil { - break + repo, err := newRepository(ctx, repoInfo, endpoint, nil, config.AuthConfig, "pull") + if err != nil { + log.G(ctx).WithFields(log.Fields{"endpoint": endpoint.URL.String(), "error": err}).Info("endpoint") + lastError = err + continue } + repositories = append(repositories, repo) + if firstOnly { + return repositories, nil + } + } + if len(repositories) == 0 { + return nil, lastError } - return repository, lastError + return repositories, nil }