Merge pull request containerd#10286 from dmcgowan/update-tls-fallback-default-ports

Allow fallback across default ports

Author: Maksym Pavlenko

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)

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",

core/remotes/docker/resolver.go

@@ -25,20 +25,22 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"os"
 	"path"
 	"strings"
 
+	"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"
 	"github.com/containerd/containerd/v2/core/remotes/docker/schema1" //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility.
 	remoteerrors "github.com/containerd/containerd/v2/core/remotes/errors"
 	"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 +734,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 +775,15 @@ func isTLSError(err error) bool {
 
 	return false
 }
+
+func isPortError(err error, host string) bool {
+	if isConnError(err) || 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
+}

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()

core/remotes/docker/resolver_unix.go

@@ -0,0 +1,28 @@
+//go:build !windows
+
+/*
+   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 docker
+
+import (
+	"errors"
+	"syscall"
+)
+
+func isConnError(err error) bool {
+	return errors.Is(err, syscall.ECONNREFUSED)
+}

core/remotes/docker/resolver_windows.go

@@ -0,0 +1,30 @@
+//go:build windows
+
+/*
+   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 docker
+
+import (
+	"errors"
+	"syscall"
+
+	"golang.org/x/sys/windows"
+)
+
+func isConnError(err error) bool {
+	return errors.Is(err, syscall.ECONNREFUSED) || errors.Is(err, windows.WSAECONNREFUSED)
+}