From 1a4a5a95fa3d82645275d5706878fee6f74abd57 Mon Sep 17 00:00:00 2001 From: apostasie Date: Wed, 4 Sep 2024 12:29:09 -0700 Subject: [PATCH] Hostsstore resolution cleanup As described in https://github.com/containerd/nerdctl/issues/3047, hosts.toml file lookup will treat https://foo:443 and https://foo differently, possibly leading to divergent behaviors *for the same registry*. This PR makes it so that a registry URL using the default https port (443) will ALSO lookup files stored in a "portless" directory. Finally, as dockerconfigresolver.go will soon go under significant changes, the hosts.toml resolution functions have been also isolated in their separate, own file. Signed-off-by: apostasie --- .../dockerconfigresolver.go | 37 ++++------- .../dockerconfigresolver/hostsstore.go | 66 +++++++++++++++++++ 2 files changed, 79 insertions(+), 24 deletions(-) create mode 100644 pkg/imgutil/dockerconfigresolver/hostsstore.go diff --git a/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go b/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go index 47a7be13c29..5262dd729d0 100644 --- a/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go +++ b/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go @@ -20,7 +20,6 @@ import ( "context" "crypto/tls" "errors" - "os" "github.com/containerd/containerd/v2/core/remotes" "github.com/containerd/containerd/v2/core/remotes/docker" @@ -57,24 +56,9 @@ func WithSkipVerifyCerts(b bool) Opt { // WithHostsDirs specifies directories like /etc/containerd/certs.d and /etc/docker/certs.d func WithHostsDirs(orig []string) Opt { - var ss []string - if len(orig) == 0 { - log.L.Debug("no hosts dir was specified") - } - for _, v := range orig { - if _, err := os.Stat(v); err == nil { - log.L.Debugf("Found hosts dir %q", v) - ss = append(ss, v) - } else { - if errors.Is(err, os.ErrNotExist) { - log.L.WithError(err).Debugf("Ignoring hosts dir %q", v) - } else { - log.L.WithError(err).Warnf("Ignoring hosts dir %q", v) - } - } - } + validDirs := validateDirectories(orig) return func(o *opts) { - o.hostsDirs = ss + o.hostsDirs = validDirs } } @@ -96,14 +80,19 @@ func NewHostOptions(ctx context.Context, refHostname string, optFuncs ...Opt) (* } var ho dockerconfig.HostOptions - ho.HostDir = func(s string) (string, error) { - for _, hostsDir := range o.hostsDirs { - found, err := dockerconfig.HostDirFromRoot(hostsDir)(s) - if (err != nil && !errdefs.IsNotFound(err)) || (found != "") { - return found, err + ho.HostDir = func(hostURL string) (string, error) { + regURL, err := Parse(hostURL) + if err != nil { + return "", err + } + dir, err := hostDirsFromRoot(regURL, o.hostsDirs) + if err != nil { + if errors.Is(err, errdefs.ErrNotFound) { + err = nil } + return "", err } - return "", nil + return dir, nil } if o.authCreds != nil { diff --git a/pkg/imgutil/dockerconfigresolver/hostsstore.go b/pkg/imgutil/dockerconfigresolver/hostsstore.go new file mode 100644 index 00000000000..0273a9cd7a6 --- /dev/null +++ b/pkg/imgutil/dockerconfigresolver/hostsstore.go @@ -0,0 +1,66 @@ +/* + 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 dockerconfigresolver + +import ( + "errors" + "os" + + "github.com/containerd/containerd/v2/core/remotes/docker/config" + "github.com/containerd/errdefs" + "github.com/containerd/log" +) + +// validateDirectories inspect a slice of strings and returns the ones that are valid readable directories +func validateDirectories(orig []string) []string { + ss := []string{} + for _, v := range orig { + fi, err := os.Stat(v) + if err != nil || !fi.IsDir() { + if !errors.Is(err, os.ErrNotExist) { + log.L.WithError(err).Warnf("Ignoring hosts location %q", v) + } + continue + } + ss = append(ss, v) + } + return ss +} + +// hostDirsFromRoot will retrieve a host.toml file for the namespace host, possibly trying without port +// if the requested port is standard. +// https://github.com/containerd/nerdctl/issues/3047 +func hostDirsFromRoot(registryURL *RegistryURL, dirs []string) (string, error) { + hostsDirs := validateDirectories(dirs) + + // Go through the configured system location to consider for hosts.toml files + for _, hostsDir := range hostsDirs { + found, err := config.HostDirFromRoot(hostsDir)(registryURL.Host) + // If we errored with anything but NotFound, or if we found one, return now + if (err != nil && !errdefs.IsNotFound(err)) || (found != "") { + return found, err + } + // If not found, and the port is standard, try again without the port + if registryURL.Port() == standardHTTPSPort { + found, err = config.HostDirFromRoot(hostsDir)(registryURL.Hostname()) + if (err != nil && !errors.Is(err, errdefs.ErrNotFound)) || (found != "") { + return found, err + } + } + } + return "", nil +}