Merge branch 'duo-edit-20260107-094508' into 'main'

Fix linter warnings in command package

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/1358

Merged-by: Igor Drozdov <idrozdov@gitlab.com>
Approved-by: Igor Drozdov <idrozdov@gitlab.com>
Co-authored-by: Vasilii Iakliushin <viakliushin@gitlab.com>

Author: 2 people

internal/command/command.go

@@ -1,3 +1,6 @@
+// Package command provides the core command execution infrastructure for gitlab-shell.
+// It defines the Command interface that all shell commands must implement,
+// along with shared utilities for logging, tracing, and context management.
 package command
 
 import (
@@ -12,17 +15,21 @@ import (
 	"gitlab.com/gitlab-org/labkit/tracing"
 )
 
+// Command is the interface that all gitlab-shell commands must implement.
+// Execute runs the command and returns the updated context and any error.
 type Command interface {
 	Execute(ctx context.Context) (context.Context, error)
 }
 
+// LogMetadata contains project and namespace information for structured logging.
 type LogMetadata struct {
 	Project         string `json:"project,omitempty"`
 	RootNamespace   string `json:"root_namespace,omitempty"`
 	ProjectID       int    `json:"project_id,omitempty"`
 	RootNamespaceID int    `json:"root_namespace_id,omitempty"`
 }
 
+// LogData contains user and request information for structured logging.
 type LogData struct {
 	Username     string      `json:"username"`
 	WrittenBytes int64       `json:"written_bytes"`
@@ -34,6 +41,8 @@ type contextKey string
 // LogDataKey is the context key used to store log data in request contexts.
 const LogDataKey contextKey = "logData"
 
+// CheckForVersionFlag checks if the -version flag was passed and prints version info if so.
+// It exits the program after printing the version.
 func CheckForVersionFlag(osArgs []string, version, buildTime string) {
 	// We can't use the flag library because gitlab-shell receives other arguments
 	// that confuse the parser.
@@ -45,7 +54,7 @@ func CheckForVersionFlag(osArgs []string, version, buildTime string) {
 	}
 }
 
-// Setup() initializes tracing from the configuration file and generates a
+// Setup initializes tracing from the configuration file and generates a
 // background context from which all other contexts in the process should derive
 // from, as it has a service name and initial correlation ID set.
 func Setup(serviceName string, config *config.Config) (context.Context, func()) {
@@ -77,10 +86,12 @@ func Setup(serviceName string, config *config.Config) (context.Context, func())
 
 	return ctx, func() {
 		finished()
-		closer.Close()
+		_ = closer.Close()
 	}
 }
 
+// NewLogData creates a new LogData instance with the given project, username, and IDs.
+// It extracts the root namespace from the project path.
 func NewLogData(project, username string, projectID, rootNamespaceID int) LogData {
 	rootNameSpace := ""
 

internal/command/githttp/pull.go

@@ -51,8 +51,6 @@ func (c *PullCommand) Execute(ctx context.Context) error {
 
 	// For Git over SSH routing
 	if data.GeoProxyFetchSSHDirectToPrimary {
-		log.ContextLogger(ctx).Info("Using Git over SSH upload pack")
-
 		client.Headers["Git-Protocol"] = c.Args.Env.GitProtocolVersion
 		return c.requestSSHUploadPack(ctx, client)
 	}
@@ -65,15 +63,9 @@ func (c *PullCommand) Execute(ctx context.Context) error {
 }
 
 func (c *PullCommand) requestSSHUploadPack(ctx context.Context, client *git.Client) error {
-	response, err := client.SSHUploadPack(ctx, io.NopCloser(c.ReadWriter.In))
-	if err != nil {
-		return err
-	}
-	defer response.Body.Close() //nolint:errcheck
-
-	_, err = io.Copy(c.ReadWriter.Out, response.Body)
+	log.ContextLogger(ctx).Info("Using Git over SSH upload pack")
 
-	return err
+	return executeSSHRequest(ctx, client.SSHUploadPack, c.ReadWriter)
 }
 
 func (c *PullCommand) requestUploadPack(ctx context.Context, client *git.Client) error {

internal/command/githttp/push.go

@@ -48,8 +48,6 @@ func (c *PushCommand) Execute(ctx context.Context) error {
 
 	// For Git over SSH routing
 	if data.GeoProxyPushSSHDirectToPrimary {
-		log.ContextLogger(ctx).Info("Using Git over SSH receive pack")
-
 		client.Headers["Git-Protocol"] = c.Args.Env.GitProtocolVersion
 		return c.requestSSHReceivePack(ctx, client)
 	}
@@ -62,15 +60,9 @@ func (c *PushCommand) Execute(ctx context.Context) error {
 }
 
 func (c *PushCommand) requestSSHReceivePack(ctx context.Context, client *git.Client) error {
-	response, err := client.SSHReceivePack(ctx, io.NopCloser(c.ReadWriter.In))
-	if err != nil {
-		return err
-	}
-	defer response.Body.Close() //nolint:errcheck
-
-	_, err = io.Copy(c.ReadWriter.Out, response.Body)
+	log.ContextLogger(ctx).Info("Using Git over SSH receive pack")
 
-	return err
+	return executeSSHRequest(ctx, client.SSHReceivePack, c.ReadWriter)
 }
 
 func (c *PushCommand) requestReceivePack(ctx context.Context, client *git.Client) error {

internal/command/githttp/util.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/readwriter"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/git"
@@ -39,3 +40,19 @@ func requestInfoRefs(ctx context.Context, client *git.Client, command gitHTTPCom
 
 	return err
 }
+
+// sshRequestFunc is a function type for SSH pack requests
+type sshRequestFunc func(ctx context.Context, body io.Reader) (*http.Response, error)
+
+// executeSSHRequest executes an SSH request and copies the response to the output
+func executeSSHRequest(ctx context.Context, requestFn sshRequestFunc, rw *readwriter.ReadWriter) error {
+	response, err := requestFn(ctx, io.NopCloser(rw.In))
+	if err != nil {
+		return err
+	}
+	defer response.Body.Close() //nolint:errcheck
+
+	_, err = io.Copy(rw.Out, response.Body)
+
+	return err
+}

internal/command/lfsauthenticate/lfsauthenticate.go

@@ -84,7 +84,9 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) {
 		return ctxWithLogData, nil
 	}
 
-	fmt.Fprintf(c.ReadWriter.Out, "%s\n", payload)
+	if _, err := fmt.Fprintf(c.ReadWriter.Out, "%s\n", payload); err != nil {
+		return ctxWithLogData, err
+	}
 
 	return ctxWithLogData, nil
 }

internal/command/readwriter/readwriter.go

@@ -1,9 +1,11 @@
+// Package readwriter provides I/O abstractions for command input and output streams.
 package readwriter
 
 import (
 	"io"
 )
 
+// ReadWriter bundles the standard input, output, and error streams for command execution.
 type ReadWriter struct {
 	Out    io.Writer
 	In     io.Reader

internal/command/receivepack/gitalycall_test.go

@@ -28,13 +28,13 @@ func TestReceivePack(t *testing.T) {
 
 			testCases := []struct {
 				username string
-				keyId    string
+				keyID    string
 			}{
 				{
 					username: "john.doe",
 				},
 				{
-					keyId: "123",
+					keyID: "123",
 				},
 			}
 
@@ -58,7 +58,7 @@ func TestReceivePack(t *testing.T) {
 				if tc.username != "" {
 					args.GitlabUsername = tc.username
 				} else {
-					args.GitlabKeyID = tc.keyId
+					args.GitlabKeyID = tc.keyID
 				}
 
 				cfg := &config.Config{GitlabURL: url}
@@ -95,7 +95,7 @@ func TestReceivePack(t *testing.T) {
 					require.Equal(t, v, actual[0])
 				}
 				require.Empty(t, testServer.ReceivedMD["some-other-ff"])
-				require.Equal(t, testServer.ReceivedMD["x-gitlab-correlation-id"][0], "a-correlation-id")
+				require.Equal(t, "a-correlation-id", testServer.ReceivedMD["x-gitlab-correlation-id"][0])
 			}
 		})
 	}

support/lint_last_known_acceptable.txt

@@ -1,15 +0,0 @@
-internal/command/command.go:1:1: package-comments: should have a package comment (revive)
-internal/command/command.go:15:6: exported: exported type Command should have comment or be unexported (revive)
-internal/command/command.go:19:6: exported: exported type LogMetadata should have comment or be unexported (revive)
-internal/command/command.go:26:6: exported: exported type LogData should have comment or be unexported (revive)
-internal/command/command.go:37:1: exported: exported function CheckForVersionFlag should have comment or be unexported (revive)
-internal/command/command.go:48:1: exported: comment on exported function Setup should be of the form "Setup ..." (revive)
-internal/command/command.go:80:15: Error return value of `closer.Close` is not checked (errcheck)
-internal/command/command.go:84:1: exported: exported function NewLogData should have comment or be unexported (revive)
-internal/command/githttp/pull.go:48: 48-65 lines are duplicate of `internal/command/githttp/push.go:45-62` (dupl)
-internal/command/githttp/push.go:45: 45-62 lines are duplicate of `internal/command/githttp/pull.go:48-65` (dupl)
-internal/command/lfsauthenticate/lfsauthenticate.go:87:13: Error return value of `fmt.Fprintf` is not checked (errcheck)
-internal/command/readwriter/readwriter.go:1:1: package-comments: should have a package comment (revive)
-internal/command/readwriter/readwriter.go:7:6: exported: exported type ReadWriter should have comment or be unexported (revive)
-internal/command/receivepack/gitalycall_test.go:31:5: var-naming: struct field keyId should be keyID (revive)
-internal/command/receivepack/gitalycall_test.go:98:5: expected-actual: need to reverse actual and expected values (testifylint)