Skip to content

Commit

Permalink
Revert "Avoid trying https for insecure registries (#1002)" (#1048)
Browse files Browse the repository at this point in the history
This reverts commit 9e56ddd.
  • Loading branch information
jonjohnsonjr committed Jun 14, 2021
1 parent a27f4a4 commit 11f8769
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 41 deletions.
5 changes: 0 additions & 5 deletions pkg/name/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,6 @@ func (r Registry) Scheme() string {
return "https"
}

// IsInsecure returns if the registry is insecure.
func (r Registry) IsInsecure() bool {
return r.insecure
}

func checkRegistry(name string) error {
// Per RFC 3986, registries (authorities) are required to be prefixed with "//"
// url.Host == hostname[:port] == authority
Expand Down
21 changes: 6 additions & 15 deletions pkg/v1/remote/transport/ping.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,12 @@ func parseChallenge(suffix string) map[string]string {
func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingResp, error) {
client := http.Client{Transport: t}

// If it is intentionally set to insecure via name.NewInsecureRegistry, try to use
// "http" for the request at first, falling back to "https" if "http" is failed.
// Otherwise, this first attempts to use "https" for every request, falling back to http
// if the registry matches our localhost heuristic.
// e.g. if insecure is true, `schemes` is ["http", "https"]
// if registry is "localhost:8080" or "192.168.0.1:8080", `schemes` is ["https", "http"]
// if insecure is not true and registry does not "match our localhost heuristic", `schemes` is ["https"]
var schemes []string
if reg.IsInsecure() {
schemes = []string{"http", "https"}
} else {
schemes = []string{"https"}
if reg.Scheme() == "http" {
schemes = append(schemes, "http")
}
// This first attempts to use "https" for every request, falling back to http
// if the registry matches our localhost heuristic or if it is intentionally
// set to insecure via name.NewInsecureRegistry.
schemes := []string{"https"}
if reg.Scheme() == "http" {
schemes = append(schemes, "http")
}

var errs []string
Expand Down
28 changes: 7 additions & 21 deletions pkg/v1/remote/transport/ping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,27 +168,18 @@ func TestUnsupportedStatus(t *testing.T) {

func TestPingHttpFallback(t *testing.T) {
tests := []struct {
reg name.Registry
wantCount int
err string
contains []string
notContains []string
}{{ // try "https" only
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`,
}, { // try "https" at first, then fall back to "http"
}, {
reg: mustRegistry("ko.local"),
wantCount: 2,
}, { // try "http" at first, then fall back to "https"
reg: mustInsecureRegistry("ko.local"),
wantCount: 1,
}, { // try "https" only
reg: mustRegistry("us.gcr.io"),
wantCount: 0,
contains: []string{"https://us.gcr.io/v2/"},
notContains: []string{"http://us.gcr.io/v2/"},
}, { // try "http" at first, then fall back to "https"
}, {
reg: mustInsecureRegistry("us.gcr.io"),
wantCount: 0,
contains: []string{"https://us.gcr.io/v2/", "http://us.gcr.io/v2/"},
Expand Down Expand Up @@ -232,11 +223,6 @@ func TestPingHttpFallback(t *testing.T) {
t.Errorf("expected err to contain %q but did not: %q", c, err)
}
}
for _, c := range test.notContains {
if strings.Contains(err.Error(), c) {
t.Errorf("unexpected err to contain %q but did: %q", c, err)
}
}
} else if got, want := err.Error(), test.err; got != want {
t.Errorf("got %q want %q", got, want)
}
Expand Down

0 comments on commit 11f8769

Please sign in to comment.