feat: put retryablehttp.Client behind feature flag

What
---
- Update the `client.HttpClient` fields to have `http.Client` and
  `retryablehttp.Client`, one of them will be `nil` depending on the
  feature flag toggle.
- Create new method `newRetryableRequest` which will create a
  `retryablehttp.Request` and use that if the
  `FF_GITLAB_SHELL_RETRYABLE_HTTP` feature flag is turned on.
- Add checks for `FF_GITLAB_SHELL_RETRYABLE_HTTP` everywhere we use the
  http client to use the `retryablehttp.Client` or the default
  `http.Client`
- New job `tests-integration-retryableHttp` to run the integraiton tests
  with the new retryablehttp client. We didn't update go tests because
  some assertions are different and will break table driven tests.

Why
---
As discussed in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/703#note_1229645097
we want to put the client behind a feature flag, not just the retry
logic. This does bring extra risk for accessing a `nil` field but there
should be checks everytime we access `RetryableHTTP` and `HTTPClient`.

Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979
Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>

1 files changed, 43 insertions, 5 deletions

diff --git a/client/gitlabnet.go b/client/gitlabnet.go
index 02bef8c..38adf2a 100644
--- a/client/gitlabnet.go
+++ b/client/gitlabnet.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"os"
 	"strings"
 	"time"
 
@@ -54,7 +55,6 @@ func NewGitlabNetClient(
 	secret string,
 	httpClient *HttpClient,
 ) (*GitlabNetClient, error) {
-
 	if httpClient == nil {
 		return nil, fmt.Errorf("Unsupported protocol")
 	}
@@ -89,7 +89,26 @@ 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{}) (*retryablehttp.Request, error) {
+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) {
 	var jsonReader io.Reader
 	if data != nil {
 		jsonData, err := json.Marshal(data)
@@ -137,9 +156,15 @@ 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{
@@ -153,18 +178,31 @@ 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()
-	response, err := c.httpClient.Do(request)
+
+	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)
+	}
 	fields := log.Fields{
 		"method":      method,
 		"url":         request.URL.String(),
@@ -172,8 +210,8 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
 	}
 	logger := log.WithContextFields(ctx, fields)
 
-	if err != nil {
-		logger.WithError(err).Error("Internal API unreachable")
+	if respErr != nil {
+		logger.WithError(respErr).Error("Internal API unreachable")
 		return nil, &ApiError{"Internal API unreachable"}
 	}