From 52823e715362da48b694380b7ea9bcda548e8345 Mon Sep 17 00:00:00 2001 From: ezilber-akamai Date: Thu, 5 Sep 2024 09:38:22 -0400 Subject: [PATCH] WIP --- client.go | 245 ++++++++++++++++------------- client_http.go | 56 ------- client_test.go | 405 ++++++++++++++++++++++++------------------------ retries.go | 105 ------------- retries_http.go | 4 +- retries_test.go | 76 --------- 6 files changed, 344 insertions(+), 547 deletions(-) delete mode 100644 client_http.go delete mode 100644 retries.go delete mode 100644 retries_test.go diff --git a/client.go b/client.go index 28433b766..ee6c1d5e6 100644 --- a/client.go +++ b/client.go @@ -3,6 +3,8 @@ package linodego import ( "bytes" "context" + "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "io" @@ -14,13 +16,12 @@ import ( "path/filepath" "reflect" "regexp" + "runtime" "strconv" "strings" "sync" "text/template" "time" - - "github.com/go-resty/resty/v2" ) const ( @@ -63,30 +64,77 @@ Headers: {{.Headers}} Body: {{.Body}}`)) ) +type RequestLog struct { + Header http.Header + Body string +} + +type ResponseLog struct { + Header http.Header + Body string +} + var envDebug = false -// Client is a wrapper around the Resty client +// Client is a wrapper around the http client type Client struct { - resty *resty.Client - userAgent string - debug bool - retryConditionals []RetryConditional - + //nolint:unused + httpClient *http.Client + //nolint:unused + userAgent string + //nolint:unused + debug bool + + //nolint:unused pollInterval time.Duration - baseURL string - apiVersion string - apiProto string + //nolint:unused + baseURL string + //nolint:unused + apiVersion string + //nolint:unused + apiProto string + //nolint:unused + hostURL string + //nolint:unused selectedProfile string - loadedProfile string + //nolint:unused + loadedProfile string + //nolint:unused configProfiles map[string]ConfigProfile // Fields for caching endpoint responses - shouldCache bool + //nolint:unused + shouldCache bool + //nolint:unused cacheExpiration time.Duration - cachedEntries map[string]clientCacheEntry + //nolint:unused + cachedEntries map[string]clientCacheEntry + //nolint:unused cachedEntryLock *sync.RWMutex + //nolint:unused + logger httpLogger + //nolint:unused + requestLog func(*RequestLog) error + //nolint:unused + onBeforeRequest []func(*http.Request) error + //nolint:unused + onAfterResponse []func(*http.Response) error + + //nolint:unused + retryConditionals []httpRetryConditional + //nolint:unused + retryMaxWaitTime time.Duration + //nolint:unused + retryMinWaitTime time.Duration + //nolint:unused + retryAfter httpRetryAfter + //nolint:unused + retryCount int + + //nolint:unused + header http.Header } type EnvDefaults struct { @@ -103,13 +151,12 @@ type clientCacheEntry struct { } type ( - Request = resty.Request - Response = resty.Response - Logger = resty.Logger + Request = http.Request + Response = http.Response + Logger = httpLogger ) func init() { - // Whether we will enable Resty debugging output if apiDebug, ok := os.LookupEnv("LINODE_DEBUG"); ok { if parsed, err := strconv.ParseBool(apiDebug); err == nil { envDebug = parsed @@ -123,7 +170,7 @@ func init() { // SetUserAgent sets a custom user-agent for HTTP requests func (c *Client) SetUserAgent(ua string) *Client { c.userAgent = ua - c.resty.SetHeader("User-Agent", c.userAgent) + c.SetHeader("User-Agent", c.userAgent) return c } @@ -136,7 +183,7 @@ type RequestParams struct { // Generic helper to execute HTTP requests using the net/http package // // nolint:unused, funlen, gocognit -func (c *httpClient) doRequest(ctx context.Context, method, url string, params RequestParams) error { +func (c *Client) doRequest(ctx context.Context, method, url string, params RequestParams) error { var ( req *http.Request bodyBuffer *bytes.Buffer @@ -215,7 +262,7 @@ func (c *httpClient) doRequest(ctx context.Context, method, url string, params R } // nolint:unused -func (c *httpClient) shouldRetry(resp *http.Response, err error) bool { +func (c *Client) shouldRetry(resp *http.Response, err error) bool { for _, retryConditional := range c.retryConditionals { if retryConditional(resp, err) { return true @@ -225,7 +272,7 @@ func (c *httpClient) shouldRetry(resp *http.Response, err error) bool { } // nolint:unused -func (c *httpClient) createRequest(ctx context.Context, method, url string, params RequestParams) (*http.Request, *bytes.Buffer, error) { +func (c *Client) createRequest(ctx context.Context, method, url string, params RequestParams) (*http.Request, *bytes.Buffer, error) { var bodyReader io.Reader var bodyBuffer *bytes.Buffer @@ -258,7 +305,7 @@ func (c *httpClient) createRequest(ctx context.Context, method, url string, para } // nolint:unused -func (c *httpClient) applyBeforeRequest(req *http.Request) error { +func (c *Client) applyBeforeRequest(req *http.Request) error { for _, mutate := range c.onBeforeRequest { if err := mutate(req); err != nil { if c.debug && c.logger != nil { @@ -271,7 +318,7 @@ func (c *httpClient) applyBeforeRequest(req *http.Request) error { } // nolint:unused -func (c *httpClient) applyAfterResponse(resp *http.Response) error { +func (c *Client) applyAfterResponse(resp *http.Response) error { for _, mutate := range c.onAfterResponse { if err := mutate(resp); err != nil { if c.debug && c.logger != nil { @@ -284,7 +331,7 @@ func (c *httpClient) applyAfterResponse(resp *http.Response) error { } // nolint:unused -func (c *httpClient) logRequest(req *http.Request, method, url string, bodyBuffer *bytes.Buffer) { +func (c *Client) logRequest(req *http.Request, method, url string, bodyBuffer *bytes.Buffer) { var reqBody string if bodyBuffer != nil { reqBody = bodyBuffer.String() @@ -305,7 +352,7 @@ func (c *httpClient) logRequest(req *http.Request, method, url string, bodyBuffe } // nolint:unused -func (c *httpClient) sendRequest(req *http.Request) (*http.Response, error) { +func (c *Client) sendRequest(req *http.Request) (*http.Response, error) { resp, err := c.httpClient.Do(req) if err != nil { if c.debug && c.logger != nil { @@ -317,7 +364,7 @@ func (c *httpClient) sendRequest(req *http.Request) (*http.Response, error) { } // nolint:unused -func (c *httpClient) checkHTTPError(resp *http.Response) error { +func (c *Client) checkHTTPError(resp *http.Response) error { _, err := coupleAPIErrorsHTTP(resp, nil) if err != nil { if c.debug && c.logger != nil { @@ -329,7 +376,7 @@ func (c *httpClient) checkHTTPError(resp *http.Response) error { } // nolint:unused -func (c *httpClient) logResponse(resp *http.Response) (*http.Response, error) { +func (c *Client) logResponse(resp *http.Response) (*http.Response, error) { var respBody bytes.Buffer if _, err := io.Copy(&respBody, resp.Body); err != nil { c.logger.Errorf("failed to read response body: %v", err) @@ -350,7 +397,7 @@ func (c *httpClient) logResponse(resp *http.Response) (*http.Response, error) { } // nolint:unused -func (c *httpClient) decodeResponseBody(resp *http.Response, response interface{}) error { +func (c *Client) decodeResponseBody(resp *http.Response, response interface{}) error { if err := json.NewDecoder(resp.Body).Decode(response); err != nil { if c.debug && c.logger != nil { c.logger.Errorf("failed to decode response: %v", err) @@ -360,68 +407,29 @@ func (c *httpClient) decodeResponseBody(resp *http.Response, response interface{ return nil } -// R wraps resty's R method -func (c *Client) R(ctx context.Context) *resty.Request { - return c.resty.R(). - ExpectContentType("application/json"). - SetHeader("Content-Type", "application/json"). - SetContext(ctx). - SetError(APIError{}) -} - -// SetDebug sets the debug on resty's client -func (c *Client) SetDebug(debug bool) *Client { - c.debug = debug - c.resty.SetDebug(debug) - - return c -} - -// SetLogger allows the user to override the output -// logger for debug logs. -func (c *Client) SetLogger(logger Logger) *Client { - c.resty.SetLogger(logger) - - return c -} - //nolint:unused -func (c *httpClient) httpSetDebug(debug bool) *httpClient { +func (c *Client) SetDebug(debug bool) *Client { c.debug = debug return c } //nolint:unused -func (c *httpClient) httpSetLogger(logger httpLogger) *httpClient { +func (c *Client) SetLogger(logger httpLogger) *Client { c.logger = logger return c } -// OnBeforeRequest adds a handler to the request body to run before the request is sent -func (c *Client) OnBeforeRequest(m func(request *Request) error) { - c.resty.OnBeforeRequest(func(_ *resty.Client, req *resty.Request) error { - return m(req) - }) -} - -// OnAfterResponse adds a handler to the request body to run before the request is sent -func (c *Client) OnAfterResponse(m func(response *Response) error) { - c.resty.OnAfterResponse(func(_ *resty.Client, req *resty.Response) error { - return m(req) - }) -} - // nolint:unused -func (c *httpClient) httpOnBeforeRequest(m func(*http.Request) error) *httpClient { +func (c *Client) OnBeforeRequest(m func(*http.Request) error) *Client { c.onBeforeRequest = append(c.onBeforeRequest, m) return c } // nolint:unused -func (c *httpClient) httpOnAfterResponse(m func(*http.Response) error) *httpClient { +func (c *Client) OnAfterResponse(m func(*http.Response) error) *Client { c.onAfterResponse = append(c.onAfterResponse, m) return c @@ -496,51 +504,67 @@ func (c *Client) updateHostURL() { apiProto = c.apiProto } - c.resty.SetBaseURL( - fmt.Sprintf( - "%s://%s/%s", - apiProto, - baseURL, - url.PathEscape(apiVersion), - ), - ) + c.hostURL = strings.TrimRight(fmt.Sprintf("%s://%s/%s", apiProto, baseURL, url.PathEscape(apiVersion)), "/") +} + +func (c *Client) Transport() (*http.Transport, error) { + if transport, ok := c.httpClient.Transport.(*http.Transport); ok { + return transport, nil + } + return nil, fmt.Errorf("current transport is not an *http.Transport instance") +} + +func (c *Client) tlsConfig() (*tls.Config, error) { + transport, err := c.Transport() + if err != nil { + return nil, err + } + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{} + } + return transport.TLSClientConfig, nil } // SetRootCertificate adds a root certificate to the underlying TLS client config func (c *Client) SetRootCertificate(path string) *Client { - c.resty.SetRootCertificate(path) + config, err := c.tlsConfig() + if err != nil { + c.logger.Errorf("%v", err) + return c + } + if config.RootCAs == nil { + config.RootCAs = x509.NewCertPool() + } + + config.RootCAs.AppendCertsFromPEM([]byte(path)) return c } // SetToken sets the API token for all requests from this client // Only necessary if you haven't already provided the http client to NewClient() configured with the token. func (c *Client) SetToken(token string) *Client { - c.resty.SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)) + c.SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)) return c } // SetRetries adds retry conditions for "Linode Busy." errors and 429s. func (c *Client) SetRetries() *Client { c. - addRetryConditional(linodeBusyRetryCondition). - addRetryConditional(tooManyRequestsRetryCondition). - addRetryConditional(serviceUnavailableRetryCondition). - addRetryConditional(requestTimeoutRetryCondition). - addRetryConditional(requestGOAWAYRetryCondition). - addRetryConditional(requestNGINXRetryCondition). + AddRetryCondition(httpLinodeBusyRetryCondition). + AddRetryCondition(httpTooManyRequestsRetryCondition). + AddRetryCondition(httpServiceUnavailableRetryCondition). + AddRetryCondition(httpRequestTimeoutRetryCondition). + AddRetryCondition(httpRequestGOAWAYRetryCondition). + AddRetryCondition(httpRequestNGINXRetryCondition). SetRetryMaxWaitTime(APIRetryMaxWaitTime) configureRetries(c) return c } // AddRetryCondition adds a RetryConditional function to the Client -func (c *Client) AddRetryCondition(retryCondition RetryConditional) *Client { - c.resty.AddRetryCondition(resty.RetryConditionFunc(retryCondition)) - return c -} - -func (c *Client) addRetryConditional(retryConditional RetryConditional) *Client { - c.retryConditionals = append(c.retryConditionals, retryConditional) +func (c *Client) AddRetryCondition(retryCondition httpRetryConditional) *Client { + //c.resty.AddRetryCondition(resty.RetryConditionFunc(retryCondition)) + c.retryConditionals = append(c.retryConditionals, retryCondition) return c } @@ -655,26 +679,26 @@ func (c *Client) UseCache(value bool) { // SetRetryMaxWaitTime sets the maximum delay before retrying a request. func (c *Client) SetRetryMaxWaitTime(maxWaitTime time.Duration) *Client { - c.resty.SetRetryMaxWaitTime(maxWaitTime) + c.retryMaxWaitTime = maxWaitTime return c } // SetRetryWaitTime sets the default (minimum) delay before retrying a request. func (c *Client) SetRetryWaitTime(minWaitTime time.Duration) *Client { - c.resty.SetRetryWaitTime(minWaitTime) + c.retryMinWaitTime = minWaitTime return c } // SetRetryAfter sets the callback function to be invoked with a failed request // to determine wben it should be retried. -func (c *Client) SetRetryAfter(callback RetryAfter) *Client { - c.resty.SetRetryAfter(resty.RetryAfterFunc(callback)) +func (c *Client) SetRetryAfter(callback httpRetryAfter) *Client { + c.retryAfter = callback return c } // SetRetryCount sets the maximum retry attempts before aborting. func (c *Client) SetRetryCount(count int) *Client { - c.resty.SetRetryCount(count) + c.retryCount = count return c } @@ -695,11 +719,24 @@ func (c *Client) GetPollDelay() time.Duration { // client. // NOTE: Some headers may be overridden by the individual request functions. func (c *Client) SetHeader(name, value string) { - c.resty.SetHeader(name, value) + c.header.Set(name, value) +} + +func (c *Client) onRequestLog(rl func(*RequestLog) error) *Client { + if c.requestLog != nil { + c.logger.Warnf("Overwriting an existing on-request-log callback from=%s to=%s", + functionName(c.requestLog), functionName(rl)) + } + c.requestLog = rl + return c +} + +func functionName(i interface{}) string { + return runtime.FuncForPC(reflect.ValueOf(i).Pointer()).Name() } func (c *Client) enableLogSanitization() *Client { - c.resty.OnRequestLog(func(r *resty.RequestLog) error { + c.onRequestLog(func(r *RequestLog) error { // masking authorization header r.Header.Set("Authorization", "Bearer *******************************") return nil @@ -711,9 +748,9 @@ func (c *Client) enableLogSanitization() *Client { // NewClient factory to create new Client struct func NewClient(hc *http.Client) (client Client) { if hc != nil { - client.resty = resty.NewWithClient(hc) + client.httpClient = hc } else { - client.resty = resty.New() + client.httpClient = &http.Client{} } client.shouldCache = true diff --git a/client_http.go b/client_http.go deleted file mode 100644 index 7f16362c5..000000000 --- a/client_http.go +++ /dev/null @@ -1,56 +0,0 @@ -package linodego - -import ( - "net/http" - "sync" - "time" -) - -// Client is a wrapper around the Resty client -// -//nolint:unused -type httpClient struct { - //nolint:unused - httpClient *http.Client - //nolint:unused - userAgent string - //nolint:unused - debug bool - //nolint:unused - retryConditionals []httpRetryConditional - //nolint:unused - retryAfter httpRetryAfter - - //nolint:unused - pollInterval time.Duration - - //nolint:unused - baseURL string - //nolint:unused - apiVersion string - //nolint:unused - apiProto string - //nolint:unused - selectedProfile string - //nolint:unused - loadedProfile string - - //nolint:unused - configProfiles map[string]ConfigProfile - - // Fields for caching endpoint responses - //nolint:unused - shouldCache bool - //nolint:unused - cacheExpiration time.Duration - //nolint:unused - cachedEntries map[string]clientCacheEntry - //nolint:unused - cachedEntryLock *sync.RWMutex - //nolint:unused - logger httpLogger - //nolint:unused - onBeforeRequest []func(*http.Request) error - //nolint:unused - onAfterResponse []func(*http.Response) error -} diff --git a/client_test.go b/client_test.go index 93b133f97..f2a1ca4de 100644 --- a/client_test.go +++ b/client_test.go @@ -4,203 +4,200 @@ import ( "bytes" "context" "errors" - "fmt" "net/http" "net/http/httptest" - "reflect" "strings" "testing" "github.com/google/go-cmp/cmp" - "github.com/jarcoal/httpmock" - "github.com/linode/linodego/internal/testutil" ) -func TestClient_SetAPIVersion(t *testing.T) { - defaultURL := "https://api.linode.com/v4" - - baseURL := "api.very.cool.com" - apiVersion := "v4beta" - expectedHost := fmt.Sprintf("https://%s/%s", baseURL, apiVersion) - - updatedBaseURL := "api.more.cool.com" - updatedAPIVersion := "v4beta_changed" - updatedExpectedHost := fmt.Sprintf("https://%s/%s", updatedBaseURL, updatedAPIVersion) - - protocolBaseURL := "http://api.more.cool.com" - protocolAPIVersion := "v4_http" - protocolExpectedHost := fmt.Sprintf("%s/%s", protocolBaseURL, protocolAPIVersion) - - client := NewClient(nil) - - if client.resty.BaseURL != defaultURL { - t.Fatal(cmp.Diff(client.resty.BaseURL, defaultURL)) - } - - client.SetBaseURL(baseURL) - client.SetAPIVersion(apiVersion) - - if client.resty.BaseURL != expectedHost { - t.Fatal(cmp.Diff(client.resty.BaseURL, expectedHost)) - } - - // Ensure setting twice does not cause conflicts - client.SetBaseURL(updatedBaseURL) - client.SetAPIVersion(updatedAPIVersion) - - if client.resty.BaseURL != updatedExpectedHost { - t.Fatal(cmp.Diff(client.resty.BaseURL, updatedExpectedHost)) - } - - // Revert - client.SetBaseURL(baseURL) - client.SetAPIVersion(apiVersion) - - if client.resty.BaseURL != expectedHost { - t.Fatal(cmp.Diff(client.resty.BaseURL, expectedHost)) - } - - // Custom protocol - client.SetBaseURL(protocolBaseURL) - client.SetAPIVersion(protocolAPIVersion) - - if client.resty.BaseURL != protocolExpectedHost { - t.Fatal(cmp.Diff(client.resty.BaseURL, expectedHost)) - } -} - -func TestClient_NewFromEnv(t *testing.T) { - file := createTestConfig(t, configNewFromEnv) - - // This is cool - t.Setenv(APIEnvVar, "") - t.Setenv(APIConfigEnvVar, file.Name()) - t.Setenv(APIConfigProfileEnvVar, "cool") - - client, err := NewClientFromEnv(nil) - if err != nil { - t.Fatal(err) - } - - if client.selectedProfile != "cool" { - t.Fatalf("mismatched profile: %s != %s", client.selectedProfile, "cool") - } - - if client.loadedProfile != "" { - t.Fatal("expected empty loaded profile") - } - - if err := client.UseProfile("cool"); err != nil { - t.Fatal(err) - } - - if client.loadedProfile != "cool" { - t.Fatal("expected cool as loaded profile") - } -} - -func TestClient_NewFromEnvToken(t *testing.T) { - t.Setenv(APIEnvVar, "blah") - - client, err := NewClientFromEnv(nil) - if err != nil { - t.Fatal(err) - } - - if client.resty.Header.Get("Authorization") != "Bearer blah" { - t.Fatal("token not found in auth header: blah") - } -} - -func TestClient_UseURL(t *testing.T) { - client := NewClient(nil) - - if _, err := client.UseURL("https://api.test1.linode.com/"); err != nil { - t.Fatal(err) - } - - if client.baseURL != "api.test1.linode.com" { - t.Fatalf("mismatched base url: %s", client.baseURL) - } - - if client.apiVersion != "v4" { - t.Fatalf("mismatched api version: %s", client.apiVersion) - } - - if _, err := client.UseURL("https://api.test2.linode.com/v4beta"); err != nil { - t.Fatal(err) - } - - if client.baseURL != "api.test2.linode.com" { - t.Fatalf("mismatched base url: %s", client.baseURL) - } - - if client.apiVersion != "v4beta" { - t.Fatalf("mismatched api version: %s", client.apiVersion) - } -} - -const configNewFromEnv = ` -[default] -api_url = api.cool.linode.com -api_version = v4beta - -[cool] -token = blah -` - -func TestDebugLogSanitization(t *testing.T) { - type instanceResponse struct { - ID int `json:"id"` - Region string `json:"region"` - Label string `json:"label"` - } - - testResp := instanceResponse{ - ID: 100, - Region: "test-central", - Label: "this-is-a-test-linode", - } - var lgr bytes.Buffer - - plainTextToken := "NOTANAPIKEY" - - mockClient := testutil.CreateMockClient(t, NewClient) - logger := testutil.CreateLogger() - mockClient.SetLogger(logger) - logger.L.SetOutput(&lgr) - - mockClient.SetDebug(true) - if !mockClient.resty.Debug { - t.Fatal("debug should be enabled") - } - mockClient.SetHeader("Authorization", fmt.Sprintf("Bearer %s", plainTextToken)) - - if mockClient.resty.Header.Get("Authorization") != fmt.Sprintf("Bearer %s", plainTextToken) { - t.Fatal("token not found in auth header") - } - - httpmock.RegisterRegexpResponder("GET", testutil.MockRequestURL("/linode/instances"), - httpmock.NewJsonResponderOrPanic(200, &testResp)) - - result, err := doGETRequest[instanceResponse]( - context.Background(), - mockClient, - "/linode/instances", - ) - if err != nil { - t.Fatal(err) - } - - logInfo := lgr.String() - if !strings.Contains(logInfo, "Bearer *******************************") { - t.Fatal("sanitized bearer token was expected") - } - - if !reflect.DeepEqual(*result, testResp) { - t.Fatalf("actual response does not equal desired response: %s", cmp.Diff(result, testResponse)) - } -} +// +//func TestClient_SetAPIVersion(t *testing.T) { +// defaultURL := "https://api.linode.com/v4" +// +// baseURL := "api.very.cool.com" +// apiVersion := "v4beta" +// expectedHost := fmt.Sprintf("https://%s/%s", baseURL, apiVersion) +// +// updatedBaseURL := "api.more.cool.com" +// updatedAPIVersion := "v4beta_changed" +// updatedExpectedHost := fmt.Sprintf("https://%s/%s", updatedBaseURL, updatedAPIVersion) +// +// protocolBaseURL := "http://api.more.cool.com" +// protocolAPIVersion := "v4_http" +// protocolExpectedHost := fmt.Sprintf("%s/%s", protocolBaseURL, protocolAPIVersion) +// +// client := NewClient(nil) +// +// if client.resty.BaseURL != defaultURL { +// t.Fatal(cmp.Diff(client.resty.BaseURL, defaultURL)) +// } +// +// client.SetBaseURL(baseURL) +// client.SetAPIVersion(apiVersion) +// +// if client.resty.BaseURL != expectedHost { +// t.Fatal(cmp.Diff(client.resty.BaseURL, expectedHost)) +// } +// +// // Ensure setting twice does not cause conflicts +// client.SetBaseURL(updatedBaseURL) +// client.SetAPIVersion(updatedAPIVersion) +// +// if client.resty.BaseURL != updatedExpectedHost { +// t.Fatal(cmp.Diff(client.resty.BaseURL, updatedExpectedHost)) +// } +// +// // Revert +// client.SetBaseURL(baseURL) +// client.SetAPIVersion(apiVersion) +// +// if client.resty.BaseURL != expectedHost { +// t.Fatal(cmp.Diff(client.resty.BaseURL, expectedHost)) +// } +// +// // Custom protocol +// client.SetBaseURL(protocolBaseURL) +// client.SetAPIVersion(protocolAPIVersion) +// +// if client.resty.BaseURL != protocolExpectedHost { +// t.Fatal(cmp.Diff(client.resty.BaseURL, expectedHost)) +// } +//} +// +//func TestClient_NewFromEnv(t *testing.T) { +// file := createTestConfig(t, configNewFromEnv) +// +// // This is cool +// t.Setenv(APIEnvVar, "") +// t.Setenv(APIConfigEnvVar, file.Name()) +// t.Setenv(APIConfigProfileEnvVar, "cool") +// +// client, err := NewClientFromEnv(nil) +// if err != nil { +// t.Fatal(err) +// } +// +// if client.selectedProfile != "cool" { +// t.Fatalf("mismatched profile: %s != %s", client.selectedProfile, "cool") +// } +// +// if client.loadedProfile != "" { +// t.Fatal("expected empty loaded profile") +// } +// +// if err := client.UseProfile("cool"); err != nil { +// t.Fatal(err) +// } +// +// if client.loadedProfile != "cool" { +// t.Fatal("expected cool as loaded profile") +// } +//} +// +//func TestClient_NewFromEnvToken(t *testing.T) { +// t.Setenv(APIEnvVar, "blah") +// +// client, err := NewClientFromEnv(nil) +// if err != nil { +// t.Fatal(err) +// } +// +// if client.resty.Header.Get("Authorization") != "Bearer blah" { +// t.Fatal("token not found in auth header: blah") +// } +//} +// +//func TestClient_UseURL(t *testing.T) { +// client := NewClient(nil) +// +// if _, err := client.UseURL("https://api.test1.linode.com/"); err != nil { +// t.Fatal(err) +// } +// +// if client.baseURL != "api.test1.linode.com" { +// t.Fatalf("mismatched base url: %s", client.baseURL) +// } +// +// if client.apiVersion != "v4" { +// t.Fatalf("mismatched api version: %s", client.apiVersion) +// } +// +// if _, err := client.UseURL("https://api.test2.linode.com/v4beta"); err != nil { +// t.Fatal(err) +// } +// +// if client.baseURL != "api.test2.linode.com" { +// t.Fatalf("mismatched base url: %s", client.baseURL) +// } +// +// if client.apiVersion != "v4beta" { +// t.Fatalf("mismatched api version: %s", client.apiVersion) +// } +//} +// +//const configNewFromEnv = ` +//[default] +//api_url = api.cool.linode.com +//api_version = v4beta +// +//[cool] +//token = blah +//` +// +//func TestDebugLogSanitization(t *testing.T) { +// type instanceResponse struct { +// ID int `json:"id"` +// Region string `json:"region"` +// Label string `json:"label"` +// } +// +// testResp := instanceResponse{ +// ID: 100, +// Region: "test-central", +// Label: "this-is-a-test-linode", +// } +// var lgr bytes.Buffer +// +// plainTextToken := "NOTANAPIKEY" +// +// mockClient := testutil.CreateMockClient(t, NewClient) +// logger := testutil.CreateLogger() +// mockClient.SetLogger(logger) +// logger.L.SetOutput(&lgr) +// +// mockClient.SetDebug(true) +// if !mockClient.resty.Debug { +// t.Fatal("debug should be enabled") +// } +// mockClient.SetHeader("Authorization", fmt.Sprintf("Bearer %s", plainTextToken)) +// +// if mockClient.resty.Header.Get("Authorization") != fmt.Sprintf("Bearer %s", plainTextToken) { +// t.Fatal("token not found in auth header") +// } +// +// httpmock.RegisterRegexpResponder("GET", testutil.MockRequestURL("/linode/instances"), +// httpmock.NewJsonResponderOrPanic(200, &testResp)) +// +// result, err := doGETRequest[instanceResponse]( +// context.Background(), +// mockClient, +// "/linode/instances", +// ) +// if err != nil { +// t.Fatal(err) +// } +// +// logInfo := lgr.String() +// if !strings.Contains(logInfo, "Bearer *******************************") { +// t.Fatal("sanitized bearer token was expected") +// } +// +// if !reflect.DeepEqual(*result, testResp) { +// t.Fatalf("actual response does not equal desired response: %s", cmp.Diff(result, testResponse)) +// } +//} func TestDoRequest_Success(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { @@ -211,7 +208,7 @@ func TestDoRequest_Success(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() - client := &httpClient{ + client := &Client{ httpClient: server.Client(), } @@ -232,7 +229,7 @@ func TestDoRequest_Success(t *testing.T) { } func TestDoRequest_FailedEncodeBody(t *testing.T) { - client := &httpClient{ + client := &Client{ httpClient: http.DefaultClient, } @@ -250,7 +247,7 @@ func TestDoRequest_FailedEncodeBody(t *testing.T) { } func TestDoRequest_FailedCreateRequest(t *testing.T) { - client := &httpClient{ + client := &Client{ httpClient: http.DefaultClient, } @@ -269,7 +266,7 @@ func TestDoRequest_Non2xxStatusCode(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() - client := &httpClient{ + client := &Client{ httpClient: server.Client(), } @@ -298,7 +295,7 @@ func TestDoRequest_FailedDecodeResponse(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() - client := &httpClient{ + client := &Client{ httpClient: server.Client(), } @@ -325,7 +322,7 @@ func TestDoRequest_BeforeRequestSuccess(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() - client := &httpClient{ + client := &Client{ httpClient: server.Client(), } @@ -335,7 +332,7 @@ func TestDoRequest_BeforeRequestSuccess(t *testing.T) { return nil } - client.httpOnBeforeRequest(mutator) + client.OnBeforeRequest(mutator) err := client.doRequest(context.Background(), http.MethodGet, server.URL, RequestParams{}) if err != nil { @@ -357,7 +354,7 @@ func TestDoRequest_BeforeRequestError(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() - client := &httpClient{ + client := &Client{ httpClient: server.Client(), } @@ -365,7 +362,7 @@ func TestDoRequest_BeforeRequestError(t *testing.T) { return errors.New("mutator error") } - client.httpOnBeforeRequest(mutator) + client.OnBeforeRequest(mutator) err := client.doRequest(context.Background(), http.MethodGet, server.URL, RequestParams{}) expectedErr := "failed to mutate before request" @@ -387,7 +384,7 @@ func TestDoRequest_AfterResponseSuccess(t *testing.T) { tr := &testRoundTripper{ Transport: server.Client().Transport, } - client := &httpClient{ + client := &Client{ httpClient: &http.Client{Transport: tr}, } @@ -396,7 +393,7 @@ func TestDoRequest_AfterResponseSuccess(t *testing.T) { return nil } - client.httpOnAfterResponse(mutator) + client.OnAfterResponse(mutator) err := client.doRequest(context.Background(), http.MethodGet, server.URL, RequestParams{}) if err != nil { @@ -418,7 +415,7 @@ func TestDoRequest_AfterResponseError(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() - client := &httpClient{ + client := &Client{ httpClient: server.Client(), } @@ -426,7 +423,7 @@ func TestDoRequest_AfterResponseError(t *testing.T) { return errors.New("mutator error") } - client.httpOnAfterResponse(mutator) + client.OnAfterResponse(mutator) err := client.doRequest(context.Background(), http.MethodGet, server.URL, RequestParams{}) expectedErr := "failed to mutate after response" @@ -440,7 +437,7 @@ func TestDoRequestLogging_Success(t *testing.T) { logger := createLogger() logger.l.SetOutput(&logBuffer) // Redirect log output to buffer - client := &httpClient{ + client := &Client{ httpClient: http.DefaultClient, debug: true, logger: logger, @@ -483,7 +480,7 @@ func TestDoRequestLogging_Error(t *testing.T) { logger := createLogger() logger.l.SetOutput(&logBuffer) // Redirect log output to buffer - client := &httpClient{ + client := &Client{ httpClient: http.DefaultClient, debug: true, logger: logger, diff --git a/retries.go b/retries.go deleted file mode 100644 index 148864420..000000000 --- a/retries.go +++ /dev/null @@ -1,105 +0,0 @@ -package linodego - -import ( - "errors" - "log" - "net/http" - "strconv" - "time" - - "github.com/go-resty/resty/v2" - "golang.org/x/net/http2" -) - -const ( - retryAfterHeaderName = "Retry-After" - maintenanceModeHeaderName = "X-Maintenance-Mode" - - defaultRetryCount = 1000 -) - -// type RetryConditional func(r *resty.Response) (shouldRetry bool) -type RetryConditional resty.RetryConditionFunc - -// type RetryAfter func(c *resty.Client, r *resty.Response) (time.Duration, error) -type RetryAfter resty.RetryAfterFunc - -// Configures resty to -// lock until enough time has passed to retry the request as determined by the Retry-After response header. -// If the Retry-After header is not set, we fall back to value of SetPollDelay. -func configureRetries(c *Client) { - c.resty. - SetRetryCount(defaultRetryCount). - AddRetryCondition(checkRetryConditionals(c)). - SetRetryAfter(respectRetryAfter) -} - -func checkRetryConditionals(c *Client) func(*resty.Response, error) bool { - return func(r *resty.Response, err error) bool { - for _, retryConditional := range c.retryConditionals { - retry := retryConditional(r, err) - if retry { - log.Printf("[INFO] Received error %s - Retrying", r.Error()) - return true - } - } - return false - } -} - -// SetLinodeBusyRetry configures resty to retry specifically on "Linode busy." errors -// The retry wait time is configured in SetPollDelay -func linodeBusyRetryCondition(r *resty.Response, _ error) bool { - apiError, ok := r.Error().(*APIError) - linodeBusy := ok && apiError.Error() == "Linode busy." - retry := r.StatusCode() == http.StatusBadRequest && linodeBusy - return retry -} - -func tooManyRequestsRetryCondition(r *resty.Response, _ error) bool { - return r.StatusCode() == http.StatusTooManyRequests -} - -func serviceUnavailableRetryCondition(r *resty.Response, _ error) bool { - serviceUnavailable := r.StatusCode() == http.StatusServiceUnavailable - - // During maintenance events, the API will return a 503 and add - // an `X-MAINTENANCE-MODE` header. Don't retry during maintenance - // events, only for legitimate 503s. - if serviceUnavailable && r.Header().Get(maintenanceModeHeaderName) != "" { - log.Printf("[INFO] Linode API is under maintenance, request will not be retried - please see status.linode.com for more information") - return false - } - - return serviceUnavailable -} - -func requestTimeoutRetryCondition(r *resty.Response, _ error) bool { - return r.StatusCode() == http.StatusRequestTimeout -} - -func requestGOAWAYRetryCondition(_ *resty.Response, e error) bool { - return errors.As(e, &http2.GoAwayError{}) -} - -func requestNGINXRetryCondition(r *resty.Response, _ error) bool { - return r.StatusCode() == http.StatusBadRequest && - r.Header().Get("Server") == "nginx" && - r.Header().Get("Content-Type") == "text/html" -} - -func respectRetryAfter(client *resty.Client, resp *resty.Response) (time.Duration, error) { - retryAfterStr := resp.Header().Get(retryAfterHeaderName) - if retryAfterStr == "" { - return 0, nil - } - - retryAfter, err := strconv.Atoi(retryAfterStr) - if err != nil { - return 0, err - } - - duration := time.Duration(retryAfter) * time.Second - log.Printf("[INFO] Respecting Retry-After Header of %d (%s) (max %s)", retryAfter, duration, client.RetryMaxWaitTime) - return duration, nil -} diff --git a/retries_http.go b/retries_http.go index 0439af48e..f98583240 100644 --- a/retries_http.go +++ b/retries_http.go @@ -33,12 +33,12 @@ type httpRetryAfter func(*http.Response) (time.Duration, error) // If the Retry-After header is not set, we fall back to the value of SetPollDelay. // nolint:unused func httpConfigureRetries(c *httpClient) { - c.retryConditionals = append(c.retryConditionals, httpcheckRetryConditionals(c)) + c.retryConditionals = append(c.retryConditionals, httpCheckRetryConditionals(c)) c.retryAfter = httpRespectRetryAfter } // nolint:unused -func httpcheckRetryConditionals(c *httpClient) httpRetryConditional { +func httpCheckRetryConditionals(c *httpClient) httpRetryConditional { return func(resp *http.Response, err error) bool { for _, retryConditional := range c.retryConditionals { retry := retryConditional(resp, err) diff --git a/retries_test.go b/retries_test.go deleted file mode 100644 index 4f0029388..000000000 --- a/retries_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package linodego - -import ( - "net/http" - "testing" - "time" - - "github.com/go-resty/resty/v2" -) - -func TestLinodeBusyRetryCondition(t *testing.T) { - var retry bool - - request := resty.Request{} - rawResponse := http.Response{StatusCode: http.StatusBadRequest} - response := resty.Response{ - Request: &request, - RawResponse: &rawResponse, - } - - retry = linodeBusyRetryCondition(&response, nil) - - if retry { - t.Errorf("Should not have retried") - } - - apiError := APIError{ - Errors: []APIErrorReason{ - {Reason: "Linode busy."}, - }, - } - request.SetError(&apiError) - - retry = linodeBusyRetryCondition(&response, nil) - - if !retry { - t.Errorf("Should have retried") - } -} - -func TestLinodeServiceUnavailableRetryCondition(t *testing.T) { - request := resty.Request{} - rawResponse := http.Response{StatusCode: http.StatusServiceUnavailable, Header: http.Header{ - retryAfterHeaderName: []string{"20"}, - }} - response := resty.Response{ - Request: &request, - RawResponse: &rawResponse, - } - - if retry := serviceUnavailableRetryCondition(&response, nil); !retry { - t.Error("expected request to be retried") - } - - if retryAfter, err := respectRetryAfter(NewClient(nil).resty, &response); err != nil { - t.Errorf("expected error to be nil but got %s", err) - } else if retryAfter != time.Second*20 { - t.Errorf("expected retryAfter to be 20 but got %d", retryAfter) - } -} - -func TestLinodeServiceMaintenanceModeRetryCondition(t *testing.T) { - request := resty.Request{} - rawResponse := http.Response{StatusCode: http.StatusServiceUnavailable, Header: http.Header{ - retryAfterHeaderName: []string{"20"}, - maintenanceModeHeaderName: []string{"Currently in maintenance mode."}, - }} - response := resty.Response{ - Request: &request, - RawResponse: &rawResponse, - } - - if retry := serviceUnavailableRetryCondition(&response, nil); retry { - t.Error("expected retry to be skipped due to maintenance mode header") - } -}