summaryrefslogtreecommitdiff
path: root/client
diff options
context:
space:
mode:
Diffstat (limited to 'client')
-rw-r--r--client/client_test.go60
-rw-r--r--client/gitlabnet.go47
-rw-r--r--client/httpclient.go30
-rw-r--r--client/httpclient_test.go8
-rw-r--r--client/testserver/testserver.go1
5 files changed, 30 insertions, 116 deletions
diff --git a/client/client_test.go b/client/client_test.go
index e48bb67..bf27ef9 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -7,7 +7,6 @@ import (
"io"
"net/http"
"net/http/httptest"
- "os"
"path"
"strings"
"testing"
@@ -306,46 +305,21 @@ func buildRequests(t *testing.T, relativeURLRoot string) []testserver.TestReques
return requests
}
-func TestRetryableHTTPFeatureToggle(t *testing.T) {
- t.Run("retryable http off", func(t *testing.T) {
- os.Setenv("FF_GITLAB_SHELL_RETRYABLE_HTTP", "0")
- reqAttempts := 0
- srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- reqAttempts++
- w.WriteHeader(500)
- }))
- defer srv.Close()
-
- httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, defaultHttpOpts)
- require.NoError(t, err)
- require.NotNil(t, httpClient.HTTPClient)
- require.Nil(t, httpClient.RetryableHTTP)
- client, err := NewGitlabNetClient("", "", "", httpClient)
- require.NoError(t, err)
-
- _, err = client.Get(context.Background(), "/")
- require.EqualError(t, err, "Internal API error (500)")
- require.Equal(t, 1, reqAttempts)
- })
-
- t.Run("retryable http on", func(t *testing.T) {
- os.Setenv("FF_GITLAB_SHELL_RETRYABLE_HTTP", "1")
- reqAttempts := 0
- srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- reqAttempts++
- w.WriteHeader(500)
- }))
- defer srv.Close()
-
- httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, defaultHttpOpts)
- require.NoError(t, err)
- require.Nil(t, httpClient.HTTPClient)
- require.NotNil(t, httpClient.RetryableHTTP)
- client, err := NewGitlabNetClient("", "", "", httpClient)
- require.NoError(t, err)
-
- _, err = client.Get(context.Background(), "/")
- require.EqualError(t, err, "Internal API unreachable")
- require.Equal(t, 3, reqAttempts)
- })
+func TestRetryOnFailure(t *testing.T) {
+ reqAttempts := 0
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ reqAttempts++
+ w.WriteHeader(500)
+ }))
+ defer srv.Close()
+
+ httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, defaultHttpOpts)
+ require.NoError(t, err)
+ require.NotNil(t, httpClient.RetryableHTTP)
+ client, err := NewGitlabNetClient("", "", "", httpClient)
+ require.NoError(t, err)
+
+ _, err = client.Get(context.Background(), "/")
+ require.EqualError(t, err, "Internal API unreachable")
+ require.Equal(t, 3, reqAttempts)
}
diff --git a/client/gitlabnet.go b/client/gitlabnet.go
index 38adf2a..352196e 100644
--- a/client/gitlabnet.go
+++ b/client/gitlabnet.go
@@ -7,7 +7,6 @@ import (
"fmt"
"io"
"net/http"
- "os"
"strings"
"time"
@@ -89,26 +88,7 @@ func appendPath(host string, path string) string {
return strings.TrimSuffix(host, "/") + "/" + strings.TrimPrefix(path, "/")
}
-func newRequest(ctx context.Context, method, host, path string, data interface{}) (*http.Request, error) {
- var jsonReader io.Reader
- if data != nil {
- jsonData, err := json.Marshal(data)
- if err != nil {
- return nil, err
- }
-
- jsonReader = bytes.NewReader(jsonData)
- }
-
- request, err := http.NewRequestWithContext(ctx, method, appendPath(host, path), jsonReader)
- if err != nil {
- return nil, err
- }
-
- return request, nil
-}
-
-func newRetryableRequest(ctx context.Context, method, host, path string, data interface{}) (*retryablehttp.Request, error) {
+func newRequest(ctx context.Context, method, host, path string, data interface{}) (*retryablehttp.Request, error) {
var jsonReader io.Reader
if data != nil {
jsonData, err := json.Marshal(data)
@@ -139,7 +119,6 @@ func parseError(resp *http.Response) error {
} else {
return &ApiError{parsedResponse.Message}
}
-
}
func (c *GitlabNetClient) Get(ctx context.Context, path string) (*http.Response, error) {
@@ -156,15 +135,9 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
return nil, err
}
- retryableRequest, err := newRetryableRequest(ctx, method, c.httpClient.Host, path, data)
- if err != nil {
- return nil, err
- }
-
user, password := c.user, c.password
if user != "" && password != "" {
request.SetBasicAuth(user, password)
- retryableRequest.SetBasicAuth(user, password)
}
claims := jwt.RegisteredClaims{
@@ -178,31 +151,19 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
return nil, err
}
request.Header.Set(apiSecretHeaderName, tokenString)
- retryableRequest.Header.Set(apiSecretHeaderName, tokenString)
originalRemoteIP, ok := ctx.Value(OriginalRemoteIPContextKey{}).(string)
if ok {
request.Header.Add("X-Forwarded-For", originalRemoteIP)
- retryableRequest.Header.Add("X-Forwarded-For", originalRemoteIP)
}
request.Header.Add("Content-Type", "application/json")
- retryableRequest.Header.Add("Content-Type", "application/json")
request.Header.Add("User-Agent", c.userAgent)
- retryableRequest.Header.Add("User-Agent", c.userAgent)
request.Close = true
- retryableRequest.Close = true
start := time.Now()
- var response *http.Response
- var respErr error
- if c.httpClient.HTTPClient != nil {
- response, respErr = c.httpClient.HTTPClient.Do(request)
- }
- if os.Getenv("FF_GITLAB_SHELL_RETRYABLE_HTTP") == "1" && c.httpClient.RetryableHTTP != nil {
- response, respErr = c.httpClient.RetryableHTTP.Do(retryableRequest)
- }
+ response, err := c.httpClient.RetryableHTTP.Do(request)
fields := log.Fields{
"method": method,
"url": request.URL.String(),
@@ -210,8 +171,8 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
}
logger := log.WithContextFields(ctx, fields)
- if respErr != nil {
- logger.WithError(respErr).Error("Internal API unreachable")
+ if err != nil {
+ logger.WithError(err).Error("Internal API unreachable")
return nil, &ApiError{"Internal API unreachable"}
}
diff --git a/client/httpclient.go b/client/httpclient.go
index 82b4b40..9b57add 100644
--- a/client/httpclient.go
+++ b/client/httpclient.go
@@ -32,7 +32,6 @@ const (
var ErrCafileNotFound = errors.New("cafile not found")
type HttpClient struct {
- HTTPClient *http.Client
RetryableHTTP *retryablehttp.Client
Host string
}
@@ -117,28 +116,15 @@ func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath stri
return nil, errors.New("unknown GitLab URL prefix")
}
- c := &http.Client{
- Transport: correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(transport)),
- Timeout: readTimeout(readTimeoutSeconds),
- }
-
- client := &HttpClient{HTTPClient: c, Host: host}
-
- if os.Getenv("FF_GITLAB_SHELL_RETRYABLE_HTTP") == "1" {
- c := retryablehttp.NewClient()
- c.RetryMax = hcc.retryMax
- c.RetryWaitMax = hcc.retryWaitMax
- c.RetryWaitMin = hcc.retryWaitMin
- c.Logger = nil
- c.HTTPClient.Transport = correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(transport))
- c.HTTPClient.Timeout = readTimeout(readTimeoutSeconds)
+ c := retryablehttp.NewClient()
+ c.RetryMax = hcc.retryMax
+ c.RetryWaitMax = hcc.retryWaitMax
+ c.RetryWaitMin = hcc.retryWaitMin
+ c.Logger = nil
+ c.HTTPClient.Transport = correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(transport))
+ c.HTTPClient.Timeout = readTimeout(readTimeoutSeconds)
- client = &HttpClient{RetryableHTTP: c, Host: host}
- }
-
- if client.HTTPClient == nil && client.RetryableHTTP == nil {
- panic("client/httpclient.go did not set http client")
- }
+ client := &HttpClient{RetryableHTTP: c, Host: host}
return client, nil
}
diff --git a/client/httpclient_test.go b/client/httpclient_test.go
index ee2c6fd..b893853 100644
--- a/client/httpclient_test.go
+++ b/client/httpclient_test.go
@@ -21,13 +21,7 @@ func TestReadTimeout(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, client)
- if client.HTTPClient != nil {
- require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.HTTPClient.Timeout)
- }
-
- if client.RetryableHTTP != nil {
- require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.RetryableHTTP.HTTPClient.Timeout)
- }
+ require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.RetryableHTTP.HTTPClient.Timeout)
}
const (
diff --git a/client/testserver/testserver.go b/client/testserver/testserver.go
index 105e7a0..7779f29 100644
--- a/client/testserver/testserver.go
+++ b/client/testserver/testserver.go
@@ -60,7 +60,6 @@ func StartHttpServer(t *testing.T, handlers []TestRequestHandler) string {
}
func StartRetryHttpServer(t *testing.T, handlers []TestRequestHandler) string {
- os.Setenv("FF_GITLAB_SHELL_RETRYABLE_HTTP", "1")
attempts := map[string]int{}
retryMiddileware := func(next func(w http.ResponseWriter, r *http.Request)) http.Handler {
View Lorry Controller Status