diff --git a/core/remotes/docker/config/hosts.go b/core/remotes/docker/config/hosts.go index a9b8691fe..86ea23238 100644 --- a/core/remotes/docker/config/hosts.go +++ b/core/remotes/docker/config/hosts.go @@ -30,11 +30,12 @@ import ( "strings" "time" - "github.com/containerd/containerd/v2/core/remotes/docker" "github.com/containerd/errdefs" "github.com/containerd/log" "github.com/pelletier/go-toml/v2" tomlu "github.com/pelletier/go-toml/v2/unstable" + + "github.com/containerd/containerd/v2/core/remotes/docker" ) // UpdateClientFunc is a function that lets you to amend http Client behavior used by registry clients. @@ -250,7 +251,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos // the request twice or consuming the request body. if host.scheme == "http" && explicitTLS { _, port, _ := net.SplitHostPort(host.host) - if port != "" && port != "80" { + if port != "80" { log.G(ctx).WithField("host", host.host).Info("host will try HTTPS first since it is configured for HTTP with a TLS configuration, consider changing host to HTTPS or removing unused TLS configuration") host.scheme = "https" rhosts[i].Client.Transport = docker.NewHTTPFallback(rhosts[i].Client.Transport) diff --git a/core/remotes/docker/config/hosts_test.go b/core/remotes/docker/config/hosts_test.go index 0ca629865..96327583f 100644 --- a/core/remotes/docker/config/hosts_test.go +++ b/core/remotes/docker/config/hosts_test.go @@ -26,8 +26,9 @@ import ( "path/filepath" "testing" - "github.com/containerd/containerd/v2/core/remotes/docker" "github.com/containerd/log/logtest" + + "github.com/containerd/containerd/v2/core/remotes/docker" ) const allCaps = docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush @@ -361,8 +362,8 @@ func TestHTTPFallback(t *testing.T) { InsecureSkipVerify: true, }, }, - expectedScheme: "http", - usesFallback: false, + expectedScheme: "https", + usesFallback: true, }, { host: "localhost", @@ -402,8 +403,8 @@ func TestHTTPFallback(t *testing.T) { InsecureSkipVerify: true, }, }, - expectedScheme: "http", - usesFallback: false, + expectedScheme: "https", + usesFallback: true, }, { host: "example.com:5000", diff --git a/core/remotes/docker/resolver.go b/core/remotes/docker/resolver.go index 96b87628d..873361dd1 100644 --- a/core/remotes/docker/resolver.go +++ b/core/remotes/docker/resolver.go @@ -25,8 +25,15 @@ import ( "net" "net/http" "net/url" + "os" "path" "strings" + "syscall" + + "github.com/containerd/errdefs" + "github.com/containerd/log" + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/containerd/containerd/v2/core/images" "github.com/containerd/containerd/v2/core/remotes" @@ -35,10 +42,6 @@ import ( "github.com/containerd/containerd/v2/pkg/reference" "github.com/containerd/containerd/v2/pkg/tracing" "github.com/containerd/containerd/v2/version" - "github.com/containerd/errdefs" - "github.com/containerd/log" - "github.com/opencontainers/go-digest" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) var ( @@ -732,7 +735,7 @@ func (f *httpFallback) RoundTrip(r *http.Request) (*http.Response, error) { // only fall back if the same host had previously fell back if f.host != r.URL.Host { resp, err := f.super.RoundTrip(r) - if !isTLSError(err) { + if !isTLSError(err) && !isPortError(err, r.URL.Host) { return resp, err } } @@ -773,3 +776,15 @@ func isTLSError(err error) bool { return false } + +func isPortError(err error, host string) bool { + if errors.Is(err, syscall.ECONNREFUSED) || os.IsTimeout(err) { + if _, port, _ := net.SplitHostPort(host); port != "" { + // Port is specified, will not retry on different port with scheme change + return false + } + return true + } + + return false +} diff --git a/core/remotes/docker/resolver_test.go b/core/remotes/docker/resolver_test.go index e2769e160..916c2cccc 100644 --- a/core/remotes/docker/resolver_test.go +++ b/core/remotes/docker/resolver_test.go @@ -33,13 +33,14 @@ import ( "testing" "time" - "github.com/containerd/containerd/v2/core/remotes" - "github.com/containerd/containerd/v2/core/remotes/docker/auth" - remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors" "github.com/containerd/errdefs" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + + "github.com/containerd/containerd/v2/core/remotes" + "github.com/containerd/containerd/v2/core/remotes/docker/auth" + remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors" ) func TestHTTPResolver(t *testing.T) { @@ -481,6 +482,34 @@ func TestHTTPFallbackTimeoutResolver(t *testing.T) { runBasicTest(t, "testname", sf) } +func TestHTTPFallbackPortError(t *testing.T) { + // This test only checks the isPortError since testing the whole http fallback would + // require listening on 80 and making sure nothing is listening on 443. + + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + host := l.Addr().String() + err = l.Close() + if err != nil { + t.Fatal(err) + } + + _, err = net.Dial("tcp", host) + if err == nil { + t.Fatal("Dial should fail after close") + } + + if isPortError(err, host) { + t.Fatalf("Expected no port error for %s with %v", host, err) + } + if !isPortError(err, "127.0.0.1") { + t.Fatalf("Expected port error for 127.0.0.1 with %v", err) + } + +} + func TestResolveProxy(t *testing.T) { var ( ctx = context.Background()