Merge branch 'ashmckenzie/improve-lint-error-reporting' into 'main'

Improve lint error reporting

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



Merged-by: Igor Drozdov <idrozdov@gitlab.com>
Approved-by: Oscar Tovar <otovar@gitlab.com>
Approved-by: Igor Drozdov <idrozdov@gitlab.com>
Reviewed-by: Oscar Tovar <otovar@gitlab.com>
Co-authored-by: Ash McKenzie <amckenzie@gitlab.com>

.gitlab-ci.yml

@@ -16,7 +16,7 @@ variables:
   TRANSFER_METER_FREQUENCY: "1s"
   DOCKER_VERSION: "20.10.15"
   BUNDLE_FROZEN: "true"
-  GO_VERSION: "golang-1.22"
+  GO_VERSION: "golang-1.23"
   GOPATH: $CI_PROJECT_DIR/.GOPATH
   DEBIAN_VERSION: "bookworm"
   RUBY_VERSION: "3.2.4"

.golangci.yml

@@ -61,6 +61,8 @@ output:
   # print linter name in the end of issue text, default is true
   print-linter-name: true
 
+  sort-results: true
+
 # all available settings of specific linters
 linters-settings:
   errcheck:

Makefile

@@ -86,13 +86,15 @@ coverage: coverage_golang
 coverage_golang:
 	[ -f cover.out ] && go tool cover -func cover.out
 
-lint: ${GOLANGCI_LINT_FILE}
-	${GOLANGCI_LINT_FILE} --version
-	${GOLANGCI_LINT_FILE} run --issues-exit-code 0 --print-issued-lines=false ${GOLANGCI_LINT_ARGS}
+lint:
+	@support/lint.sh ./...
+
+golangci: ${GOLANGCI_LINT_FILE}
+	@${GOLANGCI_LINT_FILE} run --issues-exit-code 0 --print-issued-lines=false ${GOLANGCI_LINT_ARGS}
 
 ${GOLANGCI_LINT_FILE}:
-	mkdir -p $(shell dirname ${GOLANGCI_LINT_FILE})
-	curl -L https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${ARCH}.tar.gz | tar --strip-components 1 -zOxf - golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${ARCH}/golangci-lint > ${GOLANGCI_LINT_FILE} && chmod +x ${GOLANGCI_LINT_FILE}
+	@mkdir -p $(shell dirname ${GOLANGCI_LINT_FILE})
+	@curl -L https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${ARCH}.tar.gz | tar --strip-components 1 -zOxf - golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${ARCH}/golangci-lint > ${GOLANGCI_LINT_FILE} && chmod +x ${GOLANGCI_LINT_FILE}
 
 setup: make_necessary_dirs bin/gitlab-shell
 

client/client_test.go

@@ -36,31 +36,31 @@ func TestClients(t *testing.T) {
 	}{
 		{
 			desc:   "Socket client",
-			server: testserver.StartSocketHttpServer,
+			server: testserver.StartSocketHTTPServer,
 			secret: secret,
 		},
 		{
 			desc:            "Socket client with a relative URL at /",
 			relativeURLRoot: "/",
-			server:          testserver.StartSocketHttpServer,
+			server:          testserver.StartSocketHTTPServer,
 			secret:          secret,
 		},
 		{
 			desc:            "Socket client with relative URL at /gitlab",
 			relativeURLRoot: "/gitlab",
-			server:          testserver.StartSocketHttpServer,
+			server:          testserver.StartSocketHTTPServer,
 			secret:          secret,
 		},
 		{
 			desc:   "Http client",
-			server: testserver.StartHttpServer,
+			server: testserver.StartHTTPServer,
 			secret: secret,
 		},
 		{
 			desc:   "Https client",
 			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 			server: func(t *testing.T, handlers []testserver.TestRequestHandler) string {
-				return testserver.StartHttpsServer(t, handlers, "")
+				return testserver.StartHTTPSServer(t, handlers, "")
 			},
 			secret: secret,
 		},
@@ -68,13 +68,13 @@ func TestClients(t *testing.T) {
 			desc:   "Secret with newlines",
 			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 			server: func(t *testing.T, handlers []testserver.TestRequestHandler) string {
-				return testserver.StartHttpsServer(t, handlers, "")
+				return testserver.StartHTTPSServer(t, handlers, "")
 			},
 			secret: "\n" + secret + "\n",
 		},
 		{
 			desc:   "Retry client",
-			server: testserver.StartRetryHttpServer,
+			server: testserver.StartRetryHTTPServer,
 			secret: secret,
 		},
 	}

client/httpclient_test.go

@@ -125,7 +125,7 @@ func TestRequestWithUserAgent(t *testing.T) {
 }
 
 func setup(t *testing.T, username, password string, requests []testserver.TestRequestHandler) *GitlabNetClient {
-	url := testserver.StartHttpServer(t, requests)
+	url := testserver.StartHTTPServer(t, requests)
 
 	httpClient, err := NewHTTPClientWithOpts(url, "", "", "", 1, nil)
 	require.NoError(t, err)

client/httpsclient_test.go

@@ -123,7 +123,7 @@ func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPat
 		},
 	}
 
-	url := testserver.StartHttpsServer(t, requests, clientCAPath)
+	url := testserver.StartHTTPSServer(t, requests, clientCAPath)
 
 	opts := defaultHttpOpts
 	if clientCertPath != "" && clientKeyPath != "" {

client/testserver/testserver.go

@@ -22,7 +22,8 @@ type TestRequestHandler struct {
 	Handler func(w http.ResponseWriter, r *http.Request)
 }
 
-func StartSocketHttpServer(t *testing.T, handlers []TestRequestHandler) string {
+// StartSocketHTTPServer starts a socket based HTTP server
+func StartSocketHTTPServer(t *testing.T, handlers []TestRequestHandler) string {
 	t.Helper()
 
 	// We can't use t.TempDir() here because it will create a directory that
@@ -55,7 +56,8 @@ func StartSocketHttpServer(t *testing.T, handlers []TestRequestHandler) string {
 	return url
 }
 
-func StartHttpServer(t *testing.T, handlers []TestRequestHandler) string {
+// StartHTTPServer starts a TCP based HTTP server
+func StartHTTPServer(t *testing.T, handlers []TestRequestHandler) string {
 	t.Helper()
 
 	server := httptest.NewServer(buildHandler(handlers))
@@ -64,7 +66,8 @@ func StartHttpServer(t *testing.T, handlers []TestRequestHandler) string {
 	return server.URL
 }
 
-func StartRetryHttpServer(t *testing.T, handlers []TestRequestHandler) string {
+// StartRetryHTTPServer starts a TCP based HTTP server with retry capabilities
+func StartRetryHTTPServer(t *testing.T, handlers []TestRequestHandler) string {
 	attempts := map[string]int{}
 
 	retryMiddileware := func(next func(w http.ResponseWriter, r *http.Request)) http.Handler {
@@ -92,7 +95,8 @@ func StartRetryHttpServer(t *testing.T, handlers []TestRequestHandler) string {
 	return server.URL
 }
 
-func StartHttpsServer(t *testing.T, handlers []TestRequestHandler, clientCAPath string) string {
+// StartHTTPSServer starts a TCP based HTTPS capable server
+func StartHTTPSServer(t *testing.T, handlers []TestRequestHandler, clientCAPath string) string {
 	t.Helper()
 
 	testRoot := testhelper.PrepareTestRootDir(t)

cmd/gitlab-sshd/acceptance_test.go

@@ -137,7 +137,7 @@ func startGitOverHTTPServer(t *testing.T) string {
 		},
 	}
 
-	return testserver.StartHttpServer(t, requests)
+	return testserver.StartHTTPServer(t, requests)
 }
 
 func buildAllowedResponse(t *testing.T, filename string) string {

internal/command/authorizedkeys/authorized_keys_test.go

@@ -43,7 +43,7 @@ var (
 )
 
 func TestExecute(t *testing.T) {
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 
 	defaultConfig := &config.Config{RootDir: "/tmp", GitlabUrl: url}
 

internal/command/discover/discover_test.go

@@ -45,7 +45,7 @@ var requests = []testserver.TestRequestHandler{
 }
 
 func TestExecute(t *testing.T) {
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 
 	testCases := []struct {
 		desc             string
@@ -96,7 +96,7 @@ func TestExecute(t *testing.T) {
 }
 
 func TestFailingExecute(t *testing.T) {
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 
 	testCases := []struct {
 		desc          string

internal/command/gitauditevent/audit_test.go

@@ -47,7 +47,7 @@ func TestGitAudit(t *testing.T) {
 		CommandType: commandargs.UploadArchive,
 	}
 
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 	Audit(context.Background(), s.CommandType, &config.Config{GitlabUrl: url}, &accessverifier.Response{
 		Username: testUsername,
 		Repo:     testRepo,

internal/command/githttp/pull_test.go

@@ -122,7 +122,7 @@ func TestPullExecuteWithFailedInfoRefs(t *testing.T) {
 				},
 			}
 
-			url := testserver.StartHttpServer(t, requests)
+			url := testserver.StartHTTPServer(t, requests)
 
 			cmd := &PullCommand{
 				Config: &config.Config{GitlabUrl: url},
@@ -186,7 +186,7 @@ func setupPull(t *testing.T, uploadPackStatusCode int) string {
 		},
 	}
 
-	return testserver.StartHttpServer(t, requests)
+	return testserver.StartHTTPServer(t, requests)
 }
 
 func setupSSHPull(t *testing.T, uploadPackStatusCode int) string {
@@ -208,5 +208,5 @@ func setupSSHPull(t *testing.T, uploadPackStatusCode int) string {
 		},
 	}
 
-	return testserver.StartHttpServer(t, requests)
+	return testserver.StartHTTPServer(t, requests)
 }

internal/command/githttp/push_test.go

@@ -81,7 +81,7 @@ func TestExecuteWithFailedInfoRefs(t *testing.T) {
 				},
 			}
 
-			url := testserver.StartHttpServer(t, requests)
+			url := testserver.StartHTTPServer(t, requests)
 
 			cmd := &PushCommand{
 				Config: &config.Config{GitlabUrl: url},
@@ -182,7 +182,7 @@ func setup(t *testing.T, receivePackStatusCode int) (string, io.Reader) {
 		},
 	}
 
-	return testserver.StartHttpServer(t, requests), input
+	return testserver.StartHTTPServer(t, requests), input
 }
 
 func setupSSHPush(t *testing.T, uploadPackStatusCode int) string {
@@ -204,5 +204,5 @@ func setupSSHPush(t *testing.T, uploadPackStatusCode int) string {
 		},
 	}
 
-	return testserver.StartHttpServer(t, requests)
+	return testserver.StartHTTPServer(t, requests)
 }

internal/command/healthcheck/healthcheck_test.go

@@ -45,7 +45,7 @@ func buildTestHandlers(code int, rsp *healthcheck.Response) []testserver.TestReq
 }
 
 func TestExecute(t *testing.T) {
-	url := testserver.StartSocketHttpServer(t, okHandlers)
+	url := testserver.StartSocketHTTPServer(t, okHandlers)
 
 	buffer := &bytes.Buffer{}
 	cmd := &Command{
@@ -60,7 +60,7 @@ func TestExecute(t *testing.T) {
 }
 
 func TestFailingRedisExecute(t *testing.T) {
-	url := testserver.StartSocketHttpServer(t, badRedisHandlers)
+	url := testserver.StartSocketHTTPServer(t, badRedisHandlers)
 
 	buffer := &bytes.Buffer{}
 	cmd := &Command{
@@ -74,7 +74,7 @@ func TestFailingRedisExecute(t *testing.T) {
 }
 
 func TestFailingAPIExecute(t *testing.T) {
-	url := testserver.StartSocketHttpServer(t, brokenHandlers)
+	url := testserver.StartSocketHTTPServer(t, brokenHandlers)
 
 	buffer := &bytes.Buffer{}
 	cmd := &Command{

internal/command/lfsauthenticate/lfsauthenticate_test.go

@@ -22,7 +22,7 @@ import (
 
 func TestFailedRequests(t *testing.T) {
 	requests := requesthandlers.BuildDisallowedByAPIHandlers(t)
-	url := testserver.StartHttpServer(t, requests)
+	url := testserver.StartHTTPServer(t, requests)
 
 	testCases := []struct {
 		desc           string
@@ -124,7 +124,7 @@ func TestLfsAuthenticateRequests(t *testing.T) {
 		},
 	}
 
-	url := testserver.StartHttpServer(t, requests)
+	url := testserver.StartHTTPServer(t, requests)
 
 	testCases := []struct {
 		desc           string

internal/command/lfstransfer/lfstransfer_test.go

@@ -1465,7 +1465,7 @@ func setup(t *testing.T, keyID string, repo string, op string) (string, *Command
 		},
 	}
 
-	url = testserver.StartHttpServer(t, requests)
+	url = testserver.StartHTTPServer(t, requests)
 
 	inputSource, inputSink := io.Pipe()
 	outputSource, outputSink := io.Pipe()

internal/command/personalaccesstoken/personalaccesstoken_test.go

@@ -78,7 +78,7 @@ const (
 func TestExecute(t *testing.T) {
 	setup(t)
 
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 
 	testCases := []struct {
 		desc           string
@@ -139,7 +139,7 @@ func TestExecute(t *testing.T) {
 				GitlabKeyId: "badresponse",
 				SshArgs:     []string{cmdname, "newtoken", "read_api,read_repository"},
 			},
-			expectedError: "Parsing failed",
+			expectedError: "parsing failed",
 		},
 		{
 			desc: "when API returns an error",

internal/command/receivepack/gitalycall_test.go

@@ -24,7 +24,7 @@ func TestReceivePack(t *testing.T) {
 			t.Log(fmt.Sprintf("Server address: %s", gitalyAddress))
 
 			requests := requesthandlers.BuildAllowedWithGitalyHandlers(t, gitalyAddress)
-			url := testserver.StartHttpServer(t, requests)
+			url := testserver.StartHTTPServer(t, requests)
 
 			testCases := []struct {
 				username string

internal/command/receivepack/receivepack_test.go

@@ -47,7 +47,7 @@ func TestCustomReceivePack(t *testing.T) {
 }
 
 func setup(t *testing.T, keyID string, requests []testserver.TestRequestHandler) (*Command, *bytes.Buffer) {
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 
 	output := &bytes.Buffer{}
 	input := bytes.NewBufferString("input")

internal/command/shared/accessverifier/accessverifier_test.go

@@ -50,7 +50,7 @@ func setup(t *testing.T) (*Command, *bytes.Buffer, *bytes.Buffer) {
 		},
 	}
 
-	url := testserver.StartSocketHttpServer(t, requests)
+	url := testserver.StartSocketHTTPServer(t, requests)
 
 	errBuf := &bytes.Buffer{}
 	outBuf := &bytes.Buffer{}