diff --git a/workhorse/_support/lint_last_known_acceptable.txt b/workhorse/_support/lint_last_known_acceptable.txt index e75c56c3d9725ce34fee2b2369610f968da1b53f..ecc239e63c635d214fc21ad094f1128ddac4b548 100644 --- a/workhorse/_support/lint_last_known_acceptable.txt +++ b/workhorse/_support/lint_last_known_acceptable.txt @@ -53,31 +53,6 @@ internal/git/responsewriter.go:52:1: exported: exported method HTTPResponseWrite internal/git/snapshot.go:27:2: exported: exported var SendSnapshot should have comment or be unexported (revive) internal/git/upload-pack.go:37:16: Error return value of `cw.Flush` is not checked (errcheck) internal/git/upload-pack_test.go:72:2: error-is-as: use require.ErrorIs (testifylint) -internal/gitaly/blob.go:1:1: package-comments: should have a package comment (revive) -internal/gitaly/blob.go:14:6: exported: exported type BlobClient should have comment or be unexported (revive) -internal/gitaly/blob.go:18:1: exported: exported method BlobClient.SendBlob should have comment or be unexported (revive) -internal/gitaly/diff.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/diff.go:13:6: exported: exported type DiffClient should have comment or be unexported (revive) -internal/gitaly/diff.go:17: 17-35 lines are duplicate of `internal/gitaly/diff.go:37-55` (dupl) -internal/gitaly/diff.go:37: 37-55 lines are duplicate of `internal/gitaly/diff.go:17-35` (dupl) -internal/gitaly/gitaly.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/gitaly.go:63:1: exported: exported function InitializeSidechannelRegistry should have comment or be unexported (revive) -internal/gitaly/gitaly.go:85:1: exported: exported function NewSmartHTTPClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:98:1: exported: exported function NewBlobClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:107:1: exported: exported function NewRepositoryClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:116:1: exported: exported function NewDiffClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:169:1: exported: exported function CloseConnections should have comment or be unexported (revive) -internal/gitaly/gitaly.go:174:13: Error return value of `conn.Close` is not checked (errcheck) -internal/gitaly/gitaly.go:179:14: appendAssign: append result not assigned to the same slice (gocritic) -internal/gitaly/gitaly.go:219:1: exported: exported function UnmarshalJSON should have comment or be unexported (revive) -internal/gitaly/repository.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/smarthttp.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/smarthttp.go:13:6: exported: exported type SmartHTTPClient should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:18:1: exported: exported method SmartHTTPClient.InfoRefsResponseReader should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:48:1: exported: exported method SmartHTTPClient.ReceivePack should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:84:19: Error return value of `stream.CloseSend` is not checked (errcheck) -internal/gitaly/smarthttp.go:97:1: exported: exported method SmartHTTPClient.UploadPack should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:113:20: Error return value of `waiter.Close` is not checked (errcheck) internal/headers/headers.go:10: internal/headers/headers.go:10: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "Fixme: Go back to 512 bytes once https:/..." (godox) internal/helper/exception/exception.go:11:14: SA1019: "gitlab.com/gitlab-org/labkit/correlation/raven" is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) internal/helper/exception/exception.go:36:11: SA1019: correlation.SetExtra is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) diff --git a/workhorse/internal/gitaly/blob.go b/workhorse/internal/gitaly/blob.go index c84c1731300ad0077a7bf678805b5f173c032d28..f5a7f82ce6aff5b8b8f527f25c260b7ec7a52731 100644 --- a/workhorse/internal/gitaly/blob.go +++ b/workhorse/internal/gitaly/blob.go @@ -11,10 +11,12 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/streamio" ) +// BlobClient wraps the gRPC client for Gitaly's BlobService. type BlobClient struct { gitalypb.BlobServiceClient } +// SendBlob streams the blob data from Gitaly to the HTTP response writer. func (client *BlobClient) SendBlob(ctx context.Context, w http.ResponseWriter, request *gitalypb.GetBlobRequest) error { c, err := client.GetBlob(ctx, request) if err != nil { diff --git a/workhorse/internal/gitaly/diff.go b/workhorse/internal/gitaly/diff.go index a8f427231bbb551f71a85617a6d8a4101e472254..dc036970f0f37d337e4ac901767bbe85342a0cc7 100644 --- a/workhorse/internal/gitaly/diff.go +++ b/workhorse/internal/gitaly/diff.go @@ -10,22 +10,15 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/streamio" ) +// DiffClient wraps the Gitaly DiffServiceClient. type DiffClient struct { gitalypb.DiffServiceClient } -func (client *DiffClient) SendRawDiff(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawDiffRequest) error { - c, err := client.RawDiff(ctx, request) - if err != nil { - return fmt.Errorf("rpc failed: %v", err) - } - +func (client *DiffClient) sendStream(w http.ResponseWriter, recv func() ([]byte, error)) error { w.Header().Del("Content-Length") - rr := streamio.NewReader(func() ([]byte, error) { - resp, err := c.Recv() - return resp.GetData(), err - }) + rr := streamio.NewReader(recv) if _, err := io.Copy(w, rr); err != nil { return fmt.Errorf("copy rpc data: %v", err) @@ -34,22 +27,28 @@ func (client *DiffClient) SendRawDiff(ctx context.Context, w http.ResponseWriter return nil } -func (client *DiffClient) SendRawPatch(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawPatchRequest) error { - c, err := client.RawPatch(ctx, request) +// SendRawDiff streams a raw diff to the HTTP response. +func (client *DiffClient) SendRawDiff(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawDiffRequest) error { + c, err := client.RawDiff(ctx, request) if err != nil { return fmt.Errorf("rpc failed: %v", err) } - w.Header().Del("Content-Length") - - rr := streamio.NewReader(func() ([]byte, error) { + return client.sendStream(w, func() ([]byte, error) { resp, err := c.Recv() return resp.GetData(), err }) +} - if _, err := io.Copy(w, rr); err != nil { - return fmt.Errorf("copy rpc data: %v", err) +// SendRawPatch streams a raw patch to the HTTP response. +func (client *DiffClient) SendRawPatch(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawPatchRequest) error { + c, err := client.RawPatch(ctx, request) + if err != nil { + return fmt.Errorf("rpc failed: %v", err) } - return nil + return client.sendStream(w, func() ([]byte, error) { + resp, err := c.Recv() + return resp.GetData(), err + }) } diff --git a/workhorse/internal/gitaly/gitaly.go b/workhorse/internal/gitaly/gitaly.go index e4e2c827e9407d988a75e128ffb2e9dda90b1e14..887f71815ac04f1dfda250c0d043ea291c40106d 100644 --- a/workhorse/internal/gitaly/gitaly.go +++ b/workhorse/internal/gitaly/gitaly.go @@ -1,3 +1,16 @@ +/* +Package gitaly provides a comprehensive client for interacting with Gitaly services, facilitating operations on Git repositories. Key features include: + +1. Streaming blob data from Gitaly Blob service, suitable for serving content over HTTP. +2. Retrieving and streaming diffs and patches from Gitaly server. +3. Managing gRPC connections to Gitaly server with connection caching and metadata handling. +4. Providing clients for various services including Blob, Repository, and Diff. +5. Retrieving repository archives and snapshots through the RepositoryService. +6. Handling Git operations via smart HTTP requests, including InfoRefs, ReceivePack, and UploadPack. +7. Supporting efficient streaming of request and response data. + +This package enhances interaction with Git repositories managed by Gitaly, offering a streamlined interface for version control operations and data retrieval. +*/ package gitaly import ( @@ -60,6 +73,7 @@ var ( ) ) +// InitializeSidechannelRegistry creates the side channel registry if it doesn't exist. func InitializeSidechannelRegistry(logger *logrus.Logger) { if sidechannelRegistry == nil { sidechannelRegistry = gitalyclient.NewSidechannelRegistry(logrus.NewEntry(logger)) @@ -82,6 +96,7 @@ func withOutgoingMetadata(ctx context.Context, gs api.GitalyServer) context.Cont return metadata.NewOutgoingContext(ctx, md) } +// NewSmartHTTPClient is created and returns it with updated context. func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer) (context.Context, *SmartHTTPClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -95,6 +110,7 @@ func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer) (context.C return withOutgoingMetadata(ctx, server), smartHTTPClient, nil } +// NewBlobClient is created and returns it with updated context. func NewBlobClient(ctx context.Context, server api.GitalyServer) (context.Context, *BlobClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -104,6 +120,7 @@ func NewBlobClient(ctx context.Context, server api.GitalyServer) (context.Contex return withOutgoingMetadata(ctx, server), &BlobClient{grpcClient}, nil } +// NewRepositoryClient is created and returns it with updated context. func NewRepositoryClient(ctx context.Context, server api.GitalyServer) (context.Context, *RepositoryClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -113,6 +130,7 @@ func NewRepositoryClient(ctx context.Context, server api.GitalyServer) (context. return withOutgoingMetadata(ctx, server), &RepositoryClient{grpcClient}, nil } +// NewDiffClient is created and returns it with updated context. func NewDiffClient(ctx context.Context, server api.GitalyServer) (context.Context, *DiffClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -156,27 +174,29 @@ func getOrCreateConnection(server api.GitalyServer) (*grpc.ClientConn, error) { return cachedConn, nil } - conn, err := newConnection(server) + newConn, err := newConnection(server) if err != nil { return nil, err } - cache.connections[key] = conn + cache.connections[key] = newConn - return conn, nil + return newConn, nil } +// CloseConnections closes all connections in cache. func CloseConnections() { cache.Lock() defer cache.Unlock() for _, conn := range cache.connections { - conn.Close() + _ = conn.Close() } } func newConnection(server api.GitalyServer) (*grpc.ClientConn, error) { - connOpts := append(gitalyclient.DefaultDialOpts, + connOpts := gitalyclient.DefaultDialOpts + connOpts = append(connOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(server.Token)), grpc.WithChainStreamInterceptor( grpctracing.StreamClientTracingInterceptor(), @@ -193,7 +213,6 @@ func newConnection(server api.GitalyServer) (*grpc.ClientConn, error) { grpccorrelation.WithClientName("gitlab-workhorse"), ), ), - // In https://gitlab.com/groups/gitlab-org/-/epics/8971, we added DNS discovery support to Praefect. This was // done by making two changes: // - Configure client-side round-robin load-balancing in client dial options. We added that as a default option @@ -216,6 +235,7 @@ func newConnection(server api.GitalyServer) (*grpc.ClientConn, error) { return conn, connErr } +// UnmarshalJSON into a protobuf message. func UnmarshalJSON(s string, msg proto.Message) error { return protojson.UnmarshalOptions{DiscardUnknown: true}.Unmarshal([]byte(s), msg) } diff --git a/workhorse/internal/gitaly/smarthttp.go b/workhorse/internal/gitaly/smarthttp.go index 2fdf88831b22ff1850b5187ca18570e6e1974440..e9484676707fb215a3b9c4483affb1b608b4c498 100644 --- a/workhorse/internal/gitaly/smarthttp.go +++ b/workhorse/internal/gitaly/smarthttp.go @@ -10,11 +10,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/streamio" ) +// SmartHTTPClient encapsulates the SmartHTTPServiceClient for Gitaly. type SmartHTTPClient struct { sidechannelRegistry *gitalyclient.SidechannelRegistry gitalypb.SmartHTTPServiceClient } +// InfoRefsResponseReader handles InfoRefs requests and returns an io.Reader for the response. func (client *SmartHTTPClient) InfoRefsResponseReader(ctx context.Context, repo *gitalypb.Repository, rpc string, gitConfigOptions []string, gitProtocol string) (io.Reader, error) { rpcRequest := &gitalypb.InfoRefsRequest{ Repository: repo, @@ -45,6 +47,7 @@ func infoRefsReader(stream infoRefsClient) io.Reader { }) } +// ReceivePack performs a receive pack operation with Git configuration options. func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.Repository, glID string, glUsername string, glRepository string, gitConfigOptions []string, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error { stream, err := client.PostReceivePack(ctx) if err != nil { @@ -81,7 +84,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.R return stream.Send(&gitalypb.PostReceivePackRequest{Data: data}) }) _, err := io.Copy(sw, clientRequest) - stream.CloseSend() + _ = stream.CloseSend() errC <- err }() @@ -94,6 +97,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.R return nil } +// UploadPack performs an upload pack operation with a sidechannel. func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string, gitProtocol string) (*gitalypb.PostUploadPackWithSidechannelResponse, error) { ctx, waiter := client.sidechannelRegistry.Register(ctx, func(conn gitalyclient.SidechannelConn) error { if _, err := io.Copy(conn, clientRequest); err != nil { @@ -110,7 +114,7 @@ func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Re return nil }) - defer waiter.Close() + defer waiter.Close() //nolint:errcheck rpcRequest := &gitalypb.PostUploadPackWithSidechannelRequest{ Repository: repo,