Skip to content

Commit

Permalink
Remove unused http fallback code
Browse files Browse the repository at this point in the history
  • Loading branch information
Anton-Kalpakchiev committed Nov 13, 2024
1 parent 3749318 commit 23637c8
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 96 deletions.
8 changes: 4 additions & 4 deletions lib/backend/registrybackend/blobclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down Expand Up @@ -56,7 +56,7 @@ func TestBlobDownloadBlobSuccess(t *testing.T) {
addr, stop := testutil.StartServer(r)
defer stop()

config := newTestConfig(addr)
config := Config{Address: addr}
client, err := NewBlobClient(config, tally.NoopScope)
require.NoError(err)

Expand Down Expand Up @@ -86,7 +86,7 @@ func TestBlobDownloadManifestSuccess(t *testing.T) {
addr, stop := testutil.StartServer(r)
defer stop()

config := newTestConfig(addr)
config := Config{Address: addr}
client, err := NewBlobClient(config, tally.NoopScope)
require.NoError(err)

Expand Down Expand Up @@ -116,7 +116,7 @@ func TestBlobDownloadFileNotFound(t *testing.T) {
addr, stop := testutil.StartServer(r)
defer stop()

config := newTestConfig(addr)
config := Config{Address: addr}
client, err := NewBlobClient(config, tally.NoopScope)
require.NoError(err)

Expand Down
26 changes: 0 additions & 26 deletions lib/backend/registrybackend/config_test.go

This file was deleted.

9 changes: 1 addition & 8 deletions lib/backend/registrybackend/security/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down Expand Up @@ -48,7 +48,6 @@ type Config struct {
TLS httputil.TLSConfig `yaml:"tls"`
BasicAuth *types.AuthConfig `yaml:"basic"`
RemoteCredentialsStore string `yaml:"credsStore"`
EnableHTTPFallback bool `yaml:"enableHTTPFallback"`
}

// Authenticator creates send options to authenticate requests to registry
Expand Down Expand Up @@ -97,12 +96,6 @@ func (a *authenticator) Authenticate(repo string) ([]httputil.SendOption, error)
return opts, nil
}

if config.EnableHTTPFallback {
opts = append(opts, httputil.EnableHTTPFallback())
} else {
opts = append(opts, httputil.DisableHTTPFallback())
}

if !a.shouldAuth() {
opts = append(opts, httputil.SendTLSTransport(a.roundTripper))
return opts, nil
Expand Down
6 changes: 3 additions & 3 deletions lib/backend/registrybackend/tagclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestTagDownloadSuccess(t *testing.T) {
addr, stop := testutil.StartServer(r)
defer stop()

config := newTestConfig(addr)
config := Config{Address: addr}
client, err := NewTagClient(config, tally.NoopScope)
require.NoError(err)

Expand Down Expand Up @@ -88,7 +88,7 @@ func TestTagDownloadFileNotFound(t *testing.T) {
addr, stop := testutil.StartServer(r)
defer stop()

config := newTestConfig(addr)
config := Config{Address: addr}
client, err := NewTagClient(config, tally.NoopScope)
require.NoError(err)

Expand Down
53 changes: 0 additions & 53 deletions utils/httputil/httputil.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,6 @@ type sendOptions struct {
// parts of the url. For example, url.Scheme can be changed from
// http to https.
url *url.URL

// This is not a valid http option. HTTP fallback is added to allow
// easier migration from http to https.
// In go1.11 and go1.12, the responses returned when http request is
// sent to https server are different in the fallback mode:
// go1.11 returns a network error whereas go1.12 returns BadRequest.
// This causes TestTLSClientBadAuth to fail because the test checks
// retry error.
// This flag is added to allow disabling http fallback in unit tests.
// NOTE: it does not impact how it runs in production.
httpFallbackDisabled bool
}

// SendOption allows overriding defaults for the Send function.
Expand Down Expand Up @@ -236,20 +225,6 @@ func SendRetry(options ...RetryOption) SendOption {
return func(o *sendOptions) { o.retry = retry }
}

// DisableHTTPFallback disables http fallback when https request fails.
func DisableHTTPFallback() SendOption {
return func(o *sendOptions) {
o.httpFallbackDisabled = true
}
}

// EnableHTTPFallback enables http fallback when https request fails.
func EnableHTTPFallback() SendOption {
return func(o *sendOptions) {
o.httpFallbackDisabled = false
}
}

// SendTLS sets the transport with TLS config for the HTTP client.
func SendTLS(config *tls.Config) SendOption {
return func(o *sendOptions) {
Expand Down Expand Up @@ -294,7 +269,6 @@ func Send(method, rawurl string, options ...SendOption) (*http.Response, error)
transport: nil, // Use HTTP default.
ctx: context.Background(),
url: u,
httpFallbackDisabled: true,
}
for _, o := range options {
o(opts)
Expand All @@ -314,21 +288,6 @@ func Send(method, rawurl string, options ...SendOption) (*http.Response, error)
var resp *http.Response
for {
resp, err = client.Do(req)
// Retry without tls. During migration there would be a time when the
// component receiving the tls request does not serve https response.
// TODO (@evelynl): disable retry after tls migration.
if err != nil && req.URL.Scheme == "https" && !opts.httpFallbackDisabled {
originalErr := err
resp, err = fallbackToHTTP(client, method, opts)
if err != nil {
// Sometimes the request fails for a reason unrelated to https.
// To keep this reason visible, we always include the original
// error.
err = fmt.Errorf(
"failed to fallback https to http, original https error: %s,\n"+
"fallback http error: %s", originalErr, err)
}
}
if err != nil ||
(isRetryable(resp.StatusCode) && !opts.acceptedCodes[resp.StatusCode]) ||
(opts.retry.extraCodes[resp.StatusCode]) {
Expand Down Expand Up @@ -456,18 +415,6 @@ func newRequest(method string, opts *sendOptions) (*http.Request, error) {
return req, nil
}

func fallbackToHTTP(
client *http.Client, method string, opts *sendOptions) (*http.Response, error) {

req, err := newRequest(method, opts)
if err != nil {
return nil, err
}
req.URL.Scheme = "http"

return client.Do(req)
}

func min(a, b time.Duration) time.Duration {
if a < b {
return a
Expand Down
4 changes: 2 additions & 2 deletions utils/httputil/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down Expand Up @@ -216,7 +216,7 @@ func TestTLSClientBadAuth(t *testing.T) {
badtls, err := badConfig.BuildClient()
require.NoError(err)

_, err = Get("https://"+addr+"/", SendTLS(badtls), DisableHTTPFallback())
_, err = Get("https://"+addr+"/", SendTLS(badtls))
require.True(IsNetworkError(err))
}

Expand Down

0 comments on commit 23637c8

Please sign in to comment.