From ab8dcd888bc500265768b36bfe54414a6e0d6651 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Mon, 30 Jan 2023 20:12:29 -0700 Subject: [PATCH 01/15] add streamio package for gRPC byte streams --- internal/grpc/streamio/streamio.go | 128 +++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 internal/grpc/streamio/streamio.go diff --git a/internal/grpc/streamio/streamio.go b/internal/grpc/streamio/streamio.go new file mode 100644 index 000000000000..e0998ce411df --- /dev/null +++ b/internal/grpc/streamio/streamio.go @@ -0,0 +1,128 @@ +// Package streamio contains wrappers intended for turning gRPC streams +// that send/receive messages with a []byte field into io.Writers and +// io.Readers. +// +// This file is largely copied from the gitaly project, which is licensed +// under the MIT license. A copy of that license text can be found at +// https://mit-license.org/. The code this file was based off can be found +// at https://gitlab.com/gitlab-org/gitaly/-/blob/v1.87.0/streamio/stream.go +package streamio + +import "io" + +// NewReader turns receiver into an io.Reader. Errors from the receiver +// function are passed on unmodified. This means receiver should emit +// io.EOF when done. +func NewReader(receiver func() ([]byte, error)) io.Reader { + return &receiveReader{receiver: receiver} +} + +type receiveReader struct { + receiver func() ([]byte, error) + data []byte + err error +} + +func (rr *receiveReader) Read(p []byte) (int, error) { + if len(rr.data) == 0 { + rr.data, rr.err = rr.receiver() + } + n := copy(p, rr.data) + rr.data = rr.data[n:] + if len(rr.data) == 0 { + return n, rr.err + } + return n, nil +} + +// WriteTo implements io.WriterTo. +func (rr *receiveReader) WriteTo(w io.Writer) (int64, error) { + var written int64 + + // Deal with left-over state in rr.data and rr.err, if any + if len(rr.data) > 0 { + n, err := w.Write(rr.data) + written += int64(n) + if err != nil { + return written, err + } + } + if rr.err != nil { + return written, rr.err + } + + // Consume the response stream + var errRead, errWrite error + var n int + var buf []byte + for errWrite == nil && errRead != io.EOF { + buf, errRead = rr.receiver() + if errRead != nil && errRead != io.EOF { + return written, errRead + } + + if len(buf) > 0 { + n, errWrite = w.Write(buf) + written += int64(n) + } + } + + return written, errWrite +} + +// NewWriter turns sender into an io.Writer. The sender callback will +// receive []byte arguments of length at most WriteBufferSize. +func NewWriter(sender func(p []byte) error) io.Writer { + return &sendWriter{sender: sender} +} + +// WriteBufferSize is the largest []byte that Write() will pass to its +// underlying send function. +var WriteBufferSize = 128 * 1024 + +type sendWriter struct { + sender func([]byte) error +} + +func (sw *sendWriter) Write(p []byte) (int, error) { + var sent int + + for len(p) > 0 { + chunkSize := len(p) + if chunkSize > WriteBufferSize { + chunkSize = WriteBufferSize + } + + if err := sw.sender(p[:chunkSize]); err != nil { + return sent, err + } + + sent += chunkSize + p = p[chunkSize:] + } + + return sent, nil +} + +// ReadFrom implements io.ReaderFrom. +func (sw *sendWriter) ReadFrom(r io.Reader) (int64, error) { + var nRead int64 + buf := make([]byte, WriteBufferSize) + + var errRead, errSend error + for errSend == nil && errRead != io.EOF { + var n int + + n, errRead = r.Read(buf) + nRead += int64(n) + if errRead != nil && errRead != io.EOF { + return nRead, errRead + } + + if n > 0 { + errSend = sw.sender(buf[:n]) + } + } + + return nRead, errSend +} From fde9ec9aab2f81fed02196838616989c3c30ee78 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Mon, 30 Jan 2023 20:20:14 -0700 Subject: [PATCH 02/15] add tests --- internal/grpc/streamio/streamio_test.go | 149 ++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 internal/grpc/streamio/streamio_test.go diff --git a/internal/grpc/streamio/streamio_test.go b/internal/grpc/streamio/streamio_test.go new file mode 100644 index 000000000000..a45de1106385 --- /dev/null +++ b/internal/grpc/streamio/streamio_test.go @@ -0,0 +1,149 @@ +// This file is largely copied from the gitaly project, which is licensed +// under the MIT license. A copy of that license text can be found at +// https://mit-license.org/. The code this file was based off can be found at +// https://gitlab.com/gitlab-org/gitaly/-/blob/v1.87.0/streamio/stream_test.go +package streamio + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "strings" + "testing" + "testing/iotest" + + "github.com/stretchr/testify/require" +) + +func TestReceiveSources(t *testing.T) { + testData := "Hello this is the test data that will be received" + testCases := []struct { + desc string + r io.Reader + }{ + {desc: "base", r: strings.NewReader(testData)}, + {desc: "dataerr", r: iotest.DataErrReader(strings.NewReader(testData))}, + {desc: "onebyte", r: iotest.OneByteReader(strings.NewReader(testData))}, + {desc: "dataerr(onebyte)", r: iotest.DataErrReader(iotest.OneByteReader(strings.NewReader(testData)))}, + } + + for _, tc := range testCases { + data, err := ioutil.ReadAll(&opaqueReader{NewReader(receiverFromReader(tc.r))}) + require.NoError(t, err, tc.desc) + require.Equal(t, testData, string(data), tc.desc) + } +} + +func TestReadSizes(t *testing.T) { + testData := "Hello this is the test data that will be received. It goes on for a while bla bla bla." + for n := 1; n < 100; n *= 3 { + desc := fmt.Sprintf("reads of size %d", n) + result := &bytes.Buffer{} + reader := &opaqueReader{NewReader(receiverFromReader(strings.NewReader(testData)))} + _, err := io.CopyBuffer(&opaqueWriter{result}, reader, make([]byte, n)) + + require.NoError(t, err, desc) + require.Equal(t, testData, result.String()) + } +} + +func TestWriterTo(t *testing.T) { + testData := "Hello this is the test data that will be received. It goes on for a while bla bla bla." + testCases := []struct { + desc string + r io.Reader + }{ + {desc: "base", r: strings.NewReader(testData)}, + {desc: "dataerr", r: iotest.DataErrReader(strings.NewReader(testData))}, + {desc: "onebyte", r: iotest.OneByteReader(strings.NewReader(testData))}, + {desc: "dataerr(onebyte)", r: iotest.DataErrReader(iotest.OneByteReader(strings.NewReader(testData)))}, + } + + for _, tc := range testCases { + result := &bytes.Buffer{} + reader := NewReader(receiverFromReader(tc.r)) + n, err := reader.(io.WriterTo).WriteTo(result) + + require.NoError(t, err, tc.desc) + require.Equal(t, int64(len(testData)), n, tc.desc) + require.Equal(t, testData, result.String(), tc.desc) + } +} + +func receiverFromReader(r io.Reader) func() ([]byte, error) { + return func() ([]byte, error) { + data := make([]byte, 10) + n, err := r.Read(data) + return data[:n], err + } +} + +// Hide io.WriteTo if it exists +type opaqueReader struct { + io.Reader +} + +// Hide io.ReadFrom if it exists +type opaqueWriter struct { + io.Writer +} + +func TestWriterChunking(t *testing.T) { + defer func(oldBufferSize int) { + WriteBufferSize = oldBufferSize + }(WriteBufferSize) + WriteBufferSize = 5 + + testData := "Hello this is some test data" + ts := &testSender{} + w := NewWriter(ts.send) + _, err := io.CopyBuffer(&opaqueWriter{w}, strings.NewReader(testData), make([]byte, 10)) + + require.NoError(t, err) + require.Equal(t, testData, string(bytes.Join(ts.sends, nil))) + for _, send := range ts.sends { + require.True(t, len(send) <= WriteBufferSize, "send calls may not exceed WriteBufferSize") + } +} + +type testSender struct { + sends [][]byte +} + +func (ts *testSender) send(p []byte) error { + buf := make([]byte, len(p)) + copy(buf, p) + ts.sends = append(ts.sends, buf) + return nil +} + +func TestReadFrom(t *testing.T) { + defer func(oldBufferSize int) { + WriteBufferSize = oldBufferSize + }(WriteBufferSize) + WriteBufferSize = 5 + + testData := "Hello this is the test data that will be received. It goes on for a while bla bla bla." + testCases := []struct { + desc string + r io.Reader + }{ + {desc: "base", r: strings.NewReader(testData)}, + {desc: "dataerr", r: iotest.DataErrReader(strings.NewReader(testData))}, + {desc: "onebyte", r: iotest.OneByteReader(strings.NewReader(testData))}, + {desc: "dataerr(onebyte)", r: iotest.DataErrReader(iotest.OneByteReader(strings.NewReader(testData)))}, + } + + for _, tc := range testCases { + ts := &testSender{} + n, err := NewWriter(ts.send).(io.ReaderFrom).ReadFrom(tc.r) + + require.NoError(t, err, tc.desc) + require.Equal(t, int64(len(testData)), n, tc.desc) + require.Equal(t, testData, string(bytes.Join(ts.sends, nil)), tc.desc) + for _, send := range ts.sends { + require.True(t, len(send) <= WriteBufferSize, "send calls may not exceed WriteBufferSize") + } + } +} From 91039c1526bff079eec27adabae60d2f4426ee58 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Tue, 31 Jan 2023 08:49:48 -0700 Subject: [PATCH 03/15] remove deprecated ioutil --- internal/grpc/streamio/streamio_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/grpc/streamio/streamio_test.go b/internal/grpc/streamio/streamio_test.go index a45de1106385..8ebf86dd4d5c 100644 --- a/internal/grpc/streamio/streamio_test.go +++ b/internal/grpc/streamio/streamio_test.go @@ -8,7 +8,6 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "strings" "testing" "testing/iotest" @@ -29,7 +28,7 @@ func TestReceiveSources(t *testing.T) { } for _, tc := range testCases { - data, err := ioutil.ReadAll(&opaqueReader{NewReader(receiverFromReader(tc.r))}) + data, err := io.ReadAll(&opaqueReader{NewReader(receiverFromReader(tc.r))}) require.NoError(t, err, tc.desc) require.Equal(t, testData, string(data), tc.desc) } From 12fca259db8960a2c18f8977ceda3c62a8b0dc58 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Wed, 1 Feb 2023 14:27:28 -0700 Subject: [PATCH 04/15] wip --- cmd/gitserver/server/server_grpc.go | 67 +++ cmd/gitserver/shared/shared.go | 12 + internal/debugserver/grpcui.go | 7 +- internal/gitserver/client.go | 150 +++++-- internal/gitserver/git_command.go | 59 ++- internal/gitserver/proto/buf.gen.yaml | 11 + internal/gitserver/proto/gitserver.pb.go | 410 ++++++++++++++++++ internal/gitserver/proto/gitserver.proto | 32 ++ internal/gitserver/proto/gitserver_grpc.pb.go | 132 ++++++ internal/grpc/defaults/defaults.go | 2 + .../own/codeowners/proto/codeowners.pb.go | 4 +- internal/search/searcher/client_grpc.go | 9 +- internal/symbols/client.go | 10 +- 13 files changed, 823 insertions(+), 82 deletions(-) create mode 100644 cmd/gitserver/server/server_grpc.go create mode 100644 internal/gitserver/proto/buf.gen.yaml create mode 100644 internal/gitserver/proto/gitserver.pb.go create mode 100644 internal/gitserver/proto/gitserver.proto create mode 100644 internal/gitserver/proto/gitserver_grpc.pb.go diff --git a/cmd/gitserver/server/server_grpc.go b/cmd/gitserver/server/server_grpc.go new file mode 100644 index 000000000000..122876978c21 --- /dev/null +++ b/cmd/gitserver/server/server_grpc.go @@ -0,0 +1,67 @@ +package server + +import ( + "github.com/sourcegraph/sourcegraph/internal/api" + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" + "github.com/sourcegraph/sourcegraph/internal/gitserver/protocol" + "github.com/sourcegraph/sourcegraph/internal/grpc/streamio" + "github.com/sourcegraph/sourcegraph/lib/errors" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +type GRPCServer struct { + Server *Server + proto.UnimplementedGitserverServiceServer +} + +func (gs *GRPCServer) Exec(req *proto.ExecRequest, ss proto.GitserverService_ExecServer) error { + internalReq := protocol.ExecRequest{ + Repo: api.RepoName(req.GetRepo()), + EnsureRevision: req.GetEnsureRevision(), + Args: req.GetArgs(), + Stdin: req.GetStdin(), + NoTimeout: req.GetNoTimeout(), + } + + w := streamio.NewWriter(func(p []byte) error { + return ss.Send(&proto.ExecResponse{ + Data: p, + }) + }) + + // TODO(camdencheek): set user agent from all grpc clients + execStatus, err := gs.Server.exec(ss.Context(), gs.Server.Logger, &internalReq, "unknown-grpc-client", w) + if err != nil { + if v := (&NotFoundError{}); errors.As(err, &v) { + s, err := status.New(codes.NotFound, "repo not found").WithDetails(&proto.NotFoundPayload{ + Repo: req.GetRepo(), + CloneInProgress: v.Payload.CloneInProgress, + CloneProgress: v.Payload.CloneProgress, + }) + if err != nil { + // TODO should we really panic here? + panic(err) + } + return s.Err() + + } else if errors.Is(err, ErrInvalidCommand) { + return status.New(codes.InvalidArgument, "invalid command").Err() + } + + return err + } + + if execStatus.ExitStatus != 0 || execStatus.Err != nil { + s, err := status.New(codes.Unknown, execStatus.Err.Error()).WithDetails(&proto.ExecStatusPayload{ + StatusCode: int32(execStatus.ExitStatus), + Stderr: execStatus.Stderr, + }) + if err != nil { + panic(err) + } + return s.Err() + } + + return nil +} diff --git a/cmd/gitserver/shared/shared.go b/cmd/gitserver/shared/shared.go index a705556ba7be..8b91c209d1b9 100644 --- a/cmd/gitserver/shared/shared.go +++ b/cmd/gitserver/shared/shared.go @@ -19,6 +19,8 @@ import ( "github.com/tidwall/gjson" "golang.org/x/sync/semaphore" "golang.org/x/time/rate" + "google.golang.org/grpc" + "google.golang.org/grpc/reflection" "github.com/sourcegraph/sourcegraph/cmd/gitserver/server" "github.com/sourcegraph/sourcegraph/internal/actor" @@ -38,7 +40,10 @@ import ( "github.com/sourcegraph/sourcegraph/internal/extsvc/npm" "github.com/sourcegraph/sourcegraph/internal/extsvc/pypi" "github.com/sourcegraph/sourcegraph/internal/extsvc/rubygems" + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" "github.com/sourcegraph/sourcegraph/internal/goroutine" + internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "github.com/sourcegraph/sourcegraph/internal/hostname" "github.com/sourcegraph/sourcegraph/internal/httpcli" "github.com/sourcegraph/sourcegraph/internal/instrumentation" @@ -142,6 +147,12 @@ func Main(ctx context.Context, observationCtx *observation.Context, ready servic GlobalBatchLogSemaphore: semaphore.NewWeighted(int64(batchLogGlobalConcurrencyLimit)), } + grpcServer := grpc.NewServer(defaults.ServerOptions(logger)...) + grpcServer.RegisterService(&proto.GitserverService_ServiceDesc, &server.GRPCServer{ + Server: &gitserver, + }) + reflection.Register(grpcServer) + gitserver.RegisterMetrics(observationCtx, db) if tmpDir, err := gitserver.SetupAndClearTmp(); err != nil { @@ -159,6 +170,7 @@ func Main(ctx context.Context, observationCtx *observation.Context, ready servic handler = requestclient.HTTPMiddleware(handler) handler = trace.HTTPMiddleware(logger, handler, conf.DefaultClient()) handler = instrumentation.HTTPMiddleware("", handler) + handler = internalgrpc.MultiplexHandlers(grpcServer, handler) ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/internal/debugserver/grpcui.go b/internal/debugserver/grpcui.go index 68f40e46a5cc..da1e3420ed0e 100644 --- a/internal/debugserver/grpcui.go +++ b/internal/debugserver/grpcui.go @@ -8,7 +8,6 @@ import ( "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "github.com/sourcegraph/sourcegraph/lib/errors" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" ) const gRPCWebUIPath = "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/debug/grpcui" @@ -16,13 +15,9 @@ const gRPCWebUIPath = "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/debug/grpcui" // NewGRPCWebUIEndpoint returns a new Endpoint that serves a gRPC Web UI instance // that targets the gRPC server specified by target. func NewGRPCWebUIEndpoint(target string) Endpoint { - var opts []grpc.DialOption - opts = append(opts, defaults.DialOptions()...) - opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) - var handler http.Handler = &grpcHandler{ target: target, - dialOpts: opts, + dialOpts: defaults.DialOptions(), } // gRPC Web UI expects to serve all of its resources diff --git a/internal/gitserver/client.go b/internal/gitserver/client.go index 47079c912568..3f4261ae1e92 100644 --- a/internal/gitserver/client.go +++ b/internal/gitserver/client.go @@ -27,6 +27,8 @@ import ( "go.opentelemetry.io/otel/attribute" "golang.org/x/sync/errgroup" "golang.org/x/sync/semaphore" + "google.golang.org/grpc" + "google.golang.org/grpc/status" sglog "github.com/sourcegraph/log" @@ -36,8 +38,12 @@ import ( "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/authz" "github.com/sourcegraph/sourcegraph/internal/conf" + "github.com/sourcegraph/sourcegraph/internal/featureflag" "github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain" + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" "github.com/sourcegraph/sourcegraph/internal/gitserver/protocol" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" + "github.com/sourcegraph/sourcegraph/internal/grpc/streamio" "github.com/sourcegraph/sourcegraph/internal/httpcli" "github.com/sourcegraph/sourcegraph/internal/lazyregexp" "github.com/sourcegraph/sourcegraph/internal/observation" @@ -557,7 +563,7 @@ type badRequestError struct{ error } func (e badRequestError) BadRequest() bool { return true } -func (c *RemoteGitCommand) sendExec(ctx context.Context) (_ io.ReadCloser, _ http.Header, errRes error) { +func (c *RemoteGitCommand) sendExec(ctx context.Context) (_ io.ReadCloser, errRes error) { repoName := protocol.NormalizeRepo(c.repo) span, ctx := ot.StartSpanFromContext(ctx, "Client.sendExec") //nolint:staticcheck // OT is deprecated @@ -575,38 +581,132 @@ func (c *RemoteGitCommand) sendExec(ctx context.Context) (_ io.ReadCloser, _ htt // Check that ctx is not expired. if err := ctx.Err(); err != nil { deadlineExceededCounter.Inc() - return nil, nil, err + return nil, err } - req := &protocol.ExecRequest{ - Repo: repoName, - EnsureRevision: c.EnsureRevision(), - Args: c.args[1:], - Stdin: c.stdin, - NoTimeout: c.noTimeout, + if featureflag.FromContext(ctx).GetBoolOr("grpc", false) { + req := &proto.ExecRequest{ + Repo: string(repoName), + EnsureRevision: c.EnsureRevision(), + Args: c.args[1:], + Stdin: c.stdin, + NoTimeout: c.noTimeout, + } + addr, err := c.execer.AddrForRepo(ctx, repoName) + if err != nil { + return nil, err + } + + println(addr) + conn, err := grpc.DialContext(ctx, addr, defaults.DialOptions()...) + if err != nil { + return nil, err + } + + client := proto.NewGitserverServiceClient(conn) + stream, err := client.Exec(ctx, req) + if err != nil { + return nil, err + } + r := streamio.NewReader(func() ([]byte, error) { + msg, err := stream.Recv() + if err != nil { + + return nil, err + } + return msg.GetData(), nil + }) + + return &readCloseWrapper{r: r, closeFn: conn.Close}, err + + } else { + req := &protocol.ExecRequest{ + Repo: repoName, + EnsureRevision: c.EnsureRevision(), + Args: c.args[1:], + Stdin: c.stdin, + NoTimeout: c.noTimeout, + } + resp, err := c.execer.httpPost(ctx, repoName, "exec", req) + if err != nil { + return nil, err + } + + switch resp.StatusCode { + case http.StatusOK: + return &cmdReader{rc: resp.Body, trailer: resp.Trailer}, nil + + case http.StatusNotFound: + var payload protocol.NotFoundPayload + if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil { + resp.Body.Close() + return nil, err + } + resp.Body.Close() + return nil, &gitdomain.RepoNotExistError{Repo: repoName, CloneInProgress: payload.CloneInProgress, CloneProgress: payload.CloneProgress} + + default: + resp.Body.Close() + return nil, errors.Errorf("unexpected status code: %d", resp.StatusCode) + } } - resp, err := c.execFn(ctx, repoName, "exec", req) +} + +type readCloseWrapper struct { + r io.Reader + closeFn func() error +} + +func (r *readCloseWrapper) Read(p []byte) (int, error) { + n, err := r.r.Read(p) if err != nil { - return nil, nil, err + st, ok := status.FromError(err) + if !ok { + return n, err + } + + for _, detail := range st.Details() { + switch payload := detail.(type) { + case *proto.ExecStatusPayload: + return n, &CommandStatusError{ + Message: st.Message(), + Stderr: payload.Stderr, + StatusCode: payload.StatusCode, + } + case *proto.NotFoundPayload: + return n, &gitdomain.RepoNotExistError{ + Repo: api.RepoName(payload.Repo), + CloneInProgress: payload.CloneInProgress, + CloneProgress: payload.CloneProgress, + } + } + } } + return n, err +} - switch resp.StatusCode { - case http.StatusOK: - return resp.Body, resp.Trailer, nil +func (r *readCloseWrapper) Close() error { + return r.closeFn() +} - case http.StatusNotFound: - var payload protocol.NotFoundPayload - if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil { - resp.Body.Close() - return nil, nil, err - } - resp.Body.Close() - return nil, nil, &gitdomain.RepoNotExistError{Repo: repoName, CloneInProgress: payload.CloneInProgress, CloneProgress: payload.CloneProgress} +type CommandStatusError struct { + Message string + StatusCode int32 + Stderr string +} - default: - resp.Body.Close() - return nil, nil, errors.Errorf("unexpected status code: %d", resp.StatusCode) +func (c *CommandStatusError) Error() string { + stderr := c.Stderr + if len(stderr) > 100 { + stderr = stderr[:100] + "... (truncated)" + } + if c.Message != "" { + return fmt.Sprintf("%s (stderr: %q)", c.Message, stderr) + } + if c.StatusCode != 0 { + return fmt.Sprintf("non-zero exit status: %d (stderr: %q)", c.StatusCode, stderr) } + return stderr } func (c *clientImplementor) Search(ctx context.Context, args *protocol.SearchRequest, onMatches func([]protocol.CommitMatch)) (limitHit bool, err error) { @@ -878,7 +978,7 @@ func (c *clientImplementor) gitCommand(repo api.RepoName, arg ...string) GitComm } return &RemoteGitCommand{ repo: repo, - execFn: c.httpPost, + execer: c, args: append([]string{git}, arg...), } } diff --git a/internal/gitserver/git_command.go b/internal/gitserver/git_command.go index 183b35af714b..21617e2d1da3 100644 --- a/internal/gitserver/git_command.go +++ b/internal/gitserver/git_command.go @@ -168,35 +168,36 @@ type RemoteGitCommand struct { stdin []byte noTimeout bool exitStatus int - execFn func(ctx context.Context, repo api.RepoName, op string, payload any) (resp *http.Response, err error) + execer execer +} + +type execer interface { + httpPost(ctx context.Context, repo api.RepoName, op string, payload any) (resp *http.Response, err error) + AddrForRepo(ctx context.Context, repo api.RepoName) (string, error) } // DividedOutput runs the command and returns its standard output and standard error. func (c *RemoteGitCommand) DividedOutput(ctx context.Context) ([]byte, []byte, error) { - rc, trailer, err := c.sendExec(ctx) + rc, err := c.sendExec(ctx) if err != nil { return nil, nil, err } + defer rc.Close() stdout, err := io.ReadAll(rc) if err != nil { + if v := (&CommandStatusError{}); errors.As(err, &v) { + c.exitStatus = int(v.StatusCode) + if v.Message != "" { + return stdout, []byte(v.Stderr), errors.New(v.Message) + } else { + return stdout, []byte(v.Stderr), nil + } + } return nil, nil, errors.Wrap(err, "reading exec output") } - if err := rc.Close(); err != nil { - return nil, nil, errors.Wrap(err, "closing exec reader") - } - - c.exitStatus, err = strconv.Atoi(trailer.Get("X-Exec-Exit-Status")) - if err != nil { - return nil, nil, err - } - stderr := []byte(trailer.Get("X-Exec-Stderr")) - if errorMsg := trailer.Get("X-Exec-Error"); errorMsg != "" { - return stdout, stderr, errors.New(errorMsg) - } - - return stdout, stderr, nil + return stdout, nil, nil } // Output runs the command and returns its standard output. @@ -233,15 +234,7 @@ func (c *RemoteGitCommand) String() string { return fmt.Sprintf("%q", c.args) } // non-zero return value, Read returns a non io.EOF error. Do not pass in a // started command. func (c *RemoteGitCommand) StdoutReader(ctx context.Context) (io.ReadCloser, error) { - rc, trailer, err := c.sendExec(ctx) - if err != nil { - return nil, err - } - - return &cmdReader{ - rc: rc, - trailer: trailer, - }, nil + return c.sendExec(ctx) } type cmdReader struct { @@ -252,15 +245,15 @@ type cmdReader struct { func (c *cmdReader) Read(p []byte) (int, error) { n, err := c.rc.Read(p) if err == io.EOF { - stderr := c.trailer.Get("X-Exec-Stderr") - if len(stderr) > 100 { - stderr = stderr[:100] + "... (truncated)" - } - if errorMsg := c.trailer.Get("X-Exec-Error"); errorMsg != "" { - return 0, errors.Errorf("%s (stderr: %q)", errorMsg, stderr) + statusCode, err := strconv.Atoi(c.trailer.Get("X-Exec-Exit-Status")) + if err != nil { + return n, errors.Wrap(err, "failed to parse exit status") } - if exitStatus := c.trailer.Get("X-Exec-Exit-Status"); exitStatus != "0" { - return 0, errors.Errorf("non-zero exit status: %s (stderr: %q)", exitStatus, stderr) + + return n, &CommandStatusError{ + Stderr: c.trailer.Get("X-Exec-Stderr"), + StatusCode: int32(statusCode), + Message: c.trailer.Get("X-Exec-Error"), } } return n, err diff --git a/internal/gitserver/proto/buf.gen.yaml b/internal/gitserver/proto/buf.gen.yaml new file mode 100644 index 000000000000..ca493510d2ef --- /dev/null +++ b/internal/gitserver/proto/buf.gen.yaml @@ -0,0 +1,11 @@ +# Configuration file for https://buf.build/, which we use for Protobuf code generation. +version: v1 +plugins: + - plugin: buf.build/protocolbuffers/go + out: . + opt: + - paths=source_relative + - plugin: buf.build/grpc/go + out: . + opt: + - paths=source_relative diff --git a/internal/gitserver/proto/gitserver.pb.go b/internal/gitserver/proto/gitserver.pb.go new file mode 100644 index 000000000000..359759aefcf0 --- /dev/null +++ b/internal/gitserver/proto/gitserver.pb.go @@ -0,0 +1,410 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.28.1 +// protoc (unknown) +// source: gitserver.proto + +package proto + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +type ExecRequest struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + Repo string `protobuf:"bytes,1,opt,name=repo,proto3" json:"repo,omitempty"` + EnsureRevision string `protobuf:"bytes,2,opt,name=ensure_revision,json=ensureRevision,proto3" json:"ensure_revision,omitempty"` + Args []string `protobuf:"bytes,3,rep,name=args,proto3" json:"args,omitempty"` + Stdin []byte `protobuf:"bytes,4,opt,name=stdin,proto3" json:"stdin,omitempty"` + NoTimeout bool `protobuf:"varint,5,opt,name=no_timeout,json=noTimeout,proto3" json:"no_timeout,omitempty"` +} + +func (x *ExecRequest) Reset() { + *x = ExecRequest{} + if protoimpl.UnsafeEnabled { + mi := &file_gitserver_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ExecRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ExecRequest) ProtoMessage() {} + +func (x *ExecRequest) ProtoReflect() protoreflect.Message { + mi := &file_gitserver_proto_msgTypes[0] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ExecRequest.ProtoReflect.Descriptor instead. +func (*ExecRequest) Descriptor() ([]byte, []int) { + return file_gitserver_proto_rawDescGZIP(), []int{0} +} + +func (x *ExecRequest) GetRepo() string { + if x != nil { + return x.Repo + } + return "" +} + +func (x *ExecRequest) GetEnsureRevision() string { + if x != nil { + return x.EnsureRevision + } + return "" +} + +func (x *ExecRequest) GetArgs() []string { + if x != nil { + return x.Args + } + return nil +} + +func (x *ExecRequest) GetStdin() []byte { + if x != nil { + return x.Stdin + } + return nil +} + +func (x *ExecRequest) GetNoTimeout() bool { + if x != nil { + return x.NoTimeout + } + return false +} + +type ExecResponse struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + Data []byte `protobuf:"bytes,1,opt,name=data,proto3" json:"data,omitempty"` +} + +func (x *ExecResponse) Reset() { + *x = ExecResponse{} + if protoimpl.UnsafeEnabled { + mi := &file_gitserver_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ExecResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ExecResponse) ProtoMessage() {} + +func (x *ExecResponse) ProtoReflect() protoreflect.Message { + mi := &file_gitserver_proto_msgTypes[1] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ExecResponse.ProtoReflect.Descriptor instead. +func (*ExecResponse) Descriptor() ([]byte, []int) { + return file_gitserver_proto_rawDescGZIP(), []int{1} +} + +func (x *ExecResponse) GetData() []byte { + if x != nil { + return x.Data + } + return nil +} + +type NotFoundPayload struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + Repo string `protobuf:"bytes,1,opt,name=repo,proto3" json:"repo,omitempty"` + CloneInProgress bool `protobuf:"varint,2,opt,name=clone_in_progress,json=cloneInProgress,proto3" json:"clone_in_progress,omitempty"` + CloneProgress string `protobuf:"bytes,3,opt,name=clone_progress,json=cloneProgress,proto3" json:"clone_progress,omitempty"` +} + +func (x *NotFoundPayload) Reset() { + *x = NotFoundPayload{} + if protoimpl.UnsafeEnabled { + mi := &file_gitserver_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *NotFoundPayload) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*NotFoundPayload) ProtoMessage() {} + +func (x *NotFoundPayload) ProtoReflect() protoreflect.Message { + mi := &file_gitserver_proto_msgTypes[2] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use NotFoundPayload.ProtoReflect.Descriptor instead. +func (*NotFoundPayload) Descriptor() ([]byte, []int) { + return file_gitserver_proto_rawDescGZIP(), []int{2} +} + +func (x *NotFoundPayload) GetRepo() string { + if x != nil { + return x.Repo + } + return "" +} + +func (x *NotFoundPayload) GetCloneInProgress() bool { + if x != nil { + return x.CloneInProgress + } + return false +} + +func (x *NotFoundPayload) GetCloneProgress() string { + if x != nil { + return x.CloneProgress + } + return "" +} + +type ExecStatusPayload struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + StatusCode int32 `protobuf:"varint,1,opt,name=status_code,json=statusCode,proto3" json:"status_code,omitempty"` + Stderr string `protobuf:"bytes,2,opt,name=stderr,proto3" json:"stderr,omitempty"` +} + +func (x *ExecStatusPayload) Reset() { + *x = ExecStatusPayload{} + if protoimpl.UnsafeEnabled { + mi := &file_gitserver_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ExecStatusPayload) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ExecStatusPayload) ProtoMessage() {} + +func (x *ExecStatusPayload) ProtoReflect() protoreflect.Message { + mi := &file_gitserver_proto_msgTypes[3] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ExecStatusPayload.ProtoReflect.Descriptor instead. +func (*ExecStatusPayload) Descriptor() ([]byte, []int) { + return file_gitserver_proto_rawDescGZIP(), []int{3} +} + +func (x *ExecStatusPayload) GetStatusCode() int32 { + if x != nil { + return x.StatusCode + } + return 0 +} + +func (x *ExecStatusPayload) GetStderr() string { + if x != nil { + return x.Stderr + } + return "" +} + +var File_gitserver_proto protoreflect.FileDescriptor + +var file_gitserver_proto_rawDesc = []byte{ + 0x0a, 0x0f, 0x67, 0x69, 0x74, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x2e, 0x70, 0x72, 0x6f, 0x74, + 0x6f, 0x12, 0x09, 0x67, 0x69, 0x74, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x22, 0x93, 0x01, 0x0a, + 0x0b, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x12, 0x0a, 0x04, + 0x72, 0x65, 0x70, 0x6f, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x72, 0x65, 0x70, 0x6f, + 0x12, 0x27, 0x0a, 0x0f, 0x65, 0x6e, 0x73, 0x75, 0x72, 0x65, 0x5f, 0x72, 0x65, 0x76, 0x69, 0x73, + 0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0e, 0x65, 0x6e, 0x73, 0x75, 0x72, + 0x65, 0x52, 0x65, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x12, 0x0a, 0x04, 0x61, 0x72, 0x67, + 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x09, 0x52, 0x04, 0x61, 0x72, 0x67, 0x73, 0x12, 0x14, 0x0a, + 0x05, 0x73, 0x74, 0x64, 0x69, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x05, 0x73, 0x74, + 0x64, 0x69, 0x6e, 0x12, 0x1d, 0x0a, 0x0a, 0x6e, 0x6f, 0x5f, 0x74, 0x69, 0x6d, 0x65, 0x6f, 0x75, + 0x74, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x09, 0x6e, 0x6f, 0x54, 0x69, 0x6d, 0x65, 0x6f, + 0x75, 0x74, 0x22, 0x22, 0x0a, 0x0c, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, + 0x73, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x64, 0x61, 0x74, 0x61, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, + 0x52, 0x04, 0x64, 0x61, 0x74, 0x61, 0x22, 0x78, 0x0a, 0x0f, 0x4e, 0x6f, 0x74, 0x46, 0x6f, 0x75, + 0x6e, 0x64, 0x50, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x12, 0x12, 0x0a, 0x04, 0x72, 0x65, 0x70, + 0x6f, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x72, 0x65, 0x70, 0x6f, 0x12, 0x2a, 0x0a, + 0x11, 0x63, 0x6c, 0x6f, 0x6e, 0x65, 0x5f, 0x69, 0x6e, 0x5f, 0x70, 0x72, 0x6f, 0x67, 0x72, 0x65, + 0x73, 0x73, 0x18, 0x02, 0x20, 0x01, 0x28, 0x08, 0x52, 0x0f, 0x63, 0x6c, 0x6f, 0x6e, 0x65, 0x49, + 0x6e, 0x50, 0x72, 0x6f, 0x67, 0x72, 0x65, 0x73, 0x73, 0x12, 0x25, 0x0a, 0x0e, 0x63, 0x6c, 0x6f, + 0x6e, 0x65, 0x5f, 0x70, 0x72, 0x6f, 0x67, 0x72, 0x65, 0x73, 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x0d, 0x63, 0x6c, 0x6f, 0x6e, 0x65, 0x50, 0x72, 0x6f, 0x67, 0x72, 0x65, 0x73, 0x73, + 0x22, 0x4c, 0x0a, 0x11, 0x45, 0x78, 0x65, 0x63, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x50, 0x61, + 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x12, 0x1f, 0x0a, 0x0b, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x5f, + 0x63, 0x6f, 0x64, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x52, 0x0a, 0x73, 0x74, 0x61, 0x74, + 0x75, 0x73, 0x43, 0x6f, 0x64, 0x65, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x74, 0x64, 0x65, 0x72, 0x72, + 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x73, 0x74, 0x64, 0x65, 0x72, 0x72, 0x32, 0x4f, + 0x0a, 0x10, 0x47, 0x69, 0x74, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x53, 0x65, 0x72, 0x76, 0x69, + 0x63, 0x65, 0x12, 0x3b, 0x0a, 0x04, 0x45, 0x78, 0x65, 0x63, 0x12, 0x16, 0x2e, 0x67, 0x69, 0x74, + 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, + 0x73, 0x74, 0x1a, 0x17, 0x2e, 0x67, 0x69, 0x74, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x2e, 0x45, + 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x30, 0x01, 0x42, + 0x3d, 0x5a, 0x3b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x73, 0x6f, + 0x75, 0x72, 0x63, 0x65, 0x67, 0x72, 0x61, 0x70, 0x68, 0x2f, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, + 0x67, 0x72, 0x61, 0x70, 0x68, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x67, + 0x69, 0x74, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, +} + +var ( + file_gitserver_proto_rawDescOnce sync.Once + file_gitserver_proto_rawDescData = file_gitserver_proto_rawDesc +) + +func file_gitserver_proto_rawDescGZIP() []byte { + file_gitserver_proto_rawDescOnce.Do(func() { + file_gitserver_proto_rawDescData = protoimpl.X.CompressGZIP(file_gitserver_proto_rawDescData) + }) + return file_gitserver_proto_rawDescData +} + +var file_gitserver_proto_msgTypes = make([]protoimpl.MessageInfo, 4) +var file_gitserver_proto_goTypes = []interface{}{ + (*ExecRequest)(nil), // 0: gitserver.ExecRequest + (*ExecResponse)(nil), // 1: gitserver.ExecResponse + (*NotFoundPayload)(nil), // 2: gitserver.NotFoundPayload + (*ExecStatusPayload)(nil), // 3: gitserver.ExecStatusPayload +} +var file_gitserver_proto_depIdxs = []int32{ + 0, // 0: gitserver.GitserverService.Exec:input_type -> gitserver.ExecRequest + 1, // 1: gitserver.GitserverService.Exec:output_type -> gitserver.ExecResponse + 1, // [1:2] is the sub-list for method output_type + 0, // [0:1] is the sub-list for method input_type + 0, // [0:0] is the sub-list for extension type_name + 0, // [0:0] is the sub-list for extension extendee + 0, // [0:0] is the sub-list for field type_name +} + +func init() { file_gitserver_proto_init() } +func file_gitserver_proto_init() { + if File_gitserver_proto != nil { + return + } + if !protoimpl.UnsafeEnabled { + file_gitserver_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ExecRequest); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_gitserver_proto_msgTypes[1].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ExecResponse); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_gitserver_proto_msgTypes[2].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*NotFoundPayload); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_gitserver_proto_msgTypes[3].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ExecStatusPayload); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: file_gitserver_proto_rawDesc, + NumEnums: 0, + NumMessages: 4, + NumExtensions: 0, + NumServices: 1, + }, + GoTypes: file_gitserver_proto_goTypes, + DependencyIndexes: file_gitserver_proto_depIdxs, + MessageInfos: file_gitserver_proto_msgTypes, + }.Build() + File_gitserver_proto = out.File + file_gitserver_proto_rawDesc = nil + file_gitserver_proto_goTypes = nil + file_gitserver_proto_depIdxs = nil +} diff --git a/internal/gitserver/proto/gitserver.proto b/internal/gitserver/proto/gitserver.proto new file mode 100644 index 000000000000..fdd2a2e8f0c3 --- /dev/null +++ b/internal/gitserver/proto/gitserver.proto @@ -0,0 +1,32 @@ +syntax = "proto3"; + +package gitserver; + +option go_package = "github.com/sourcegraph/sourcegraph/internal/gitserver/proto"; + +service GitserverService { + rpc Exec(ExecRequest) returns (stream ExecResponse) {} +} + +message ExecRequest { + string repo = 1; + string ensure_revision = 2; + repeated string args = 3; + bytes stdin = 4; + bool no_timeout = 5; +} + +message ExecResponse { + bytes data = 1; +} + +message NotFoundPayload { + string repo = 1; + bool clone_in_progress = 2; + string clone_progress = 3; +} + +message ExecStatusPayload { + int32 status_code = 1; + string stderr = 2; +} diff --git a/internal/gitserver/proto/gitserver_grpc.pb.go b/internal/gitserver/proto/gitserver_grpc.pb.go new file mode 100644 index 000000000000..537113aa3085 --- /dev/null +++ b/internal/gitserver/proto/gitserver_grpc.pb.go @@ -0,0 +1,132 @@ +// Code generated by protoc-gen-go-grpc. DO NOT EDIT. +// versions: +// - protoc-gen-go-grpc v1.2.0 +// - protoc (unknown) +// source: gitserver.proto + +package proto + +import ( + context "context" + grpc "google.golang.org/grpc" + codes "google.golang.org/grpc/codes" + status "google.golang.org/grpc/status" +) + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the grpc package it is being compiled against. +// Requires gRPC-Go v1.32.0 or later. +const _ = grpc.SupportPackageIsVersion7 + +// GitserverServiceClient is the client API for GitserverService service. +// +// For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. +type GitserverServiceClient interface { + Exec(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (GitserverService_ExecClient, error) +} + +type gitserverServiceClient struct { + cc grpc.ClientConnInterface +} + +func NewGitserverServiceClient(cc grpc.ClientConnInterface) GitserverServiceClient { + return &gitserverServiceClient{cc} +} + +func (c *gitserverServiceClient) Exec(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (GitserverService_ExecClient, error) { + stream, err := c.cc.NewStream(ctx, &GitserverService_ServiceDesc.Streams[0], "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/gitserver.GitserverService/Exec", opts...) + if err != nil { + return nil, err + } + x := &gitserverServiceExecClient{stream} + if err := x.ClientStream.SendMsg(in); err != nil { + return nil, err + } + if err := x.ClientStream.CloseSend(); err != nil { + return nil, err + } + return x, nil +} + +type GitserverService_ExecClient interface { + Recv() (*ExecResponse, error) + grpc.ClientStream +} + +type gitserverServiceExecClient struct { + grpc.ClientStream +} + +func (x *gitserverServiceExecClient) Recv() (*ExecResponse, error) { + m := new(ExecResponse) + if err := x.ClientStream.RecvMsg(m); err != nil { + return nil, err + } + return m, nil +} + +// GitserverServiceServer is the server API for GitserverService service. +// All implementations must embed UnimplementedGitserverServiceServer +// for forward compatibility +type GitserverServiceServer interface { + Exec(*ExecRequest, GitserverService_ExecServer) error + mustEmbedUnimplementedGitserverServiceServer() +} + +// UnimplementedGitserverServiceServer must be embedded to have forward compatible implementations. +type UnimplementedGitserverServiceServer struct { +} + +func (UnimplementedGitserverServiceServer) Exec(*ExecRequest, GitserverService_ExecServer) error { + return status.Errorf(codes.Unimplemented, "method Exec not implemented") +} +func (UnimplementedGitserverServiceServer) mustEmbedUnimplementedGitserverServiceServer() {} + +// UnsafeGitserverServiceServer may be embedded to opt out of forward compatibility for this service. +// Use of this interface is not recommended, as added methods to GitserverServiceServer will +// result in compilation errors. +type UnsafeGitserverServiceServer interface { + mustEmbedUnimplementedGitserverServiceServer() +} + +func RegisterGitserverServiceServer(s grpc.ServiceRegistrar, srv GitserverServiceServer) { + s.RegisterService(&GitserverService_ServiceDesc, srv) +} + +func _GitserverService_Exec_Handler(srv interface{}, stream grpc.ServerStream) error { + m := new(ExecRequest) + if err := stream.RecvMsg(m); err != nil { + return err + } + return srv.(GitserverServiceServer).Exec(m, &gitserverServiceExecServer{stream}) +} + +type GitserverService_ExecServer interface { + Send(*ExecResponse) error + grpc.ServerStream +} + +type gitserverServiceExecServer struct { + grpc.ServerStream +} + +func (x *gitserverServiceExecServer) Send(m *ExecResponse) error { + return x.ServerStream.SendMsg(m) +} + +// GitserverService_ServiceDesc is the grpc.ServiceDesc for GitserverService service. +// It's only intended for direct use with grpc.RegisterService, +// and not to be introspected or modified (even as a copy) +var GitserverService_ServiceDesc = grpc.ServiceDesc{ + ServiceName: "gitserver.GitserverService", + HandlerType: (*GitserverServiceServer)(nil), + Methods: []grpc.MethodDesc{}, + Streams: []grpc.StreamDesc{ + { + StreamName: "Exec", + Handler: _GitserverService_Exec_Handler, + ServerStreams: true, + }, + }, + Metadata: "gitserver.proto", +} diff --git a/internal/grpc/defaults/defaults.go b/internal/grpc/defaults/defaults.go index b34a00a974af..d7442723b341 100644 --- a/internal/grpc/defaults/defaults.go +++ b/internal/grpc/defaults/defaults.go @@ -7,6 +7,7 @@ package defaults import ( "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" "github.com/sourcegraph/log" "github.com/sourcegraph/sourcegraph/internal/actor" @@ -22,6 +23,7 @@ func DialOptions() []grpc.DialOption { // because these options depend on some globals (tracer, trace sampling) // that are not initialized during init time. return []grpc.DialOption{ + grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithChainStreamInterceptor( internalgrpc.StreamClientPropagator(actor.ActorPropagator{}), internalgrpc.StreamClientPropagator(policy.ShouldTracePropagator{}), diff --git a/internal/own/codeowners/proto/codeowners.pb.go b/internal/own/codeowners/proto/codeowners.pb.go index e16b86a8151c..2c8dc526abd9 100644 --- a/internal/own/codeowners/proto/codeowners.pb.go +++ b/internal/own/codeowners/proto/codeowners.pb.go @@ -268,11 +268,11 @@ var file_codeowners_proto_rawDesc = []byte{ 0x6e, 0x65, 0x72, 0x12, 0x16, 0x0a, 0x06, 0x68, 0x61, 0x6e, 0x64, 0x6c, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x68, 0x61, 0x6e, 0x64, 0x6c, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x65, 0x6d, 0x61, 0x69, - 0x6c, 0x42, 0x43, 0x5a, 0x41, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, + 0x6c, 0x42, 0x42, 0x5a, 0x40, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x67, 0x72, 0x61, 0x70, 0x68, 0x2f, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x67, 0x72, 0x61, 0x70, 0x68, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x6f, 0x77, 0x6e, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x6f, 0x77, 0x6e, 0x65, 0x72, 0x73, 0x2f, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/internal/search/searcher/client_grpc.go b/internal/search/searcher/client_grpc.go index a1bdd6c2006d..0ad23decfd1c 100644 --- a/internal/search/searcher/client_grpc.go +++ b/internal/search/searcher/client_grpc.go @@ -12,12 +12,11 @@ import ( "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/endpoint" "github.com/sourcegraph/sourcegraph/internal/errcode" - grpcdefaults "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "github.com/sourcegraph/sourcegraph/internal/search" "github.com/sourcegraph/sourcegraph/internal/searcher/proto" "github.com/sourcegraph/sourcegraph/lib/errors" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" ) // Search searches repo@commit with p. @@ -82,11 +81,7 @@ func SearchGRPC( return false, errors.Wrap(err, "failed to parse URL") } - var opts []grpc.DialOption - opts = append(opts, grpcdefaults.DialOptions()...) - opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) - - clientConn, err := grpc.DialContext(ctx, parsed.Host, opts...) + clientConn, err := grpc.DialContext(ctx, parsed.Host, defaults.DialOptions()...) if err != nil { return false, err } diff --git a/internal/symbols/client.go b/internal/symbols/client.go index 91c48e7168c9..44d623860826 100644 --- a/internal/symbols/client.go +++ b/internal/symbols/client.go @@ -20,7 +20,6 @@ import ( "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "github.com/sourcegraph/sourcegraph/internal/symbols/proto" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" "google.golang.org/protobuf/types/known/emptypb" "github.com/sourcegraph/sourcegraph/internal/actor" @@ -523,14 +522,7 @@ func (c *Client) dialGRPC(ctx context.Context, repository api.RepoName) (*grpc.C return nil, errors.Wrap(err, "parsing symbols service URL") } - opts := []grpc.DialOption{ - // 🚨 SECURITY: We use insecure connections to the symbols service. During the - // grpc prototyping phase - we're leaving TLS authentication out of scope. - grpc.WithTransportCredentials(insecure.NewCredentials()), - } - - opts = append(opts, defaults.DialOptions()...) - conn, err := grpc.DialContext(ctx, u.Host, opts...) + conn, err := grpc.DialContext(ctx, u.Host, defaults.DialOptions()...) if err != nil { return nil, errors.Wrap(err, "dialing symbols GRPC service") } From e239485a662f39c87080eeb21497e1ed967c1f0f Mon Sep 17 00:00:00 2001 From: Geoffrey Gilmore Date: Wed, 1 Feb 2023 14:14:03 -0800 Subject: [PATCH 05/15] git_command.go: properly propagate io.EOF if git command exited cleanly --- internal/gitserver/git_command.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/internal/gitserver/git_command.go b/internal/gitserver/git_command.go index 21617e2d1da3..f0d0d876b948 100644 --- a/internal/gitserver/git_command.go +++ b/internal/gitserver/git_command.go @@ -247,15 +247,30 @@ func (c *cmdReader) Read(p []byte) (int, error) { if err == io.EOF { statusCode, err := strconv.Atoi(c.trailer.Get("X-Exec-Exit-Status")) if err != nil { - return n, errors.Wrap(err, "failed to parse exit status") + return n, errors.Wrap(err, "failed to parse exit status code") } - return n, &CommandStatusError{ - Stderr: c.trailer.Get("X-Exec-Stderr"), + errorMessage := c.trailer.Get("X-Exec-Error") + + // did the command exit cleanly? + if statusCode == 0 && errorMessage == "" { + // yes - propagate io.EOF + + return n, io.EOF + } + + // no - report it + + stderr := c.trailer.Get("X-Exec-Stderr") + err = &CommandStatusError{ + Stderr: stderr, StatusCode: int32(statusCode), - Message: c.trailer.Get("X-Exec-Error"), + Message: errorMessage, } + + return n, err } + return n, err } From acf1b7c4229fda8310871db53173fe28cfe194d1 Mon Sep 17 00:00:00 2001 From: Geoffrey Gilmore Date: Wed, 1 Feb 2023 14:50:03 -0800 Subject: [PATCH 06/15] add grpcUI debug endpoints --- cmd/gitserver/shared/debug.go | 12 +++++++++++ cmd/gitserver/shared/service.go | 7 ++++++- cmd/gitserver/shared/shared.go | 23 +++++++++++++--------- enterprise/cmd/gitserver/shared/service.go | 6 +++++- 4 files changed, 37 insertions(+), 11 deletions(-) create mode 100644 cmd/gitserver/shared/debug.go diff --git a/cmd/gitserver/shared/debug.go b/cmd/gitserver/shared/debug.go new file mode 100644 index 000000000000..aebce98693ea --- /dev/null +++ b/cmd/gitserver/shared/debug.go @@ -0,0 +1,12 @@ +package shared + +import ( + "github.com/sourcegraph/sourcegraph/internal/debugserver" +) + +// GRPCWebUIDebugEndpoint returns a debug endpoint that serves the GRPCWebUI that targets +// this gitserver instance. +func GRPCWebUIDebugEndpoint() debugserver.Endpoint { + addr := getAddr() + return debugserver.NewGRPCWebUIEndpoint(addr) +} diff --git a/cmd/gitserver/shared/service.go b/cmd/gitserver/shared/service.go index 454bbc9b5215..ae777c9a98f3 100644 --- a/cmd/gitserver/shared/service.go +++ b/cmd/gitserver/shared/service.go @@ -14,7 +14,12 @@ type svc struct{} func (svc) Name() string { return "gitserver" } func (svc) Configure() (env.Config, []debugserver.Endpoint) { - return LoadConfig(), nil + c := LoadConfig() + endpoints := []debugserver.Endpoint{ + GRPCWebUIDebugEndpoint(), + } + + return c, endpoints } func (svc) Start(ctx context.Context, observationCtx *observation.Context, ready service.ReadyFunc, config env.Config) error { diff --git a/cmd/gitserver/shared/shared.go b/cmd/gitserver/shared/shared.go index 8b91c209d1b9..148a0911d502 100644 --- a/cmd/gitserver/shared/shared.go +++ b/cmd/gitserver/shared/shared.go @@ -190,15 +190,7 @@ func Main(ctx context.Context, observationCtx *observation.Context, ready servic gitserver.StartClonePipeline(ctx) - addr := os.Getenv("GITSERVER_ADDR") - if addr == "" { - port := "3178" - host := "" - if env.InsecureDev { - host = "127.0.0.1" - } - addr = net.JoinHostPort(host, port) - } + addr := getAddr() srv := &http.Server{ Addr: addr, Handler: handler, @@ -583,3 +575,16 @@ func syncRateLimiters(ctx context.Context, logger log.Logger, store database.Ext } } } + +func getAddr() string { + addr := os.Getenv("GITSERVER_ADDR") + if addr == "" { + port := "3178" + host := "" + if env.InsecureDev { + host = "127.0.0.1" + } + addr = net.JoinHostPort(host, port) + } + return addr +} diff --git a/enterprise/cmd/gitserver/shared/service.go b/enterprise/cmd/gitserver/shared/service.go index 43489fc9b126..bc5ae987d2a7 100644 --- a/enterprise/cmd/gitserver/shared/service.go +++ b/enterprise/cmd/gitserver/shared/service.go @@ -15,7 +15,11 @@ type svc struct{} func (svc) Name() string { return "gitserver" } func (svc) Configure() (env.Config, []debugserver.Endpoint) { - return shared.LoadConfig(), nil + c := shared.LoadConfig() + endpoints := []debugserver.Endpoint{ + shared.GRPCWebUIDebugEndpoint(), + } + return c, endpoints } func (svc) Start(ctx context.Context, observationCtx *observation.Context, ready service.ReadyFunc, config env.Config) error { From 3fd67d83c3b0e2dab63fbefff2e3152ceec403fa Mon Sep 17 00:00:00 2001 From: Geoffrey Gilmore Date: Thu, 2 Feb 2023 08:48:30 -0800 Subject: [PATCH 07/15] sg lint -fix protobuf --- internal/gitserver/proto/gitserver.proto | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/gitserver/proto/gitserver.proto b/internal/gitserver/proto/gitserver.proto index fdd2a2e8f0c3..4209596bba0d 100644 --- a/internal/gitserver/proto/gitserver.proto +++ b/internal/gitserver/proto/gitserver.proto @@ -9,24 +9,24 @@ service GitserverService { } message ExecRequest { - string repo = 1; - string ensure_revision = 2; - repeated string args = 3; - bytes stdin = 4; - bool no_timeout = 5; + string repo = 1; + string ensure_revision = 2; + repeated string args = 3; + bytes stdin = 4; + bool no_timeout = 5; } message ExecResponse { - bytes data = 1; + bytes data = 1; } message NotFoundPayload { - string repo = 1; - bool clone_in_progress = 2; - string clone_progress = 3; + string repo = 1; + bool clone_in_progress = 2; + string clone_progress = 3; } message ExecStatusPayload { - int32 status_code = 1; - string stderr = 2; + int32 status_code = 1; + string stderr = 2; } From 7aa1ef5eed68ec60a3ba395551783958093f9ccd Mon Sep 17 00:00:00 2001 From: Geoffrey Gilmore Date: Thu, 2 Feb 2023 13:28:57 -0800 Subject: [PATCH 08/15] Update client.go --- internal/gitserver/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/gitserver/client.go b/internal/gitserver/client.go index 3f4261ae1e92..988ac5351061 100644 --- a/internal/gitserver/client.go +++ b/internal/gitserver/client.go @@ -597,7 +597,6 @@ func (c *RemoteGitCommand) sendExec(ctx context.Context) (_ io.ReadCloser, errRe return nil, err } - println(addr) conn, err := grpc.DialContext(ctx, addr, defaults.DialOptions()...) if err != nil { return nil, err From 0946e7b8a73f2d136ab5f05b6c226cd068497280 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Tue, 7 Feb 2023 08:59:37 -0700 Subject: [PATCH 09/15] remove unused imports --- internal/symbols/client.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/symbols/client.go b/internal/symbols/client.go index 4680bd7b94f4..38f4a64ff153 100644 --- a/internal/symbols/client.go +++ b/internal/symbols/client.go @@ -17,8 +17,6 @@ import ( otlog "github.com/opentracing/opentracing-go/log" "github.com/sourcegraph/go-ctags" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" - "google.golang.org/protobuf/types/known/emptypb" "github.com/sourcegraph/sourcegraph/internal/actor" "github.com/sourcegraph/sourcegraph/internal/api" From 2f0aabed9a744a424959d0f9b47ef901342b6700 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Tue, 7 Feb 2023 09:07:38 -0700 Subject: [PATCH 10/15] fix imports --- internal/search/searcher/client_grpc.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/search/searcher/client_grpc.go b/internal/search/searcher/client_grpc.go index eb23c97f89c8..85cdde16fd8d 100644 --- a/internal/search/searcher/client_grpc.go +++ b/internal/search/searcher/client_grpc.go @@ -9,17 +9,15 @@ import ( "time" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" "github.com/sourcegraph/sourcegraph/cmd/searcher/protocol" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/endpoint" "github.com/sourcegraph/sourcegraph/internal/errcode" - grpcdefaults "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "github.com/sourcegraph/sourcegraph/internal/search" proto "github.com/sourcegraph/sourcegraph/internal/searcher/v1" "github.com/sourcegraph/sourcegraph/lib/errors" - "google.golang.org/grpc" ) // Search searches repo@commit with p. From 9a95e57cc22ef24731d5da0e7ffe1bd46bb2bc34 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Tue, 7 Feb 2023 09:39:02 -0700 Subject: [PATCH 11/15] log and propagate error --- cmd/gitserver/server/server_grpc.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/gitserver/server/server_grpc.go b/cmd/gitserver/server/server_grpc.go index 122876978c21..51c756cf4df9 100644 --- a/cmd/gitserver/server/server_grpc.go +++ b/cmd/gitserver/server/server_grpc.go @@ -1,6 +1,7 @@ package server import ( + "github.com/sourcegraph/log" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" "github.com/sourcegraph/sourcegraph/internal/gitserver/protocol" @@ -40,8 +41,8 @@ func (gs *GRPCServer) Exec(req *proto.ExecRequest, ss proto.GitserverService_Exe CloneProgress: v.Payload.CloneProgress, }) if err != nil { - // TODO should we really panic here? - panic(err) + gs.Server.Logger.Error("failed to marshal status", log.Error(err)) + return err } return s.Err() @@ -58,7 +59,8 @@ func (gs *GRPCServer) Exec(req *proto.ExecRequest, ss proto.GitserverService_Exe Stderr: execStatus.Stderr, }) if err != nil { - panic(err) + gs.Server.Logger.Error("failed to marshal status", log.Error(err)) + return err } return s.Err() } From 07ea28268ce4048e8fb2d8fdc5c12c8e9cba87ac Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Tue, 7 Feb 2023 09:43:17 -0700 Subject: [PATCH 12/15] Apply suggestions from code review Co-authored-by: Geoffrey Gilmore --- internal/gitserver/git_command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gitserver/git_command.go b/internal/gitserver/git_command.go index f0d0d876b948..7563594c4d64 100644 --- a/internal/gitserver/git_command.go +++ b/internal/gitserver/git_command.go @@ -191,7 +191,7 @@ func (c *RemoteGitCommand) DividedOutput(ctx context.Context) ([]byte, []byte, e if v.Message != "" { return stdout, []byte(v.Stderr), errors.New(v.Message) } else { - return stdout, []byte(v.Stderr), nil + return stdout, []byte(v.Stderr), v } } return nil, nil, errors.Wrap(err, "reading exec output") From dfa64aba599e06505b1bec50178ecd03914b8618 Mon Sep 17 00:00:00 2001 From: Geoffrey Gilmore Date: Wed, 8 Feb 2023 21:02:25 +0100 Subject: [PATCH 13/15] internal/gitserver: wrap test servers in GRPC handling logic --- internal/gitserver/client_test.go | 19 +++++++--- .../gitserver/integration_tests/test_utils.go | 36 ++++++++++++------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/internal/gitserver/client_test.go b/internal/gitserver/client_test.go index 708ede6a0245..700e8609fdb4 100644 --- a/internal/gitserver/client_test.go +++ b/internal/gitserver/client_test.go @@ -7,6 +7,12 @@ import ( "encoding/base64" "encoding/json" "fmt" + "github.com/sourcegraph/log/logtest" + + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" + grpc2 "github.com/sourcegraph/sourcegraph/internal/grpc" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" + "google.golang.org/grpc" "io" "net/http" "net/http/httptest" @@ -26,8 +32,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/sourcegraph/log/logtest" - "github.com/sourcegraph/sourcegraph/cmd/gitserver/server" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/database" @@ -437,7 +441,7 @@ func TestClient_ResolveRevisions(t *testing.T) { }} db := newMockDB() - srv := httptest.NewServer((&server.Server{ + s := server.Server{ Logger: logtest.Scoped(t), ReposDir: filepath.Join(root, "repos"), GetRemoteURLFunc: func(_ context.Context, name api.RepoName) (string, error) { @@ -447,7 +451,14 @@ func TestClient_ResolveRevisions(t *testing.T) { return &server.GitRepoSyncer{}, nil }, DB: db, - }).Handler()) + } + + grpcServer := grpc.NewServer(defaults.ServerOptions(logtest.Scoped(t))...) + grpcServer.RegisterService(&proto.GitserverService_ServiceDesc, &server.GRPCServer{Server: &s}) + + handler := grpc2.MultiplexHandlers(grpcServer, s.Handler()) + srv := httptest.NewServer(handler) + defer srv.Close() u, _ := url.Parse(srv.URL) diff --git a/internal/gitserver/integration_tests/test_utils.go b/internal/gitserver/integration_tests/test_utils.go index 2b4c5bbabbfe..d1f0415a9016 100644 --- a/internal/gitserver/integration_tests/test_utils.go +++ b/internal/gitserver/integration_tests/test_utils.go @@ -2,6 +2,10 @@ package inttests import ( "context" + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" + internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" + "google.golang.org/grpc" "net" "net/http" "os" @@ -51,20 +55,26 @@ func InitGitserver() { gr := database.NewMockGitserverRepoStore() db.GitserverReposFunc.SetDefaultReturn(gr) + s := server.Server{ + Logger: sglog.Scoped("server", "the gitserver service"), + ObservationCtx: &observation.TestContext, + ReposDir: filepath.Join(root, "repos"), + GetRemoteURLFunc: func(ctx context.Context, name api.RepoName) (string, error) { + return filepath.Join(root, "remotes", string(name)), nil + }, + GetVCSSyncer: func(ctx context.Context, name api.RepoName) (server.VCSSyncer, error) { + return &server.GitRepoSyncer{}, nil + }, + GlobalBatchLogSemaphore: semaphore.NewWeighted(32), + DB: db, + } + + grpcServer := grpc.NewServer(defaults.ServerOptions(logger)...) + grpcServer.RegisterService(&proto.GitserverService_ServiceDesc, &server.GRPCServer{Server: &s}) + handler := internalgrpc.MultiplexHandlers(grpcServer, s.Handler()) + srv := &http.Server{ - Handler: (&server.Server{ - Logger: sglog.Scoped("server", "the gitserver service"), - ObservationCtx: &observation.TestContext, - ReposDir: filepath.Join(root, "repos"), - GetRemoteURLFunc: func(ctx context.Context, name api.RepoName) (string, error) { - return filepath.Join(root, "remotes", string(name)), nil - }, - GetVCSSyncer: func(ctx context.Context, name api.RepoName) (server.VCSSyncer, error) { - return &server.GitRepoSyncer{}, nil - }, - GlobalBatchLogSemaphore: semaphore.NewWeighted(32), - DB: db, - }).Handler(), + Handler: handler, } go func() { if err := srv.Serve(l); err != nil { From 8b43f7309b44cf5e8e8525326636cbe5dc9e2c08 Mon Sep 17 00:00:00 2001 From: Geoffrey Gilmore Date: Wed, 8 Feb 2023 21:04:23 +0100 Subject: [PATCH 14/15] grpc2 -> internalgrpc --- internal/gitserver/client_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/gitserver/client_test.go b/internal/gitserver/client_test.go index 700e8609fdb4..0306bca45fb1 100644 --- a/internal/gitserver/client_test.go +++ b/internal/gitserver/client_test.go @@ -10,7 +10,7 @@ import ( "github.com/sourcegraph/log/logtest" "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" - grpc2 "github.com/sourcegraph/sourcegraph/internal/grpc" + internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "google.golang.org/grpc" "io" @@ -456,7 +456,7 @@ func TestClient_ResolveRevisions(t *testing.T) { grpcServer := grpc.NewServer(defaults.ServerOptions(logtest.Scoped(t))...) grpcServer.RegisterService(&proto.GitserverService_ServiceDesc, &server.GRPCServer{Server: &s}) - handler := grpc2.MultiplexHandlers(grpcServer, s.Handler()) + handler := internalgrpc.MultiplexHandlers(grpcServer, s.Handler()) srv := httptest.NewServer(handler) defer srv.Close() From 9ec300b4c9cfebba2842c674fe3204ffd79504d1 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Wed, 8 Feb 2023 15:26:12 -0700 Subject: [PATCH 15/15] goimports --- internal/gitserver/client_test.go | 10 ++++++---- internal/gitserver/integration_tests/test_utils.go | 11 ++++++----- internal/gitserver/proto/gitserver.pb.go | 5 +++-- internal/gitserver/proto/gitserver_grpc.pb.go | 1 + 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/gitserver/client_test.go b/internal/gitserver/client_test.go index 0306bca45fb1..ce5d3df361ed 100644 --- a/internal/gitserver/client_test.go +++ b/internal/gitserver/client_test.go @@ -7,12 +7,9 @@ import ( "encoding/base64" "encoding/json" "fmt" + "github.com/sourcegraph/log/logtest" - "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" - internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" - "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" - "google.golang.org/grpc" "io" "net/http" "net/http/httptest" @@ -25,6 +22,11 @@ import ( "testing" "time" + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" + internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" + "google.golang.org/grpc" + "github.com/sourcegraph/sourcegraph/internal/conf" "github.com/sourcegraph/sourcegraph/schema" diff --git a/internal/gitserver/integration_tests/test_utils.go b/internal/gitserver/integration_tests/test_utils.go index d1f0415a9016..cc5c15e589c3 100644 --- a/internal/gitserver/integration_tests/test_utils.go +++ b/internal/gitserver/integration_tests/test_utils.go @@ -2,10 +2,6 @@ package inttests import ( "context" - "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" - internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" - "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" - "google.golang.org/grpc" "net" "net/http" "os" @@ -15,15 +11,20 @@ import ( "strings" "testing" + "golang.org/x/sync/semaphore" + "google.golang.org/grpc" + sglog "github.com/sourcegraph/log" "github.com/sourcegraph/sourcegraph/cmd/gitserver/server" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/database" "github.com/sourcegraph/sourcegraph/internal/gitserver" + "github.com/sourcegraph/sourcegraph/internal/gitserver/proto" + internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc" + "github.com/sourcegraph/sourcegraph/internal/grpc/defaults" "github.com/sourcegraph/sourcegraph/internal/httpcli" "github.com/sourcegraph/sourcegraph/internal/observation" - "golang.org/x/sync/semaphore" ) var root string diff --git a/internal/gitserver/proto/gitserver.pb.go b/internal/gitserver/proto/gitserver.pb.go index 359759aefcf0..425738008315 100644 --- a/internal/gitserver/proto/gitserver.pb.go +++ b/internal/gitserver/proto/gitserver.pb.go @@ -7,10 +7,11 @@ package proto import ( - protoreflect "google.golang.org/protobuf/reflect/protoreflect" - protoimpl "google.golang.org/protobuf/runtime/protoimpl" reflect "reflect" sync "sync" + + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" ) const ( diff --git a/internal/gitserver/proto/gitserver_grpc.pb.go b/internal/gitserver/proto/gitserver_grpc.pb.go index 537113aa3085..b26595c1a501 100644 --- a/internal/gitserver/proto/gitserver_grpc.pb.go +++ b/internal/gitserver/proto/gitserver_grpc.pb.go @@ -8,6 +8,7 @@ package proto import ( context "context" + grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status"