From 8fe17e2c5ced0ce477283464a1633511d60b5d7e Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 13 Sep 2023 12:54:36 -0400 Subject: [PATCH 1/4] Populate default retry configuration options --- client_configuration.go | 2 +- client_option.go | 24 ++++++++++++++++++++---- client_test.go | 13 ++++++++++++- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/client_configuration.go b/client_configuration.go index 23f50633..873da2b1 100644 --- a/client_configuration.go +++ b/client_configuration.go @@ -166,7 +166,7 @@ type RetryConfiguration struct { // Default: 1000 milliseconds RetryWaitMin time.Duration `env:"VAULT_RETRY_WAIT_MIN"` - // MaxRetryWait controls the maximum time to wait before retrying when + // RetryWaitMax controls the maximum time to wait before retrying when // a 5xx or 412 error occurs. // Default: 1500 milliseconds RetryWaitMax time.Duration `env:"VAULT_RETRY_WAIT_MAX"` diff --git a/client_option.go b/client_option.go index df42cb16..cd322c9a 100644 --- a/client_option.go +++ b/client_option.go @@ -54,18 +54,34 @@ func WithRequestTimeout(timeout time.Duration) ClientOption { } // WithTLS configures the TLS settings in the base http.Client. -func WithTLS(configuration TLSConfiguration) ClientOption { +func WithTLS(tls TLSConfiguration) ClientOption { return func(c *ClientConfiguration) error { - c.TLS = configuration + c.TLS = tls return nil } } // WithRetryConfiguration configures the internal go-retryablehttp client. // The library sets reasonable defaults for this setting. -func WithRetryConfiguration(configuration RetryConfiguration) ClientOption { +func WithRetryConfiguration(retry RetryConfiguration) ClientOption { return func(c *ClientConfiguration) error { - c.RetryConfiguration = configuration + // if any of the required RetryConfiguration values are missing, use defaults + defaultRetryConfiguration := DefaultConfiguration().RetryConfiguration + + if retry.CheckRetry == nil { + retry.CheckRetry = defaultRetryConfiguration.CheckRetry + } + + if retry.Backoff == nil { + retry.Backoff = defaultRetryConfiguration.Backoff + } + + if retry.ErrorHandler == nil { + retry.ErrorHandler = defaultRetryConfiguration.ErrorHandler + } + + c.RetryConfiguration = retry + return nil } } diff --git a/client_test.go b/client_test.go index 9cc6909e..82c81d60 100644 --- a/client_test.go +++ b/client_test.go @@ -4,6 +4,7 @@ package vault import ( + "fmt" "testing" "time" @@ -29,7 +30,7 @@ func Test_Client_Clone(t *testing.T) { clone := client.Clone() - assert.Equal(t, client, clone) + testEqual(t, client, clone) assert.Equal(t, "http://test", clone.Configuration().Address) assert.Equal(t, 30*time.Second, clone.Configuration().RequestTimeout) @@ -38,3 +39,13 @@ func Test_Client_Clone(t *testing.T) { assert.Equal(t, "test-token", clone.clientRequestModifiers.headers.token) assert.Equal(t, "test-namespace", clone.clientRequestModifiers.headers.namespace) } + +// We cannot compare the two objects directly since they have nested func pointers +func testEqual(t *testing.T, c1 *Client, c2 *Client) { + assert.Equal(t, fmt.Sprintf("%v", c1.configuration), fmt.Sprintf("%v", c2.configuration)) + assert.Equal(t, fmt.Sprintf("%v", c1.parsedBaseAddress), fmt.Sprintf("%v", c2.parsedBaseAddress)) + assert.Equal(t, fmt.Sprintf("%v", c1.client), fmt.Sprintf("%v", c2.client)) + assert.Equal(t, fmt.Sprintf("%v", c1.clientWithRetries), fmt.Sprintf("%v", c2.clientWithRetries)) + assert.Equal(t, fmt.Sprintf("%v", c1.clientRequestModifiers), fmt.Sprintf("%v", c2.clientRequestModifiers)) + assert.Equal(t, fmt.Sprintf("%v", c1.replicationStates.states), fmt.Sprintf("%v", c2.replicationStates.states)) +} From 0db4e340d04fe63c53b9b490528f841d98a9511e Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 13 Sep 2023 12:57:40 -0400 Subject: [PATCH 2/4] assertEqual --- client_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client_test.go b/client_test.go index 82c81d60..9d131962 100644 --- a/client_test.go +++ b/client_test.go @@ -30,7 +30,7 @@ func Test_Client_Clone(t *testing.T) { clone := client.Clone() - testEqual(t, client, clone) + assertEqual(t, client, clone) assert.Equal(t, "http://test", clone.Configuration().Address) assert.Equal(t, 30*time.Second, clone.Configuration().RequestTimeout) @@ -40,8 +40,8 @@ func Test_Client_Clone(t *testing.T) { assert.Equal(t, "test-namespace", clone.clientRequestModifiers.headers.namespace) } -// We cannot compare the two objects directly since they have nested func pointers -func testEqual(t *testing.T, c1 *Client, c2 *Client) { +// assertEqual compares the two clients, accounting for nested func pointers +func assertEqual(t *testing.T, c1 *Client, c2 *Client) { assert.Equal(t, fmt.Sprintf("%v", c1.configuration), fmt.Sprintf("%v", c2.configuration)) assert.Equal(t, fmt.Sprintf("%v", c1.parsedBaseAddress), fmt.Sprintf("%v", c2.parsedBaseAddress)) assert.Equal(t, fmt.Sprintf("%v", c1.client), fmt.Sprintf("%v", c2.client)) From 5ba77bbc0559c703e40be74c99f0bce354488648 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 13 Sep 2023 13:49:03 -0400 Subject: [PATCH 3/4] better comment --- client_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 9d131962..1b7035e7 100644 --- a/client_test.go +++ b/client_test.go @@ -40,7 +40,8 @@ func Test_Client_Clone(t *testing.T) { assert.Equal(t, "test-namespace", clone.clientRequestModifiers.headers.namespace) } -// assertEqual compares the two clients, accounting for nested func pointers +// assertEqual compares the two clients, accounting for the nested func pointers, +// which are not comparable in Go (https://go.dev/ref/spec#Comparison_operators) func assertEqual(t *testing.T, c1 *Client, c2 *Client) { assert.Equal(t, fmt.Sprintf("%v", c1.configuration), fmt.Sprintf("%v", c2.configuration)) assert.Equal(t, fmt.Sprintf("%v", c1.parsedBaseAddress), fmt.Sprintf("%v", c2.parsedBaseAddress)) From c76d49130f4c7b68c3d3e4eb538c5fa4c0ade80a Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 13 Sep 2023 13:50:08 -0400 Subject: [PATCH 4/4] . --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 1b7035e7..0c2acc84 100644 --- a/client_test.go +++ b/client_test.go @@ -41,7 +41,7 @@ func Test_Client_Clone(t *testing.T) { } // assertEqual compares the two clients, accounting for the nested func pointers, -// which are not comparable in Go (https://go.dev/ref/spec#Comparison_operators) +// which are not comparable in Go (https://go.dev/ref/spec#Comparison_operators). func assertEqual(t *testing.T, c1 *Client, c2 *Client) { assert.Equal(t, fmt.Sprintf("%v", c1.configuration), fmt.Sprintf("%v", c2.configuration)) assert.Equal(t, fmt.Sprintf("%v", c1.parsedBaseAddress), fmt.Sprintf("%v", c2.parsedBaseAddress))