Return both https and http errors on ping fallback (#859)

* 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).

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, "; "))
 }

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)
+		}
 	}
 }