enable configurable client retries with backoff in RekorClient (#1096)

* enable retries with backoff in RekorClient

Signed-off-by: Bob Callaway <bcallaway@google.com>

* add ability to explicitly disable logger

Signed-off-by: Bob Callaway <bcallaway@google.com>

* pass diff logger types, ensure User-Agent is still set

Signed-off-by: Bob Callaway <bcallaway@google.com>

Signed-off-by: Bob Callaway <bcallaway@google.com>

go.mod

@@ -53,9 +53,15 @@ require (
 	sigs.k8s.io/release-utils v0.7.3
 )
 
-require golang.org/x/exp v0.0.0-20220823124025-807a23277127
+require (
+	github.com/hashicorp/go-retryablehttp v0.7.1
+	golang.org/x/exp v0.0.0-20220823124025-807a23277127
+)
 
-require filippo.io/edwards25519 v1.0.0-rc.1 // indirect
+require (
+	filippo.io/edwards25519 v1.0.0-rc.1 // indirect
+	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
+)
 
 require (
 	cloud.google.com/go v0.103.0 // indirect

go.sum

@@ -508,12 +508,16 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb
 github.com/hanwen/go-fuse v1.0.0/go.mod h1:unqXarDXqzAk0rt98O2tVndEPIpUgLD9+rwFisZH3Ok=
 github.com/hanwen/go-fuse/v2 v2.1.0/go.mod h1:oRyA5eK+pvJyv5otpO/DgccS8y/RvYMaO00GgRLGryc=
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
+github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
+github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
+github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
 github.com/hashicorp/go-hclog v1.2.1 h1:YQsLlGDJgwhXFpucSPyVbCBviQtjlHv3jLTlp8YmtEw=
 github.com/hashicorp/go-immutable-radix v1.3.1 h1:DKHmCUm2hRBK510BaiZlwvpD40f8bJFeZnpfm2KLowc=
 github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
 github.com/hashicorp/go-plugin v1.4.4 h1:NVdrSdFRt3SkZtNckJ6tog7gbpRrcbOjQi/rgF7JYWQ=
 github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ=
+github.com/hashicorp/go-retryablehttp v0.7.1/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
 github.com/hashicorp/go-rootcerts v1.0.2 h1:jzhAVGtqPKbwpyCPELlgNWhE1znq+qwJtW5Oi2viEzc=
 github.com/hashicorp/go-secure-stdlib/mlock v0.1.2 h1:p4AKXPPS24tO8Wc8i1gLvSKdmkiSY5xuju57czJ/IJQ=
 github.com/hashicorp/go-secure-stdlib/parseutil v0.1.6 h1:om4Al8Oy7kCm/B86rLCLah4Dt5Aa0Fr5rYBG60OzwHQ=

pkg/client/options.go

@@ -14,18 +14,30 @@
 
 package client
 
-import "net/http"
+import (
+	"net/http"
+
+	"github.com/hashicorp/go-retryablehttp"
+)
 
 // Option is a functional option for customizing static signatures.
 type Option func(*options)
 
 type options struct {
-	UserAgent string
+	UserAgent  string
+	RetryCount uint
+	Logger     interface{}
 }
 
+const (
+	// DefaultRetryCount is the default number of retries.
+	DefaultRetryCount = 3
+)
+
 func makeOptions(opts ...Option) *options {
 	o := &options{
-		UserAgent: "",
+		UserAgent:  "",
+		RetryCount: DefaultRetryCount,
 	}
 
 	for _, opt := range opts {
@@ -42,6 +54,23 @@ func WithUserAgent(userAgent string) Option {
 	}
 }
 
+// WithRetryCount sets the number of retries.
+func WithRetryCount(retryCount uint) Option {
+	return func(o *options) {
+		o.RetryCount = retryCount
+	}
+}
+
+// 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
+		}
+	}
+}
+
 type roundTripper struct {
 	http.RoundTripper
 	UserAgent string

pkg/client/options_test.go

@@ -15,31 +15,47 @@
 package client
 
 import (
+	"log"
 	"net/http"
+	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 )
 
 func TestMakeOptions(t *testing.T) {
+	customLogger := log.New(os.Stdout, "", log.LstdFlags)
+
 	tests := []struct {
 		desc string
 
 		opts []Option
 		want *options
 	}{{
 		desc: "no opts",
-		want: &options{},
+		want: &options{RetryCount: DefaultRetryCount},
 	}, {
 		desc: "WithUserAgent",
 		opts: []Option{WithUserAgent("test user agent")},
-		want: &options{UserAgent: "test user agent"},
+		want: &options{UserAgent: "test user agent", RetryCount: DefaultRetryCount},
+	}, {
+		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)},
+		want: &options{UserAgent: "", RetryCount: DefaultRetryCount},
 	}}
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
 			got := makeOptions(tc.opts...)
-			if d := cmp.Diff(tc.want, got); d != "" {
-				t.Errorf("makeOptions() returned unexpected result (-want +got): %s", d)
+			if d := cmp.Diff(tc.want, got, cmp.Comparer(func(a, b *log.Logger) bool { return a == b })); d != "" {
+				t.Errorf("makeOptions(%v) returned unexpected result (-want +got): %s", tc.desc, d)
 			}
 		})
 	}

pkg/client/rekor_client.go

@@ -20,6 +20,7 @@ import (
 	"github.com/go-openapi/runtime"
 	httptransport "github.com/go-openapi/runtime/client"
 	"github.com/go-openapi/strfmt"
+	retryablehttp "github.com/hashicorp/go-retryablehttp"
 	"github.com/sigstore/rekor/pkg/generated/client"
 	"github.com/sigstore/rekor/pkg/util"
 	"github.com/spf13/viper"
@@ -32,7 +33,14 @@ func GetRekorClient(rekorServerURL string, opts ...Option) (*client.Rekor, error
 	}
 	o := makeOptions(opts...)
 
-	rt := httptransport.New(url.Host, client.DefaultBasePath, []string{url.Scheme})
+	retryableClient := retryablehttp.NewClient()
+	retryableClient.RetryMax = int(o.RetryCount)
+	retryableClient.Logger = o.Logger
+
+	httpClient := retryableClient.StandardClient()
+	httpClient.Transport = createRoundTripper(httpClient.Transport, o)
+
+	rt := httptransport.NewWithClient(url.Host, client.DefaultBasePath, []string{url.Scheme}, httpClient)
 	rt.Consumers["application/json"] = runtime.JSONConsumer()
 	rt.Consumers["application/x-pem-file"] = runtime.TextConsumer()
 	rt.Consumers["application/pem-certificate-chain"] = runtime.TextConsumer()
@@ -44,8 +52,6 @@ func GetRekorClient(rekorServerURL string, opts ...Option) (*client.Rekor, error
 		rt.DefaultAuthentication = httptransport.APIKeyAuth("apiKey", "query", viper.GetString("api-key"))
 	}
 
-	rt.Transport = createRoundTripper(rt.Transport, o)
-
 	registry := strfmt.Default
 	registry.Add("signedCheckpoint", &util.SignedNote{}, util.SignedCheckpointValidator)
 	return client.New(rt, registry), nil

pkg/client/rekor_client_test.go

@@ -78,7 +78,7 @@ func TestAPIKey(t *testing.T) {
 
 }
 
-func TestGetRekorClientWithOptions(t *testing.T) {
+func TestGetRekorClientWithUserAgent(t *testing.T) {
 	t.Parallel()
 	expectedUserAgent := "test User-Agent"
 	requestReceived := false
@@ -105,3 +105,31 @@ func TestGetRekorClientWithOptions(t *testing.T) {
 		t.Fatal("no requests were received")
 	}
 }
+
+func TestGetRekorClientWithRetryCount(t *testing.T) {
+	t.Parallel()
+	expectedCount := 2
+	actualCount := 0
+	testServer := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			actualCount++
+			file := []byte{}
+
+			if actualCount < expectedCount {
+				w.WriteHeader(http.StatusInternalServerError)
+			} else {
+				w.WriteHeader(http.StatusOK)
+				_, _ = w.Write(file)
+			}
+		}))
+	defer testServer.Close()
+
+	client, err := GetRekorClient(testServer.URL, WithRetryCount(2))
+	if err != nil {
+		t.Error(err)
+	}
+	_, err = client.Tlog.GetLogInfo(nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}