From 80d253e00169df34964ca1c1f6227b32bc7b5c65 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer <jacob@gitlab.com> Date: Thu, 30 Mar 2017 18:29:15 +0200 Subject: [PATCH] Allow passing pb.Repository from Rails to Gitaly We are in the middle of making the Gitaly 'Repository' message contain more fields than it used to. This change makes sure that workhorse automatically passes through the new message format (as long as the gitaly client versions in gitlab-rails and workhorse match). --- internal/api/api.go | 12 ++++++++ internal/git/info-refs.go | 2 +- internal/gitaly/smarthttp.go | 5 ++-- main_test.go | 55 ++++++++++++++++++++++-------------- 4 files changed, 49 insertions(+), 25 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 09baa172be9ab..9d39ba83abf1b 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/prometheus/client_golang/prometheus" + pb "gitlab.com/gitlab-org/gitaly-proto/go" "gitlab.com/gitlab-org/gitlab-workhorse/internal/badgateway" "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" @@ -90,6 +91,9 @@ type Response struct { Terminal *TerminalSettings // Path to Gitaly Socket GitalySocketPath string + // Repository object for making gRPC requests to Gitaly. This will + // eventually replace the RepoPath field. + Repository pb.Repository } // singleJoiningSlash is taken from reverseproxy.go:NewSingleHostReverseProxy @@ -209,6 +213,14 @@ func (api *API) PreAuthorize(suffix string, r *http.Request) (httpResponse *http return httpResponse, nil, fmt.Errorf("preAuthorizeHandler: decode authorization response: %v", err) } + // This is for backwards compatiblity, can be depracated when Rails + // always sends a 'Repository' message. For the time being we cannot + // count on this, so we put some minimal data in the Repository struct. + + if authResponse.Repository.Path == "" { + authResponse.Repository.Path = authResponse.RepoPath + } + return httpResponse, authResponse, nil } diff --git a/internal/git/info-refs.go b/internal/git/info-refs.go index 05efe1438805a..83b33efc36ac7 100644 --- a/internal/git/info-refs.go +++ b/internal/git/info-refs.go @@ -67,7 +67,7 @@ func handleGetInfoRefsWithGitaly(w http.ResponseWriter, a *api.Response, rpc str return fmt.Errorf("GetInfoRefsHandler: %v", err) } - infoRefsResponseWriter, err := smarthttp.InfoRefsResponseWriterTo(a.RepoPath, rpc) + infoRefsResponseWriter, err := smarthttp.InfoRefsResponseWriterTo(a.Repository, rpc) if err != nil { return fmt.Errorf("GetInfoRefsHandler: %v", err) } diff --git a/internal/gitaly/smarthttp.go b/internal/gitaly/smarthttp.go index 9e9dfebdcfbbd..6da70b994a16e 100644 --- a/internal/gitaly/smarthttp.go +++ b/internal/gitaly/smarthttp.go @@ -13,9 +13,8 @@ type SmartHTTPClient struct { pb.SmartHTTPClient } -func (client *SmartHTTPClient) InfoRefsResponseWriterTo(repoPath, rpc string) (io.WriterTo, error) { - repo := &pb.Repository{Path: repoPath} - rpcRequest := &pb.InfoRefsRequest{Repository: repo} +func (client *SmartHTTPClient) InfoRefsResponseWriterTo(repo pb.Repository, rpc string) (io.WriterTo, error) { + rpcRequest := &pb.InfoRefsRequest{Repository: &repo} var c pbhelper.InfoRefsClient var err error diff --git a/main_test.go b/main_test.go index 9545abda33fd4..a20248234ac73 100644 --- a/main_test.go +++ b/main_test.go @@ -594,35 +594,48 @@ func TestApiContentTypeBlock(t *testing.T) { } func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) { - apiResponse := gitOkBody(t) - gitalyServer := startGitalyServer(t) defer func() { gitalyServer.Stop() gitaly.CloseConnections() }() - apiResponse.GitalySocketPath = gitalySocketPath - ts := testAuthServer(nil, 200, apiResponse) - defer ts.Close() - - ws := startWorkhorseServer(ts.URL) - defer ws.Close() + apiResponse := gitOkBody(t) + repoPath := apiResponse.RepoPath - resource := "/gitlab-org/gitlab-test.git/info/refs?service=git-upload-pack" - resp, err := http.Get(ws.URL + resource) - if err != nil { - t.Fatal(err) - } - defer resp.Body.Close() - responseBody, err := ioutil.ReadAll(resp.Body) - if err != nil { - t.Error(err) - } + for _, testCase := range []struct { + repoPath string + repository pb.Repository + }{ + {repoPath: repoPath}, + {repoPath: repoPath, repository: pb.Repository{Path: repoPath, StorageName: "foobar", RelativePath: "baz.git"}}, + } { + func() { + apiResponse.RepoPath = testCase.repoPath + apiResponse.Repository = testCase.repository + apiResponse.GitalySocketPath = gitalySocketPath + ts := testAuthServer(nil, 200, apiResponse) + defer ts.Close() + + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + resource := "/gitlab-org/gitlab-test.git/info/refs?service=git-upload-pack" + resp, err := http.Get(ws.URL + resource) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + responseBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Error(err) + } - expectedContent := testhelper.GitalyInfoRefsResponseMock - if !bytes.Equal(responseBody, []byte(expectedContent)) { - t.Errorf("GET %q: Expected %q, got %q", resource, expectedContent, responseBody) + expectedContent := testhelper.GitalyInfoRefsResponseMock + if !bytes.Equal(responseBody, []byte(expectedContent)) { + t.Errorf("GET %q: Expected %q, got %q", resource, expectedContent, responseBody) + } + }() } } -- GitLab