Lint issue in gitaly

workhorse/_support/lint_last_known_acceptable.txt

@@ -49,31 +49,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/imageresizer/image_resizer.go:1:1: package-comments: should have a package comment (revive)
 internal/imageresizer/image_resizer.go:30:6: exported: exported type Resizer should have comment or be unexported (revive)

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 {

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
+	})
 }

workhorse/internal/gitaly/gitaly.go

+/*
+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)
 }

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,