From c622bdf031dfb5105e5cfe36f36fc7c975a4a722 Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Fri, 21 Jul 2023 01:39:28 +0530 Subject: Rangediff: Implement RawRangeDiff Service and Range Notation Validation Implement the RawRangeDiff function in the rangediff service to process RangeDiffRequests and send the results back to the client. The revisions and range notation from the request are validated, with unsupported range notations returning appropriate errors. An error is also returned when any revision is empty. The sendRawOutput function is introduced to execute the provided git.Command and direct the output to the supplied io.Writer. Signed-off-by: Siddharth Asthana --- internal/gitaly/service/rangediff/rangediff.go | 56 +++++++++ .../gitaly/service/rangediff/rangediff_test.go | 134 +++++++++++++++++++++ internal/gitaly/service/rangediff/server.go | 21 ++++ 3 files changed, 211 insertions(+) create mode 100644 internal/gitaly/service/rangediff/rangediff.go create mode 100644 internal/gitaly/service/rangediff/rangediff_test.go create mode 100644 internal/gitaly/service/rangediff/server.go diff --git a/internal/gitaly/service/rangediff/rangediff.go b/internal/gitaly/service/rangediff/rangediff.go new file mode 100644 index 000000000..21409dd4d --- /dev/null +++ b/internal/gitaly/service/rangediff/rangediff.go @@ -0,0 +1,56 @@ +package rangediff + +import ( + "context" + "io" + + "gitlab.com/gitlab-org/gitaly/v16/internal/git" + "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" + "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/v16/streamio" +) + +// RawRangeDiff processes the RangeDiffRequest and sends the result back to the client. +func (s *server) RawRangeDiff(in *gitalypb.RangeDiffRequest, stream gitalypb.RangeDiffService_RawRangeDiffServer) error { + if in.GetRangeNotation() != gitalypb.RangeDiffRequest_TWO_REVS { + return structerr.NewInvalidArgument("only TWO_REVS range notation is supported") + } + + if !isValidRevision(stream.Context(), in.GetRev1OrRange1()) || !isValidRevision(stream.Context(), in.GetRev2OrRange2()) { + return structerr.NewInvalidArgument("revisions are not valid") + } + + // Create the range-diff command + cmd := git.Command{ + Name: "range-diff", + Flags: []git.Option{}, + Args: []string{in.GetRev1OrRange1(), "...", in.GetRev2OrRange2()}, + } + + sw := streamio.NewWriter(func(p []byte) error { + return stream.Send(&gitalypb.RawRangeDiffResponse{Data: p}) + }) + + return sendRawOutput(stream.Context(), s.gitCmdFactory, in.Repository, sw, cmd) +} + +// sendRawOutput runs the provided git.Command and sends the output to the provided io.Writer. +func sendRawOutput(ctx context.Context, gitCmdFactory git.CommandFactory, repo *gitalypb.Repository, sender io.Writer, subCmd git.Command) error { + cmd, err := gitCmdFactory.New(ctx, repo, subCmd) + if err != nil { + return structerr.NewInternal("cmd: %w", err) + } + + if _, err := io.Copy(sender, cmd); err != nil { + return structerr.NewAborted("send: %w", err) + } + + return cmd.Wait() +} + +func isValidRevision(ctx context.Context, revision string) bool { + if len(revision) == 0 { + return false + } + return true +} diff --git a/internal/gitaly/service/rangediff/rangediff_test.go b/internal/gitaly/service/rangediff/rangediff_test.go new file mode 100644 index 000000000..ece1a09fb --- /dev/null +++ b/internal/gitaly/service/rangediff/rangediff_test.go @@ -0,0 +1,134 @@ +package rangediff + +import ( + "bytes" + "context" + "io" + "testing" + + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service" + "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" + "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" + "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg" + "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testserver" + "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +func setupRangeDiffService(tb testing.TB, ctx context.Context, opt ...testserver.GitalyServerOpt) (config.Cfg, *gitalypb.Repository, string, gitalypb.RangeDiffServiceClient) { + cfg := testcfg.Build(tb) + + addr := testserver.RunGitalyServer(tb, cfg, func(srv *grpc.Server, deps *service.Dependencies) { + gitalypb.RegisterRangeDiffServiceServer(srv, NewServer( + deps.GetLocator(), + deps.GetGitCmdFactory(), + )) + }, opt...) + cfg.SocketPath = addr + + conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(tb, err) + tb.Cleanup(func() { testhelper.MustClose(tb, conn) }) + + client := gitalypb.NewRangeDiffServiceClient(conn) + + repo, repoPath := gittest.CreateRepository(tb, ctx, cfg, gittest.CreateRepositoryConfig{ + Seed: gittest.SeedGitLabTest, + }) + + return cfg, repo, repoPath, client +} + +func TestRawRangeDiff_successful(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + cfg, repoProto, _, client := setupRangeDiffService(t, ctx) + repoProto, _ = gittest.CreateRepository(t, ctx, cfg) + + request := &gitalypb.RangeDiffRequest{ + Repository: repoProto, + Rev1OrRange1: "master~2", + Rev2OrRange2: "master", + RangeNotation: gitalypb.RangeDiffRequest_TWO_REVS, + } + + stream, err := client.RawRangeDiff(ctx, request) + require.NoError(t, err) + + var buffer bytes.Buffer + writer := io.Writer(&buffer) + + for { + resp, err := stream.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + _, err = writer.Write(resp.GetData()) + require.NoError(t, err) + } +} + +func TestRawRangeDiff_inputValidation(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + cfg, repoProto, _, client := setupRangeDiffService(t, ctx) + repoProto, _ = gittest.CreateRepository(t, ctx, cfg) + + tests := []struct { + desc string + request *gitalypb.RangeDiffRequest + expectedErr error + }{ + { + desc: "empty rev1 or range1", + request: &gitalypb.RangeDiffRequest{ + Repository: repoProto, + Rev2OrRange2: "master", + RangeNotation: gitalypb.RangeDiffRequest_TWO_REVS, + }, + expectedErr: structerr.NewInvalidArgument("revisions cannot be empty"), + }, + { + desc: "empty rev2 or range2", + request: &gitalypb.RangeDiffRequest{ + Repository: repoProto, + Rev1OrRange1: "master~2", + RangeNotation: gitalypb.RangeDiffRequest_TWO_REVS, + }, + expectedErr: structerr.NewInvalidArgument("revisions cannot be empty"), + }, + { + desc: "unsupported range notation", + request: &gitalypb.RangeDiffRequest{ + Repository: repoProto, + Rev1OrRange1: "master~2", + Rev2OrRange2: "master", + }, + expectedErr: structerr.NewInvalidArgument("only TWO_REVS range notation is supported"), + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + stream, err := client.RawRangeDiff(ctx, tc.request) + require.NoError(t, err) + err = drainRawRangeDiffResponse(stream) + testhelper.RequireGrpcError(t, tc.expectedErr, err) + }) + } +} + +func drainRawRangeDiffResponse(c gitalypb.RangeDiffService_RawRangeDiffClient) error { + var err error + for err == nil { + _, err = c.Recv() + } + return err +} diff --git a/internal/gitaly/service/rangediff/server.go b/internal/gitaly/service/rangediff/server.go new file mode 100644 index 000000000..c444be16a --- /dev/null +++ b/internal/gitaly/service/rangediff/server.go @@ -0,0 +1,21 @@ +package rangediff + +import ( + "gitlab.com/gitlab-org/gitaly/v16/internal/git" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" + "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" +) + +type server struct { + gitalypb.UnimplementedRangeDiffServiceServer + locator storage.Locator + gitCmdFactory git.CommandFactory +} + +// NewServer creates a new instance of a grpc RangeDiffServiceServer +func NewServer(locator storage.Locator, gitCmdFactory git.CommandFactory) gitalypb.RangeDiffServiceServer { + return &server{ + locator: locator, + gitCmdFactory: gitCmdFactory, + } +} -- cgit v1.2.3