From a02fb7a80264167199c527e4e91da24a9cd55d66 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Wed, 5 Jun 2024 20:25:53 +0200 Subject: [PATCH 1/5] Geo: Replace Git over HTTP calls with Workhorse HTTP endpoint for SSH Contributes to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/762 **Problem** There is a new endpoint for SSH Pull implemented in Workhorse. We want to use it for Geo Git Pull calls. **Solution** * Define a boolean parameter `GeoProxyFetchSSHDirectToPrimary` hidden behind the feature flag. * Make a call to `ssh-upload-pack` endpoint for git pull commands. Changelog: added --- internal/command/githttp/pull.go | 17 +++++++++ internal/command/githttp/pull_test.go | 41 +++++++++++++++++++++ internal/gitlabnet/accessverifier/client.go | 1 + internal/gitlabnet/git/client.go | 10 +++++ internal/gitlabnet/git/client_test.go | 27 ++++++++++++++ 5 files changed, 96 insertions(+) diff --git a/internal/command/githttp/pull.go b/internal/command/githttp/pull.go index bc4a0ad7..79ec6efe 100644 --- a/internal/command/githttp/pull.go +++ b/internal/command/githttp/pull.go @@ -37,6 +37,11 @@ func (c *PullCommand) Execute(ctx context.Context) error { data := c.Response.Payload.Data client := &git.Client{URL: data.PrimaryRepo, Headers: data.RequestHeaders} + // For Git over SSH routing + if data.GeoProxyFetchSSHDirectToPrimary { + return c.requestSSHUploadPack(ctx, client) + } + if err := c.requestInfoRefs(ctx, client); err != nil { return err } @@ -64,6 +69,18 @@ func (c *PullCommand) requestInfoRefs(ctx context.Context, client *git.Client) e return err } +func (c *PullCommand) requestSSHUploadPack(ctx context.Context, client *git.Client) error { + response, err := client.SSHUploadPack(ctx, io.NopCloser(c.ReadWriter.In)) + if err != nil { + return err + } + defer response.Body.Close() + + _, err = io.Copy(c.ReadWriter.Out, response.Body) + + return err +} + func (c *PullCommand) requestUploadPack(ctx context.Context, client *git.Client, geoProxyFetchDirectToPrimaryWithOptions bool) error { pipeReader, pipeWriter := io.Pipe() go c.readFromStdin(pipeWriter, geoProxyFetchDirectToPrimaryWithOptions) diff --git a/internal/command/githttp/pull_test.go b/internal/command/githttp/pull_test.go index b3584d26..1deeb645 100644 --- a/internal/command/githttp/pull_test.go +++ b/internal/command/githttp/pull_test.go @@ -58,6 +58,27 @@ func TestPullExecuteWithDepth(t *testing.T) { require.Equal(t, infoRefsWithoutPrefix, output.String()) } +func TestPullExecuteWithSSHUploadPack(t *testing.T) { + url := setupSSHPull(t, http.StatusOK) + output := &bytes.Buffer{} + input := strings.NewReader(cloneResponse + "0009done\n") + + cmd := &PullCommand{ + Config: &config.Config{GitlabUrl: url}, + ReadWriter: &readwriter.ReadWriter{Out: output, In: input}, + Response: &accessverifier.Response{ + Payload: accessverifier.CustomPayload{ + Data: accessverifier.CustomPayloadData{ + PrimaryRepo: url, GeoProxyFetchDirectToPrimaryWithOptions: true, GeoProxyFetchSSHDirectToPrimary: true, + }, + }, + }, + } + + require.NoError(t, cmd.Execute(context.Background())) + require.Equal(t, "upload-pack-response", output.String()) +} + func TestPullExecuteWithFailedInfoRefs(t *testing.T) { testCases := []struct { desc string @@ -157,3 +178,23 @@ func setupPull(t *testing.T, uploadPackStatusCode int) string { return testserver.StartHttpServer(t, requests) } + +func setupSSHPull(t *testing.T, uploadPackStatusCode int) string { + requests := []testserver.TestRequestHandler{ + { + Path: "/ssh-upload-pack", + Handler: func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + defer r.Body.Close() + + require.True(t, strings.HasSuffix(string(body), "0009done\n")) + + w.Write([]byte("upload-pack-response")) + w.WriteHeader(uploadPackStatusCode) + }, + }, + } + + return testserver.StartHttpServer(t, requests) +} diff --git a/internal/gitlabnet/accessverifier/client.go b/internal/gitlabnet/accessverifier/client.go index 8ab5060a..77b93da9 100644 --- a/internal/gitlabnet/accessverifier/client.go +++ b/internal/gitlabnet/accessverifier/client.go @@ -56,6 +56,7 @@ type CustomPayloadData struct { GeoProxyDirectToPrimary bool `json:"geo_proxy_direct_to_primary"` GeoProxyFetchDirectToPrimary bool `json:"geo_proxy_fetch_direct_to_primary"` GeoProxyFetchDirectToPrimaryWithOptions bool `json:"geo_proxy_fetch_direct_to_primary_with_options"` + GeoProxyFetchSSHDirectToPrimary bool `json:"geo_proxy_fetch_ssh_direct_to_primary"` } // CustomPayload represents a custom payload diff --git a/internal/gitlabnet/git/client.go b/internal/gitlabnet/git/client.go index 35d6a740..55d1b429 100644 --- a/internal/gitlabnet/git/client.go +++ b/internal/gitlabnet/git/client.go @@ -56,6 +56,16 @@ func (c *Client) UploadPack(ctx context.Context, body io.Reader) (*http.Response return c.do(request) } +// SSHUploadPack sends a SSH Git fetch request to the server. +func (c *Client) SSHUploadPack(ctx context.Context, body io.Reader) (*http.Response, error) { + request, err := http.NewRequestWithContext(ctx, http.MethodPost, c.URL+"/ssh-upload-pack", body) + if err != nil { + return nil, err + } + + return c.do(request) +} + func (c *Client) do(request *http.Request) (*http.Response, error) { for k, v := range c.Headers { request.Header.Add(k, v) diff --git a/internal/gitlabnet/git/client_test.go b/internal/gitlabnet/git/client_test.go index 07a3ff18..a54c2759 100644 --- a/internal/gitlabnet/git/client_test.go +++ b/internal/gitlabnet/git/client_test.go @@ -64,6 +64,20 @@ func TestUploadPack(t *testing.T) { require.Equal(t, "git-upload-pack: content", string(body)) } +func TestSSHUploadPack(t *testing.T) { + client := setup(t) + + refsBody := "0032want 0a53e9ddeaddad63ad106860237bbf53411d11a7\n" + response, err := client.SSHUploadPack(context.Background(), bytes.NewReader([]byte(refsBody))) + require.NoError(t, err) + defer response.Body.Close() + + body, err := io.ReadAll(response.Body) + require.NoError(t, err) + + require.Equal(t, "ssh-upload-pack: content", string(body)) +} + func TestFailedHTTPRequest(t *testing.T) { requests := []testserver.TestRequestHandler{ { @@ -166,6 +180,19 @@ func setup(t *testing.T) *Client { w.Write([]byte("git-upload-pack: content")) }, }, + { + Path: "/ssh-upload-pack", + Handler: func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, customHeaders["Authorization"], r.Header.Get("Authorization")) + require.Equal(t, customHeaders["Header-One"], r.Header.Get("Header-One")) + + _, err := io.ReadAll(r.Body) + require.NoError(t, err) + defer r.Body.Close() + + w.Write([]byte("ssh-upload-pack: content")) + }, + }, } client := &Client{ -- GitLab From 54deaab0bedecc3c1f1aebf93c4b9cac5c0c5517 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Tue, 11 Jun 2024 11:34:27 +0200 Subject: [PATCH 2/5] Extract "/ssh-upload-pack" into constant --- internal/gitlabnet/git/client.go | 7 +++++-- internal/gitlabnet/git/client_test.go | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/gitlabnet/git/client.go b/internal/gitlabnet/git/client.go index 55d1b429..6bc0cc0b 100644 --- a/internal/gitlabnet/git/client.go +++ b/internal/gitlabnet/git/client.go @@ -14,7 +14,10 @@ var httpClient = &http.Client{ Transport: client.NewTransport(client.DefaultTransport()), } -const repoUnavailableErrMsg = "Remote repository is unavailable" +const ( + repoUnavailableErrMsg = "Remote repository is unavailable" + sshUploadPackPath = "/ssh-upload-pack" +) // Client represents a client for interacting with Git repositories. type Client struct { @@ -58,7 +61,7 @@ func (c *Client) UploadPack(ctx context.Context, body io.Reader) (*http.Response // SSHUploadPack sends a SSH Git fetch request to the server. func (c *Client) SSHUploadPack(ctx context.Context, body io.Reader) (*http.Response, error) { - request, err := http.NewRequestWithContext(ctx, http.MethodPost, c.URL+"/ssh-upload-pack", body) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, c.URL+sshUploadPackPath, body) if err != nil { return nil, err } diff --git a/internal/gitlabnet/git/client_test.go b/internal/gitlabnet/git/client_test.go index a54c2759..1e9b4539 100644 --- a/internal/gitlabnet/git/client_test.go +++ b/internal/gitlabnet/git/client_test.go @@ -181,7 +181,7 @@ func setup(t *testing.T) *Client { }, }, { - Path: "/ssh-upload-pack", + Path: sshUploadPackPath, Handler: func(w http.ResponseWriter, r *http.Request) { require.Equal(t, customHeaders["Authorization"], r.Header.Get("Authorization")) require.Equal(t, customHeaders["Header-One"], r.Header.Get("Header-One")) -- GitLab From c13b1f48d571383fae0fb0c106a9a5a627f4216b Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Tue, 11 Jun 2024 12:25:47 +0200 Subject: [PATCH 3/5] Pass Git-Protocol to the request --- internal/command/githttp/pull.go | 4 ++++ internal/command/githttp/pull_test.go | 14 +++++++++++++- internal/command/uploadpack/uploadpack.go | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/command/githttp/pull.go b/internal/command/githttp/pull.go index 79ec6efe..2e8894ca 100644 --- a/internal/command/githttp/pull.go +++ b/internal/command/githttp/pull.go @@ -6,6 +6,7 @@ import ( "fmt" "io" + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/commandargs" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/accessverifier" @@ -20,6 +21,7 @@ var uploadPackHttpPrefix = []byte("001e# service=git-upload-pack\n0000") type PullCommand struct { Config *config.Config ReadWriter *readwriter.ReadWriter + Args *commandargs.Shell Response *accessverifier.Response } @@ -39,6 +41,8 @@ func (c *PullCommand) Execute(ctx context.Context) error { // For Git over SSH routing if data.GeoProxyFetchSSHDirectToPrimary { + client.Headers["Git-Protocol"] = c.Args.Env.GitProtocolVersion + return c.requestSSHUploadPack(ctx, client) } diff --git a/internal/command/githttp/pull_test.go b/internal/command/githttp/pull_test.go index 1deeb645..d6ead923 100644 --- a/internal/command/githttp/pull_test.go +++ b/internal/command/githttp/pull_test.go @@ -11,9 +11,11 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver" + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/commandargs" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/accessverifier" + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/sshenv" ) var cloneResponse = `0090want 11d731b83788cd556abea7b465c6bee52d89923c multi_ack_detailed side-band-64k thin-pack ofs-delta deepen-since deepen-not agent=git/2.41.0 @@ -69,10 +71,18 @@ func TestPullExecuteWithSSHUploadPack(t *testing.T) { Response: &accessverifier.Response{ Payload: accessverifier.CustomPayload{ Data: accessverifier.CustomPayloadData{ - PrimaryRepo: url, GeoProxyFetchDirectToPrimaryWithOptions: true, GeoProxyFetchSSHDirectToPrimary: true, + PrimaryRepo: url, + GeoProxyFetchDirectToPrimaryWithOptions: true, + GeoProxyFetchSSHDirectToPrimary: true, + RequestHeaders: map[string]string{"Authorization": "token"}, }, }, }, + Args: &commandargs.Shell{ + Env: sshenv.Env{ + GitProtocolVersion: "version=2", + }, + }, } require.NoError(t, cmd.Execute(context.Background())) @@ -189,6 +199,8 @@ func setupSSHPull(t *testing.T, uploadPackStatusCode int) string { defer r.Body.Close() require.True(t, strings.HasSuffix(string(body), "0009done\n")) + require.Equal(t, r.Header.Get("Git-Protocol"), "version=2") + require.Equal(t, r.Header.Get("Authorization"), "token") w.Write([]byte("upload-pack-response")) w.WriteHeader(uploadPackStatusCode) diff --git a/internal/command/uploadpack/uploadpack.go b/internal/command/uploadpack/uploadpack.go index 4ac80c48..42ceb876 100644 --- a/internal/command/uploadpack/uploadpack.go +++ b/internal/command/uploadpack/uploadpack.go @@ -51,6 +51,7 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) { cmd := githttp.PullCommand{ Config: c.Config, ReadWriter: c.ReadWriter, + Args: c.Args, Response: response, } -- GitLab From 232a02cbb1cc88e0b9bf0234002a93a4d8702632 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Tue, 11 Jun 2024 12:29:29 +0200 Subject: [PATCH 4/5] Add a log entry for new Git over SSH flow --- internal/command/githttp/pull.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/command/githttp/pull.go b/internal/command/githttp/pull.go index 2e8894ca..fcf4803c 100644 --- a/internal/command/githttp/pull.go +++ b/internal/command/githttp/pull.go @@ -12,6 +12,7 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/accessverifier" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/git" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/pktline" + "gitlab.com/gitlab-org/labkit/log" ) const pullService = "git-upload-pack" @@ -41,8 +42,9 @@ func (c *PullCommand) Execute(ctx context.Context) error { // For Git over SSH routing if data.GeoProxyFetchSSHDirectToPrimary { - client.Headers["Git-Protocol"] = c.Args.Env.GitProtocolVersion + log.ContextLogger(ctx).Info("Using Git over SSH upload pack") + client.Headers["Git-Protocol"] = c.Args.Env.GitProtocolVersion return c.requestSSHUploadPack(ctx, client) } -- GitLab From a0abf5675213869bbd01f6309d07a2114b0952f6 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Wed, 12 Jun 2024 01:33:08 +0000 Subject: [PATCH 5/5] Reverse assertion value order --- internal/command/githttp/pull_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/command/githttp/pull_test.go b/internal/command/githttp/pull_test.go index d6ead923..3e9fcf90 100644 --- a/internal/command/githttp/pull_test.go +++ b/internal/command/githttp/pull_test.go @@ -199,8 +199,8 @@ func setupSSHPull(t *testing.T, uploadPackStatusCode int) string { defer r.Body.Close() require.True(t, strings.HasSuffix(string(body), "0009done\n")) - require.Equal(t, r.Header.Get("Git-Protocol"), "version=2") - require.Equal(t, r.Header.Get("Authorization"), "token") + require.Equal(t, "version=2", r.Header.Get("Git-Protocol")) + require.Equal(t, "token", r.Header.Get("Authorization")) w.Write([]byte("upload-pack-response")) w.WriteHeader(uploadPackStatusCode) -- GitLab