From e258e1cef3a93c58555f2c4505a5d69a730c5eb8 Mon Sep 17 00:00:00 2001 From: Mykolas Krupauskas Date: Wed, 22 Mar 2023 14:07:29 -0700 Subject: [PATCH 1/2] add http client timeout option to raw config provider Signed-off-by: Mykolas Krupauskas --- common/client.go | 14 +++++++----- common/configuration.go | 36 +++++++++++++++++++++++++++++-- common/configuration_test.go | 41 ++++++++++++++++++++++++++++++++++-- 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/common/client.go b/common/client.go index fca3d765da..e397a64a79 100644 --- a/common/client.go +++ b/common/client.go @@ -209,7 +209,7 @@ func newBaseClient(signer HTTPRequestSigner, dispatcher HTTPRequestDispatcher) B return baseClient } -func defaultHTTPDispatcher() http.Client { +func defaultHTTPDispatcher(timeout time.Duration) http.Client { var httpClient http.Client var tp = http.DefaultTransport.(*http.Transport) if isExpectHeaderDisabled := IsEnvVarFalse(UsingExpectHeaderEnvVar); !isExpectHeaderDisabled { @@ -239,21 +239,25 @@ func defaultHTTPDispatcher() http.Client { tp.TLSClientConfig = &tls.Config{RootCAs: pool} } httpClient = http.Client{ - Timeout: defaultTimeout, + Timeout: timeout, Transport: tp, } return httpClient } -func defaultBaseClient(provider KeyProvider) BaseClient { - dispatcher := defaultHTTPDispatcher() +func defaultBaseClient(provider ConfigurationProvider) BaseClient { + timeout := defaultTimeout + if httpConfigProvider, ok := provider.(httpConfigurationProvider); ok { + timeout = httpConfigProvider.HTTPTimeout() + } + dispatcher := defaultHTTPDispatcher(timeout) signer := DefaultRequestSigner(provider) return newBaseClient(signer, &dispatcher) } // DefaultBaseClientWithSigner creates a default base client with a given signer func DefaultBaseClientWithSigner(signer HTTPRequestSigner) BaseClient { - dispatcher := defaultHTTPDispatcher() + dispatcher := defaultHTTPDispatcher(defaultTimeout) return newBaseClient(signer, &dispatcher) } diff --git a/common/configuration.go b/common/configuration.go index 2989598a43..65f89b4157 100644 --- a/common/configuration.go +++ b/common/configuration.go @@ -12,6 +12,7 @@ import ( "path" "regexp" "strings" + "time" ) // AuthenticationType for auth @@ -47,6 +48,10 @@ type ConfigurationProvider interface { AuthType() (AuthConfig, error) } +type httpConfigurationProvider interface { + HTTPTimeout() time.Duration +} + // IsConfigurationProviderValid Tests all parts of the configuration provider do not return an error, this method will // not check AuthType(), since authType() is not required to be there. func IsConfigurationProviderValid(conf ConfigurationProvider) (ok bool, err error) { @@ -75,11 +80,34 @@ type rawConfigurationProvider struct { fingerprint string privateKey string privateKeyPassphrase *string + httpTimeout time.Duration +} + +// RawConfigurationProviderOption is a raw configuration provider option. +type RawConfigurationProviderOption interface { + apply(*rawConfigurationProvider) +} + +type httpTimeoutOption struct { + timeout time.Duration +} + +// WithRawConfigProviderHTTPTimeout creates a raw config provider HTTP timeout option. +func WithRawConfigProviderHTTPTimeout(timeout time.Duration) RawConfigurationProviderOption { + return &httpTimeoutOption{timeout} +} + +func (o *httpTimeoutOption) apply(p *rawConfigurationProvider) { + p.httpTimeout = o.timeout } // NewRawConfigurationProvider will create a ConfigurationProvider with the arguments of the function -func NewRawConfigurationProvider(tenancy, user, region, fingerprint, privateKey string, privateKeyPassphrase *string) ConfigurationProvider { - return rawConfigurationProvider{tenancy, user, region, fingerprint, privateKey, privateKeyPassphrase} +func NewRawConfigurationProvider(tenancy, user, region, fingerprint, privateKey string, privateKeyPassphrase *string, opts ...RawConfigurationProviderOption) ConfigurationProvider { + p := rawConfigurationProvider{tenancy, user, region, fingerprint, privateKey, privateKeyPassphrase, defaultTimeout} + for _, opt := range opts { + opt.apply(&p) + } + return p } func (p rawConfigurationProvider) PrivateRSAKey() (key *rsa.PrivateKey, err error) { @@ -134,6 +162,10 @@ func (p rawConfigurationProvider) AuthType() (AuthConfig, error) { return AuthConfig{UnknownAuthenticationType, false, nil}, nil } +func (p rawConfigurationProvider) HTTPTimeout() time.Duration { + return p.httpTimeout +} + // environmentConfigurationProvider reads configuration from environment variables type environmentConfigurationProvider struct { PrivateKeyPassword string diff --git a/common/configuration_test.go b/common/configuration_test.go index 7281739c7d..643eb3c216 100644 --- a/common/configuration_test.go +++ b/common/configuration_test.go @@ -7,12 +7,13 @@ import ( "fmt" "io/ioutil" "os" - "testing" - "path" "strings" + "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -208,6 +209,42 @@ func TestEnvironmentConfigurationProvider_BadRegion(t *testing.T) { assert.Error(t, e) } +func TestHTTPTimeoutRawConfigurationProvider(t *testing.T) { + tests := []struct { + name string + opts []RawConfigurationProviderOption + want time.Duration + }{ + { + name: "default", + want: defaultTimeout, + }, + { + name: "timeout option", + opts: []RawConfigurationProviderOption{ + WithRawConfigProviderHTTPTimeout(time.Hour), + }, + want: time.Hour, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewRawConfigurationProvider( + "ocid1.tenancy.oc1..aaaaaaaaxf3fuazos", + "ocid1.user.oc1..aaaaaaaa3p67n2kmpxnbcnff", + "us-ashburn-1", + "af:81:71:8e:d2", + testPrivateKeyConf, + nil, + tt.opts..., + ) + httpConfigProvider, ok := c.(httpConfigurationProvider) + require.True(t, ok) + assert.Equal(t, tt.want, httpConfigProvider.HTTPTimeout()) + }) + } +} + func TestFileConfigurationProvider_parseConfigFileData(t *testing.T) { data := `[DEFAULT] user=someuser From af9feab2373152c8048b1dae8eb10887644553d5 Mon Sep 17 00:00:00 2001 From: Mykolas Krupauskas Date: Wed, 22 Mar 2023 14:30:14 -0700 Subject: [PATCH 2/2] remove unintentional change in test imports Signed-off-by: Mykolas Krupauskas --- common/configuration_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/configuration_test.go b/common/configuration_test.go index 643eb3c216..4f5f500f17 100644 --- a/common/configuration_test.go +++ b/common/configuration_test.go @@ -7,11 +7,12 @@ import ( "fmt" "io/ioutil" "os" - "path" - "strings" "testing" "time" + "path" + "strings" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" )