From 23637c89970122603fdf305ec1475aeec1c956a1 Mon Sep 17 00:00:00 2001 From: Anton Kalpakchiev Date: Wed, 13 Nov 2024 20:20:57 +0100 Subject: [PATCH] Remove unused http fallback code --- .../registrybackend/blobclient_test.go | 8 +-- lib/backend/registrybackend/config_test.go | 26 --------- .../registrybackend/security/security.go | 9 +--- lib/backend/registrybackend/tagclient_test.go | 6 +-- utils/httputil/httputil.go | 53 ------------------- utils/httputil/tls_test.go | 4 +- 6 files changed, 10 insertions(+), 96 deletions(-) delete mode 100644 lib/backend/registrybackend/config_test.go diff --git a/lib/backend/registrybackend/blobclient_test.go b/lib/backend/registrybackend/blobclient_test.go index 68d6cc149..ca681a2c4 100644 --- a/lib/backend/registrybackend/blobclient_test.go +++ b/lib/backend/registrybackend/blobclient_test.go @@ -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, @@ -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) @@ -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) @@ -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) diff --git a/lib/backend/registrybackend/config_test.go b/lib/backend/registrybackend/config_test.go deleted file mode 100644 index 897c015fe..000000000 --- a/lib/backend/registrybackend/config_test.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2016-2019 Uber Technologies, Inc. -// -// 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 registrybackend - -import "github.com/uber/kraken/lib/backend/registrybackend/security" - -func newTestConfig(addr string) Config { - return Config{ - Address: addr, - Security: security.Config{ - EnableHTTPFallback: true, - }, - } -} diff --git a/lib/backend/registrybackend/security/security.go b/lib/backend/registrybackend/security/security.go index b53c7722a..aeef3a80e 100644 --- a/lib/backend/registrybackend/security/security.go +++ b/lib/backend/registrybackend/security/security.go @@ -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, @@ -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 @@ -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 diff --git a/lib/backend/registrybackend/tagclient_test.go b/lib/backend/registrybackend/tagclient_test.go index 01fd7b6f4..1094ada90 100644 --- a/lib/backend/registrybackend/tagclient_test.go +++ b/lib/backend/registrybackend/tagclient_test.go @@ -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, @@ -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) @@ -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) diff --git a/utils/httputil/httputil.go b/utils/httputil/httputil.go index e83ce3dde..b9bb6d9b9 100644 --- a/utils/httputil/httputil.go +++ b/utils/httputil/httputil.go @@ -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. @@ -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) { @@ -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) @@ -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]) { @@ -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 diff --git a/utils/httputil/tls_test.go b/utils/httputil/tls_test.go index 566462552..7542c5425 100644 --- a/utils/httputil/tls_test.go +++ b/utils/httputil/tls_test.go @@ -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, @@ -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)) }