diff --git a/go.mod b/go.mod index f17ea6765..8581a35ce 100644 --- a/go.mod +++ b/go.mod @@ -56,12 +56,11 @@ require ( cloud.google.com/go/pubsub v1.45.3 github.com/AdamKorcz/go-fuzz-headers-1 v0.0.0-20230919221257-8b5d3ce2d11d github.com/DATA-DOG/go-sqlmock v1.5.2 + github.com/avast/retry-go/v4 v4.6.0 github.com/cyberphone/json-canonicalization v0.0.0-20220623050100-57a0ce2678a7 github.com/go-redis/redismock/v9 v9.2.0 github.com/go-sql-driver/mysql v1.8.1 github.com/golang/mock v1.6.0 - github.com/hashicorp/go-cleanhttp v0.5.2 - github.com/hashicorp/go-retryablehttp v0.7.7 github.com/jmoiron/sqlx v1.4.0 github.com/redis/go-redis/v9 v9.7.0 github.com/sassoftware/relic/v7 v7.6.2 @@ -132,7 +131,9 @@ require ( github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 // indirect github.com/google/s2a-go v0.1.9 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect + github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect + github.com/hashicorp/go-retryablehttp v0.7.7 // indirect github.com/hashicorp/go-rootcerts v1.0.2 // indirect github.com/hashicorp/go-secure-stdlib/parseutil v0.1.7 // indirect github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect diff --git a/go.sum b/go.sum index 83d72a90a..5b369482b 100644 --- a/go.sum +++ b/go.sum @@ -64,6 +64,8 @@ github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPp github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= +github.com/avast/retry-go/v4 v4.6.0 h1:K9xNA+KeB8HHc2aWFuLb25Offp+0iVRXEvFx8IinRJA= +github.com/avast/retry-go/v4 v4.6.0/go.mod h1:gvWlPhBVsvBbLkVGDg/KwvBv0bEkCOLRRSHKIr2PyOE= github.com/aws/aws-sdk-go v1.55.5 h1:KKUZBfBoyqy5d3swXyiC7Q76ic40rYcbqH7qjh59kzU= github.com/aws/aws-sdk-go v1.55.5/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU= github.com/aws/aws-sdk-go-v2 v1.32.8 h1:cZV+NUS/eGxKXMtmyhtYPJ7Z4YLoI/V8bkTdRZfYhGo= diff --git a/pkg/client/options.go b/pkg/client/options.go index c1135a71c..55cb84b60 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -15,10 +15,16 @@ package client import ( + "crypto/tls" + "errors" + "fmt" "net/http" + "net/url" + "regexp" + "strings" "time" - "github.com/hashicorp/go-retryablehttp" + "github.com/avast/retry-go/v4" ) // Option is a functional option for customizing static signatures. @@ -30,7 +36,6 @@ type options struct { RetryWaitMin time.Duration RetryWaitMax time.Duration InsecureTLS bool - Logger interface{} NoDisableKeepalives bool Headers map[string][]string } @@ -81,14 +86,9 @@ func WithRetryWaitMax(t time.Duration) Option { } } -// WithLogger sets the logger; it must implement either retryablehttp.Logger or retryablehttp.LeveledLogger; if not, this will not take effect. -func WithLogger(logger interface{}) Option { - return func(o *options) { - switch logger.(type) { - case retryablehttp.Logger, retryablehttp.LeveledLogger: - o.Logger = logger - } - } +// WithLogger sets the logger; this method is deprecated and will not take any effect. +func WithLogger(_ interface{}) Option { + return func(*options) {} } // WithInsecureTLS disables TLS verification. @@ -113,33 +113,73 @@ func WithHeaders(h map[string][]string) Option { } type roundTripper struct { - http.RoundTripper - UserAgent string - Headers map[string][]string + inner http.RoundTripper + *options } // RoundTrip implements `http.RoundTripper` -func (rt *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - req.Header.Set("User-Agent", rt.UserAgent) - for k, v := range rt.Headers { +func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err error) { + req.Header.Set("User-Agent", rt.options.UserAgent) + for k, v := range rt.options.Headers { for _, h := range v { req.Header.Add(k, h) } } - return rt.RoundTripper.RoundTrip(req) + + err = retry.Do(func() (err error) { + res, err = rt.inner.RoundTrip(req) + return shouldRetry(res, err) + }, + retry.Attempts(rt.options.RetryCount), + retry.Delay(rt.options.RetryWaitMin), + retry.MaxDelay(rt.options.RetryWaitMax), + retry.DelayType(retry.BackOffDelay), + ) + + return res, err +} + +var tooManyRedirectyRe = regexp.MustCompile(`stopped after \d+ redirects\z`) + +func shouldRetry(resp *http.Response, err error) error { + if err != nil { + urlErr := &url.Error{} + + // Filter well known URL errors + if errors.As(err, &urlErr) { + certVerificationErr := &tls.CertificateVerificationError{} + + if tooManyRedirectyRe.MatchString(urlErr.Error()) || + strings.Contains(urlErr.Error(), "unsupported protocol scheme") || + strings.Contains(urlErr.Error(), "invalid header") || + strings.Contains(urlErr.Error(), "certificate is not trusted") || + errors.As(urlErr.Err, &certVerificationErr) { + return nil + } + } + + // Retry any other errror + return err + } + + if resp.StatusCode == http.StatusTooManyRequests { + return fmt.Errorf("retry %d: %s", resp.StatusCode, resp.Status) + } + + if resp.StatusCode == 0 || (resp.StatusCode >= 500 && + resp.StatusCode != http.StatusNotImplemented) { + return fmt.Errorf("retry unexpected HTTP status %d: %s", resp.StatusCode, resp.Status) + } + + return nil } -func createRoundTripper(inner http.RoundTripper, o *options) http.RoundTripper { +func wrapRoundTripper(inner http.RoundTripper, o *options) http.RoundTripper { if inner == nil { inner = http.DefaultTransport } - if o.UserAgent == "" && o.Headers == nil { - // There's nothing to do... - return inner - } return &roundTripper{ - RoundTripper: inner, - UserAgent: o.UserAgent, - Headers: o.Headers, + inner: inner, + options: o, } } diff --git a/pkg/client/options_test.go b/pkg/client/options_test.go index d212ea011..9be8ea877 100644 --- a/pkg/client/options_test.go +++ b/pkg/client/options_test.go @@ -15,17 +15,14 @@ package client import ( - "log" "net/http" - "os" "testing" "github.com/google/go-cmp/cmp" + "go.uber.org/zap" ) func TestMakeOptions(t *testing.T) { - customLogger := log.New(os.Stdout, "", log.LstdFlags) - tests := []struct { desc string @@ -42,10 +39,6 @@ func TestMakeOptions(t *testing.T) { desc: "WithRetryCount", opts: []Option{WithRetryCount(2)}, want: &options{UserAgent: "", RetryCount: 2}, - }, { - desc: "WithLogger", - opts: []Option{WithLogger(customLogger)}, - want: &options{UserAgent: "", RetryCount: DefaultRetryCount, Logger: customLogger}, }, { desc: "WithLoggerNil", opts: []Option{WithLogger(nil)}, @@ -62,7 +55,9 @@ func TestMakeOptions(t *testing.T) { for _, tc := range tests { t.Run(tc.desc, func(t *testing.T) { got := makeOptions(tc.opts...) - if d := cmp.Diff(tc.want, got, cmp.Comparer(func(a, b *log.Logger) bool { return a == b })); d != "" { + if d := cmp.Diff(tc.want, got, cmp.Comparer(func(a, b zap.SugaredLogger) bool { + return a == b + })); d != "" { t.Errorf("makeOptions(%v) returned unexpected result (-want +got): %s", tc.desc, d) } }) @@ -82,11 +77,11 @@ func (m *mockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) return m.resp, m.err } -func TestCreateRoundTripper(t *testing.T) { +func TestWrapRoundTripper(t *testing.T) { t.Run("always returns non-nil", func(t *testing.T) { - got := createRoundTripper(nil, &options{}) + got := wrapRoundTripper(nil, &options{}) if got == nil { - t.Errorf("createRoundTripper() should never return a nil `http.RoundTripper`") + t.Errorf("wrapRoundTripper() should never return a nil `http.RoundTripper`") } }) @@ -104,7 +99,7 @@ func TestCreateRoundTripper(t *testing.T) { expectedUserAgent := "test UserAgent" m := &mockRoundTripper{} - rt := createRoundTripper(m, &options{ + rt := wrapRoundTripper(m, &options{ UserAgent: expectedUserAgent, }) m.resp = testResp diff --git a/pkg/client/rekor_client.go b/pkg/client/rekor_client.go index 470ca5eaa..52b18b0dd 100644 --- a/pkg/client/rekor_client.go +++ b/pkg/client/rekor_client.go @@ -16,15 +16,15 @@ package client import ( "crypto/tls" + "net" "net/http" "net/url" + "time" "github.com/go-openapi/runtime" httptransport "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" - "github.com/hashicorp/go-cleanhttp" - retryablehttp "github.com/hashicorp/go-retryablehttp" "github.com/sigstore/rekor/pkg/generated/client" "github.com/sigstore/rekor/pkg/util" ) @@ -36,25 +36,28 @@ func GetRekorClient(rekorServerURL string, opts ...Option) (*client.Rekor, error } o := makeOptions(opts...) - retryableClient := retryablehttp.NewClient() - defaultTransport := cleanhttp.DefaultTransport() - if o.NoDisableKeepalives { - defaultTransport.DisableKeepAlives = false + transport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + DualStack: true, + }).DialContext, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + ForceAttemptHTTP2: true, + MaxIdleConnsPerHost: -1, + DisableKeepAlives: !o.NoDisableKeepalives, } if o.InsecureTLS { - /* #nosec G402 */ - defaultTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: o.InsecureTLS} //nolint:gosec } - retryableClient.HTTPClient = &http.Client{ - Transport: defaultTransport, - } - retryableClient.RetryMax = int(o.RetryCount) - retryableClient.RetryWaitMin = o.RetryWaitMin - retryableClient.RetryWaitMax = o.RetryWaitMax - retryableClient.Logger = o.Logger - httpClient := retryableClient.StandardClient() - httpClient.Transport = createRoundTripper(httpClient.Transport, o) + httpClient := &http.Client{ + Transport: wrapRoundTripper(transport, o), + } // sanitize path if url.Path == "" {