Repository: google/go-containerregistry
Pull request #859 (2020-12-03 18:08:23)

Return both https and http errors on ping fallback
Currently, we just return the last one, which is very misleading if
you're using self-signed certs for a domain that allows http fallback
(e.g. localhost).

Fixes https://github.com/google/go-containerregistry/issues/738


Patch:
diff --git a/.goreleaser.yml b/.goreleaser.yml
index d8aea2f7..1538ac63 100644
--- a/.goreleaser.yml
+++ b/.goreleaser.yml
@@ -16,6 +16,7 @@ builds:
   - -trimpath
   ldflags:
   - "-s -w -X github.com/google/go-containerregistry/cmd/crane/cmd.Version={{.Version}}"
+  - "-s -w -X github.com/google/go-containerregistry/pkg/v1/remote/transport.Version={{.Version}}"
   goarch:
     - amd64
     - arm
@@ -38,6 +39,7 @@ builds:
   - -trimpath
   ldflags:
   - "-s -w -X github.com/google/go-containerregistry/cmd/crane/cmd.Version={{.Version}}"
+  - "-s -w -X github.com/google/go-containerregistry/pkg/v1/remote/transport.Version={{.Version}}"
   goarch:
     - amd64
     - arm
diff --git a/pkg/v1/remote/transport/ping.go b/pkg/v1/remote/transport/ping.go
index 6ed1d464..396d4e03 100644
--- a/pkg/v1/remote/transport/ping.go
+++ b/pkg/v1/remote/transport/ping.go
@@ -16,6 +16,7 @@ package transport
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -78,7 +79,7 @@ func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingRes
 		schemes = append(schemes, "http")
 	}
 
-	var connErr error
+	var errs []string
 	for _, scheme := range schemes {
 		url := fmt.Sprintf("%s://%s/v2/", scheme, reg.Name())
 		req, err := http.NewRequest(http.MethodGet, url, nil)
@@ -87,7 +88,7 @@ func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingRes
 		}
 		resp, err := client.Do(req.WithContext(ctx))
 		if err != nil {
-			connErr = err
+			errs = append(errs, err.Error())
 			// Potentially retry with http.
 			continue
 		}
@@ -124,5 +125,5 @@ func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingRes
 			return nil, CheckError(resp, http.StatusOK, http.StatusUnauthorized)
 		}
 	}
-	return nil, connErr
+	return nil, errors.New(strings.Join(errs, "; "))
 }
diff --git a/pkg/v1/remote/transport/useragent.go b/pkg/v1/remote/transport/useragent.go
index 5d709099..74a9e71b 100644
--- a/pkg/v1/remote/transport/useragent.go
+++ b/pkg/v1/remote/transport/useragent.go
@@ -20,13 +20,19 @@ import (
 	"runtime/debug"
 )
 
+var (
+	// Version can be set via:
+	// -ldflags="-X 'github.com/google/go-containerregistry/pkg/v1/remote/transport.Version=$TAG'"
+	Version string
+
+	ggcrVersion = defaultUserAgent
+)
+
 const (
 	defaultUserAgent = "go-containerregistry"
 	moduleName       = "github.com/google/go-containerregistry"
 )
 
-var ggcrVersion = defaultUserAgent
-
 type userAgentTransport struct {
 	inner http.RoundTripper
 	ua    string
@@ -39,6 +45,11 @@ func init() {
 }
 
 func version() string {
+	if Version != "" {
+		// Version was set via ldflags, just return it.
+		return Version
+	}
+
 	info, ok := debug.ReadBuildInfo()
 	if !ok {
 		return ""


Test patch:
diff --git a/pkg/v1/remote/transport/ping_test.go b/pkg/v1/remote/transport/ping_test.go
index 6131cf28..aa3a42f3 100644
--- a/pkg/v1/remote/transport/ping_test.go
+++ b/pkg/v1/remote/transport/ping_test.go
@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -169,22 +170,26 @@ func TestPingHttpFallback(t *testing.T) {
 	tests := []struct {
 		reg       name.Registry
 		wantCount int
+		err       string
+		contains  []string
 	}{{
 		reg:       mustRegistry("gcr.io"),
 		wantCount: 1,
+		err:       `Get "https://gcr.io/v2/": http: server gave HTTP response to HTTPS client`,
 	}, {
 		reg:       mustRegistry("ko.local"),
 		wantCount: 2,
 	}, {
 		reg:       mustInsecureRegistry("us.gcr.io"),
-		wantCount: 2,
+		wantCount: 0,
+		contains:  []string{"https://us.gcr.io/v2/", "http://us.gcr.io/v2/"},
 	}}
 
 	gotCount := 0
 	server := httptest.NewServer(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			gotCount++
-			http.Error(w, "Forbidden", http.StatusForbidden)
+			w.WriteHeader(http.StatusOK)
 		}))
 	defer server.Close()
 
@@ -195,15 +200,32 @@ func TestPingHttpFallback(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		pr, err := ping(context.Background(), test.reg, tprt)
-		if err == nil {
-			t.Errorf("ping() = %v", pr)
+		// This is the last one, fatal error it.
+		if strings.Contains(test.reg.String(), "us.gcr.io") {
+			server.Close()
 		}
 
-		if test.wantCount != gotCount {
-			t.Errorf("%s: got %d requests, wanted %d", test.reg.String(), test.wantCount, gotCount)
+		_, err := ping(context.Background(), test.reg, tprt)
+		if got, want := gotCount, test.wantCount; got != want {
+			t.Errorf("%s: got %d requests, wanted %d", test.reg.String(), got, want)
 		}
 		gotCount = 0
+
+		if err == nil {
+			if test.err != "" {
+				t.Error("expected err, got nil")
+			}
+			continue
+		}
+		if len(test.contains) != 0 {
+			for _, c := range test.contains {
+				if !strings.Contains(err.Error(), c) {
+					t.Errorf("expected err to contain %q but did not: %q", c, err)
+				}
+			}
+		} else if got, want := err.Error(), test.err; got != want {
+			t.Errorf("got %q want %q", got, want)
+		}
 	}
 }