Use a separate HTTP Client for Geo requests

Currently, the client that performs HTTP requests for Geo pushes
is the same as the one that performs internal HTTP requests.

However, it causes problems with the following setups:

- Internal requests are configured to be via UNIX connections
- The Primary node is configured to be HTTP(S)

The UNIX client cannot do the request to HTTP(S).

This commit introduces a separate client for Geo requests.

client/httpclient.go

@@ -119,7 +119,7 @@ func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath stri
 	c.RetryWaitMax = hcc.retryWaitMax
 	c.RetryWaitMin = hcc.retryWaitMin
 	c.Logger = nil
-	c.HTTPClient.Transport = newTransport(transport)
+	c.HTTPClient.Transport = NewTransport(transport)
 	c.HTTPClient.Timeout = readTimeout(readTimeoutSeconds)
 
 	client := &HttpClient{RetryableHTTP: c, Host: host}

client/transport.go

@@ -55,7 +55,11 @@ func (rt *transport) RoundTrip(request *http.Request) (*http.Response, error) {
 	return response, nil
 }
 
-func newTransport(next http.RoundTripper) http.RoundTripper {
+func DefaultTransport() http.RoundTripper {
+	return http.DefaultTransport.(*http.Transport).Clone()
+}
+
+func NewTransport(next http.RoundTripper) http.RoundTripper {
 	t := &transport{next: next}
 	return correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(t))
 }

internal/command/githttp/push.go

@@ -34,11 +34,7 @@ type PushCommand struct {
 // 6. Return the output to the user
 func (c *PushCommand) Execute(ctx context.Context) error {
 	data := c.Response.Payload.Data
-	client, err := git.NewClient(c.Config, data.PrimaryRepo, data.RequestHeaders)
-	if err != nil {
-		return err
-	}
-
+	client := &git.Client{Url: data.PrimaryRepo, Headers: data.RequestHeaders}
 	if err := c.requestInfoRefs(ctx, client); err != nil {
 		return err
 	}

internal/command/githttp/push_test.go

@@ -51,7 +51,7 @@ func TestExecuteWithFailedInfoRefs(t *testing.T) {
 		{
 			desc:        "request failed",
 			statusCode:  http.StatusForbidden,
-			expectedErr: "Internal API error (403)",
+			expectedErr: "Remote repository is unavailable",
 		}, {
 			desc:            "unexpected response",
 			statusCode:      http.StatusOK,
@@ -108,7 +108,7 @@ func TestExecuteWithFailedReceivePack(t *testing.T) {
 
 	err := cmd.Execute(context.Background())
 	require.Error(t, err)
-	require.Equal(t, "Internal API error (403)", err.Error())
+	require.Equal(t, "Remote repository is unavailable", err.Error())
 }
 
 func setup(t *testing.T, receivePackStatusCode int) (string, io.Reader) {

internal/gitlabnet/git/client.go

@@ -2,32 +2,25 @@ package git
 
 import (
 	"context"
-	"fmt"
 	"io"
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client"
-	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
-	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet"
 )
 
-type Client struct {
-	url     string
-	headers map[string]string
-	client  *client.GitlabNetClient
+var httpClient = &http.Client{
+	Transport: client.NewTransport(client.DefaultTransport()),
 }
 
-func NewClient(cfg *config.Config, url string, headers map[string]string) (*Client, error) {
-	client, err := gitlabnet.GetClient(cfg)
-	if err != nil {
-		return nil, fmt.Errorf("Error creating http client: %v", err)
-	}
+const repoUnavailableErrMsg = "Remote repository is unavailable"
 
-	return &Client{client: client, headers: headers, url: url}, nil
+type Client struct {
+	Url     string
+	Headers map[string]string
 }
 
 func (c *Client) InfoRefs(ctx context.Context, service string) (*http.Response, error) {
-	request, err := http.NewRequestWithContext(ctx, http.MethodGet, c.url+"/info/refs?service="+service, nil)
+	request, err := http.NewRequestWithContext(ctx, http.MethodGet, c.Url+"/info/refs?service="+service, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -36,7 +29,7 @@ func (c *Client) InfoRefs(ctx context.Context, service string) (*http.Response,
 }
 
 func (c *Client) ReceivePack(ctx context.Context, body io.Reader) (*http.Response, error) {
-	request, err := http.NewRequestWithContext(ctx, http.MethodPost, c.url+"/git-receive-pack", body)
+	request, err := http.NewRequestWithContext(ctx, http.MethodPost, c.Url+"/git-receive-pack", body)
 	if err != nil {
 		return nil, err
 	}
@@ -47,10 +40,14 @@ func (c *Client) ReceivePack(ctx context.Context, body io.Reader) (*http.Respons
 }
 
 func (c *Client) do(request *http.Request) (*http.Response, error) {
-
-	for k, v := range c.headers {
+	for k, v := range c.Headers {
 		request.Header.Add(k, v)
 	}
 
-	return c.client.Do(request)
+	response, err := httpClient.Do(request)
+	if err != nil || response.StatusCode >= 400 {
+		return nil, &client.ApiError{repoUnavailableErrMsg}
+	}
+
+	return response, nil
 }

internal/gitlabnet/git/client_test.go

@@ -8,8 +8,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	httpclient "gitlab.com/gitlab-org/gitlab-shell/v14/client"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
-	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
 )
 
 var customHeaders = map[string]string{
@@ -50,6 +50,21 @@ func TestReceivePack(t *testing.T) {
 	require.Equal(t, "git-receive-pack: content", string(body))
 }
 
+func TestFailedHTTPRequest(t *testing.T) {
+	client := &Client{
+		Url:     testserver.StartHttpServer(t, []testserver.TestRequestHandler{}),
+		Headers: customHeaders,
+	}
+
+	response, err := client.InfoRefs(context.Background(), "git-receive-pack")
+	require.Nil(t, response)
+	require.Error(t, err)
+
+	var apiErr *httpclient.ApiError
+	require.ErrorAs(t, err, &apiErr)
+	require.EqualError(t, err, repoUnavailableErrMsg)
+}
+
 func setup(t *testing.T) *Client {
 	requests := []testserver.TestRequestHandler{
 		{
@@ -80,9 +95,10 @@ func setup(t *testing.T) *Client {
 		},
 	}
 
-	url := testserver.StartHttpServer(t, requests)
-	client, err := NewClient(&config.Config{GitlabUrl: url}, url, customHeaders)
-	require.NoError(t, err)
+	client := &Client{
+		Url:     testserver.StartHttpServer(t, requests),
+		Headers: customHeaders,
+	}
 
 	return client
 }