Merge branch '799-shared-packages' into 'main'

Lint fixes for shared packages

Closes #799

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

Merged-by: Ash McKenzie <[email protected]>
Approved-by: Ash McKenzie <[email protected]>
Co-authored-by: Archish <[email protected]>

Author: 2 people

client/transport.go

@@ -1,3 +1,4 @@
+// Package client provides an HTTP client with enhanced logging, tracing, and correlation handling.
 package client
 
 import (
@@ -13,6 +14,7 @@ type transport struct {
 	next http.RoundTripper
 }
 
+// RoundTrip executes a single HTTP transaction, adding logging and tracing capabilities.
 func (rt *transport) RoundTrip(request *http.Request) (*http.Response, error) {
 	ctx := request.Context()
 
@@ -55,10 +57,12 @@ func (rt *transport) RoundTrip(request *http.Request) (*http.Response, error) {
 	return response, nil
 }
 
+// DefaultTransport returns a clone of the default HTTP transport.
 func DefaultTransport() http.RoundTripper {
 	return http.DefaultTransport.(*http.Transport).Clone()
 }
 
+// NewTransport creates a new transport with logging, tracing, and correlation handling.
 func NewTransport(next http.RoundTripper) http.RoundTripper {
 	t := &transport{next: next}
 	return correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(t))

internal/command/shared/disallowedcommand/disallowedcommand.go

@@ -1,7 +1,9 @@
+// Package disallowedcommand provides an error for handling disallowed commands.
 package disallowedcommand
 
 import "errors"
 
 var (
-	Error = errors.New("Disallowed command")
+	// Error is returned when a disallowed command is encountered.
+	Error = errors.New("Disallowed command") //nolint:stylecheck // Used to display the error message to the user.
 )

internal/command/uploadarchive/gitalycall_test.go

@@ -21,15 +21,15 @@ func TestUploadArchive(t *testing.T) {
 	for _, network := range []string{"unix", "tcp", "dns"} {
 		t.Run(fmt.Sprintf("via %s network", network), func(t *testing.T) {
 			gitalyAddress, testServer := testserver.StartGitalyServer(t, network)
-			t.Log(fmt.Sprintf("Server address: %s", gitalyAddress))
+			t.Logf("Server address: %s", gitalyAddress)
 
 			requests := requesthandlers.BuildAllowedWithGitalyHandlers(t, gitalyAddress)
 			url := testserver.StartHTTPServer(t, requests)
 
 			output := &bytes.Buffer{}
 			input := &bytes.Buffer{}
 
-			userId := "1"
+			userID := "1"
 			repo := "group/repo"
 
 			env := sshenv.Env{
@@ -39,7 +39,7 @@ func TestUploadArchive(t *testing.T) {
 			}
 
 			args := &commandargs.Shell{
-				GitlabKeyId: userId,
+				GitlabKeyId: userID,
 				CommandType: commandargs.UploadArchive,
 				SshArgs:     []string{"git-upload-archive", repo},
 				Env:         env,

support/lint_last_known_acceptable.txt

@@ -38,8 +38,6 @@ client/testserver/testserver.go:20:6: exported: exported type TestRequestHandler
 client/testserver/testserver.go:46:12: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
 client/testserver/testserver.go:52:17: Error return value of `server.Serve` is not checked (errcheck)
 client/testserver/testserver.go:117:18: G304: Potential file inclusion via variable (gosec)
-client/transport.go:58:1: exported: exported function DefaultTransport should have comment or be unexported (revive)
-client/transport.go:62:1: exported: exported function NewTransport should have comment or be unexported (revive)
 cmd/gitlab-shell-authorized-keys-check/main.go:40:14: Error return value of `fmt.Fprintf` is not checked (errcheck)
 cmd/gitlab-shell-authorized-keys-check/main.go:43:13: unnecessary conversion (unconvert)
 cmd/gitlab-shell/command/command.go:74: cmd/gitlab-shell/command/command.go:74: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "FIXME: When 1.21+ only Golang is support..." (godox)
@@ -200,10 +198,6 @@ internal/command/shared/customaction/customaction_test.go:115:5: go-require: do
 internal/command/shared/customaction/customaction_test.go:117:5: go-require: do not use require in http handlers (testifylint)
 internal/command/shared/customaction/customaction_test.go:118:5: go-require: do not use require in http handlers (testifylint)
 internal/command/shared/customaction/customaction_test.go:121:5: go-require: do not use require in http handlers (testifylint)
-internal/command/shared/disallowedcommand/disallowedcommand.go:1:1: package-comments: should have a package comment (revive)
-internal/command/shared/disallowedcommand/disallowedcommand.go:6:2: exported: exported var Error should have comment or be unexported (revive)
-internal/command/uploadarchive/gitalycall_test.go:24:4: S1038: should use t.Logf(...) instead of t.Log(fmt.Sprintf(...)) (gosimple)
-internal/command/uploadarchive/gitalycall_test.go:32:4: var-naming: var userId should be userID (revive)
 internal/config/config.go:1:1: package-comments: should have a package comment (revive)
 internal/config/config.go:21:2: G101: Potential hardcoded credentials (gosec)
 internal/config/config.go:24:6: exported: exported type YamlDuration should have comment or be unexported (revive)