From c4b06f09ef2b46f63e277a540400ddbc6d91aef5 Mon Sep 17 00:00:00 2001
From: Andrew Newdigate <andrew@gitlab.com>
Date: Thu, 23 Aug 2018 14:10:38 +0000
Subject: [PATCH] Lint Workhorse
---
.gitlab-ci.yml | 5 ++
Makefile | 101 ++++++++++++++++++-------
_support/lint.sh | 11 +++
_support/validate-formatting.sh | 11 +++
archive_test.go | 2 +-
internal/badgateway/roundtripper.go | 36 +++++----
internal/builds/register.go | 1 -
internal/git/command.go | 3 +-
internal/git/git-http.go | 4 +-
internal/git/git-http_test.go | 4 +-
internal/git/info-refs.go | 2 +-
internal/git/pktline_test.go | 2 +-
internal/git/receive-pack.go | 2 +-
internal/git/responsewriter.go | 8 +-
internal/git/snapshot.go | 2 -
internal/git/upload-pack.go | 2 +-
internal/objectstore/prometheus.go | 7 +-
internal/redis/keywatcher.go | 4 +-
internal/redis/redis.go | 2 +-
internal/sendfile/sendfile.go | 1 -
internal/staticpages/servefile_test.go | 2 +-
internal/terminal/auth_checker.go | 2 +-
internal/terminal/terminal.go | 9 +--
internal/terminal/wrappers_test.go | 2 +-
internal/testhelper/gitaly.go | 8 +-
internal/upload/accelerate.go | 2 +-
internal/upstream/metrics.go | 4 +-
internal/upstream/notfoundunless.go | 4 +-
internal/upstream/routes.go | 4 +-
internal/upstream/upstream.go | 10 +--
internal/zipartifacts/metadata.go | 3 +-
main_test.go | 7 +-
sendfile_test.go | 4 +-
33 files changed, 167 insertions(+), 104 deletions(-)
create mode 100755 _support/lint.sh
create mode 100755 _support/validate-formatting.sh
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index dd7650b50440a..2975783971544 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,5 +1,10 @@
image: golang:1.8
+verify:
+ image: golang:1.10
+ script:
+ - make verify
+
.test_template: &test_definition
script:
- apt update -qq && apt install -y unzip bzip2
diff --git a/Makefile b/Makefile
index 11b2258b03626..fad667053e85c 100644
--- a/Makefile
+++ b/Makefile
@@ -10,6 +10,8 @@ VERSION := $(shell git describe)-$(shell date -u +%Y%m%d.%H%M%S)
GOBUILD := go build -ldflags "-X main.Version=$(VERSION)"
EXE_ALL := gitlab-zip-cat gitlab-zip-metadata gitlab-workhorse
+MINIMUM_SUPPORTED_GO_VERSION := 1.8
+
# Some users may have these variables set in their environment, but doing so could break
# their build process, so unset then
unexport GOROOT
@@ -20,6 +22,11 @@ export PATH := $(GOPATH)/bin:$(PATH)
# Returns a list of all non-vendored (local packages)
LOCAL_PACKAGES = $(shell cd "$(PKG_BUILD_DIR)" && GOPATH=$(GOPATH) go list ./... | grep -v -e '^$(PKG)/vendor/' -e '^$(PKG)/ruby/')
+LOCAL_GO_FILES = $(shell find -L $(PKG_BUILD_DIR) -name "*.go" -not -path "$(PKG_BUILD_DIR)/vendor/*" -not -path "$(PKG_BUILD_DIR)/_build/*")
+
+define message
+ @echo "### $(1)"
+endef
.NOTPARALLEL:
@@ -27,7 +34,7 @@ LOCAL_PACKAGES = $(shell cd "$(PKG_BUILD_DIR)" && GOPATH=$(GOPATH) go list ./...
all: clean-build $(EXE_ALL)
$(TARGET_SETUP):
- @echo "### Setting up $(TARGET_SETUP)"
+ $(call message,"Setting up target directory")
rm -rf $(TARGET_DIR)
mkdir -p "$(dir $(PKG_BUILD_DIR))"
ln -sf ../../../.. "$(PKG_BUILD_DIR)"
@@ -35,72 +42,112 @@ $(TARGET_SETUP):
touch "$(TARGET_SETUP)"
gitlab-zip-cat: $(TARGET_SETUP) $(shell find cmd/gitlab-zip-cat/ -name '*.go')
+ $(call message,Building $@)
$(GOBUILD) -o $(BUILD_DIR)/$@ $(PKG)/cmd/$@
gitlab-zip-metadata: $(TARGET_SETUP) $(shell find cmd/gitlab-zip-metadata/ -name '*.go')
+ $(call message,Building $@)
$(GOBUILD) -o $(BUILD_DIR)/$@ $(PKG)/cmd/$@
gitlab-workhorse: $(TARGET_SETUP) $(shell find . -name '*.go' | grep -v '^\./_')
+ $(call message,Building $@)
$(GOBUILD) -o $(BUILD_DIR)/$@ $(PKG)
.PHONY: install
install: gitlab-workhorse gitlab-zip-cat gitlab-zip-metadata
- @echo "### install"
+ $(call message,$@)
mkdir -p $(DESTDIR)$(PREFIX)/bin/
cd $(BUILD_DIR) && install gitlab-workhorse gitlab-zip-cat gitlab-zip-metadata $(DESTDIR)$(PREFIX)/bin/
.PHONY: test
-test: $(TARGET_SETUP) govendor prepare-tests
- @echo "### verifying formatting with go fmt"
- @go fmt $(LOCAL_PACKAGES) | awk '{ print } END { if (NR > 0) { print "Please run go fmt"; exit 1 } }'
-
- _support/detect-context.sh
-
- @echo "### running govendor sync"
- cd $(PKG_BUILD_DIR) && govendor sync
-
- @echo "### running tests"
+test: $(TARGET_SETUP) prepare-tests
+ $(call message,$@)
@go test $(LOCAL_PACKAGES)
@echo SUCCESS
-.PHONY: govendor
-govendor: $(TARGET_SETUP)
- @command -v govendor || go get github.com/kardianos/govendor
-
.PHONY: coverage
coverage: $(TARGET_SETUP) prepare-tests
- @echo "### coverage"
+ $(call message,$@)
@go test -cover -coverprofile=test.coverage $(LOCAL_PACKAGES)
go tool cover -html=test.coverage -o coverage.html
rm -f test.coverage
-.PHONY: fmt
-fmt:
- @echo "### fmt"
- @go fmt $(LOCAL_PACKAGES)
-
.PHONY: clean
clean: clean-workhorse clean-build
- @echo "### clean"
+ $(call message,$@)
rm -rf testdata/data testdata/scratch
.PHONY: clean-workhorse
clean-workhorse:
- @echo "### clean-workhorse"
+ $(call message,$@)
rm -f $(EXE_ALL)
.PHONY: release
release:
- @echo "### release"
+ $(call message,$@)
sh _support/release.sh
.PHONY: clean-build
clean-build:
- @echo "### clean-build"
+ $(call message,$@)
rm -rf $(TARGET_DIR)
.PHONY: prepare-tests
-prepare-tests: testdata/data/group/test.git $(EXE_ALL)
+prepare-tests: govendor-sync testdata/data/group/test.git $(EXE_ALL)
testdata/data/group/test.git:
+ $(call message,$@)
git clone --quiet --bare https://gitlab.com/gitlab-org/gitlab-test.git $@
+
+.PHONY: verify
+verify: lint vet detect-context check-formatting megacheck
+
+.PHONY: lint
+lint: $(TARGET_SETUP) govendor-sync
+ $(call message,Verify: $@)
+ @command -v golint || go get -v golang.org/x/lint/golint
+ @_support/lint.sh $(LOCAL_PACKAGES)
+
+.PHONY: vet
+vet: $(TARGET_SETUP) govendor-sync
+ $(call message,Verify: $@)
+ @go vet $(LOCAL_PACKAGES)
+
+.PHONY: detect-context
+detect-context: $(TARGET_SETUP)
+ $(call message,Verify: $@)
+ _support/detect-context.sh
+
+.PHONY: check-formatting
+check-formatting: $(TARGET_SETUP) install-goimports
+ $(call message,Verify: $@)
+ @_support/validate-formatting.sh $(LOCAL_GO_FILES)
+
+# Megacheck will tailor some responses given a minimum Go version, so pass that through the CLI
+# Additionally, megacheck will not return failure exit codes unless explicitely told to via the
+# `-simple.exit-non-zero` `-unused.exit-non-zero` and `-staticcheck.exit-non-zero` flags
+.PHONY: megacheck
+megacheck: $(TARGET_SETUP) govendor-sync
+ $(call message,Verify: $@)
+ @command -v megacheck || go get -v honnef.co/go/tools/cmd/megacheck
+ @megacheck -go $(MINIMUM_SUPPORTED_GO_VERSION) -simple.exit-non-zero -unused.exit-non-zero -staticcheck.exit-non-zero $(LOCAL_PACKAGES)
+
+# Some vendor components, used for testing are GPL, so we don't distribute them
+# and need to go a sync before using them
+.PHONY: govendor-sync
+govendor-sync: $(TARGET_SETUP)
+ $(call message,$@)
+ @command -v govendor || go get github.com/kardianos/govendor
+ @cd $(PKG_BUILD_DIR) && govendor sync
+
+# In addition to fixing imports, goimports also formats your code in the same style as gofmt
+# so it can be used as a replacement.
+.PHONY: fmt
+fmt: $(TARGET_SETUP) install-goimports
+ $(call message,$@)
+ @goimports -w -l $(LOCAL_GO_FILES)
+
+.PHONY: goimports
+install-goimports: $(TARGET_SETUP)
+ $(call message,$@)
+ @command -v goimports || go get -v golang.org/x/tools/cmd/goimports
diff --git a/_support/lint.sh b/_support/lint.sh
new file mode 100755
index 0000000000000..b016f088d80c8
--- /dev/null
+++ b/_support/lint.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+# Unfortunately, workhorse fails many lint checks which we currently ignore
+LINT_RESULT=$(golint "$@"|grep -Ev 'should have|should be|use ALL_CAPS in Go names')
+
+if [ -n "${LINT_RESULT}" ]; then
+ echo >&2 "Formatting or imports need fixing: 'make fmt'"
+ echo ">>${LINT_RESULT}<<"
+ exit 1
+fi
+
diff --git a/_support/validate-formatting.sh b/_support/validate-formatting.sh
new file mode 100755
index 0000000000000..b4381f580b291
--- /dev/null
+++ b/_support/validate-formatting.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+set -eu
+
+IMPORT_RESULT=$(goimports -e -l "$@")
+
+if [ -n "${IMPORT_RESULT}" ]; then
+ echo >&2 "Formatting or imports need fixing: 'make fmt'"
+ echo "${IMPORT_RESULT}"
+ exit 1
+fi
diff --git a/archive_test.go b/archive_test.go
index d0d13a39c4f3c..6f9877d1361c9 100644
--- a/archive_test.go
+++ b/archive_test.go
@@ -155,7 +155,7 @@ func TestDownloadCacheHit(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- if bytes.Compare(actual, cachedContent) != 0 {
+ if !bytes.Equal(actual, cachedContent) {
t.Fatal("Unexpected file contents in download")
}
}
diff --git a/internal/badgateway/roundtripper.go b/internal/badgateway/roundtripper.go
index fbc272b751201..a5409ce060811 100644
--- a/internal/badgateway/roundtripper.go
+++ b/internal/badgateway/roundtripper.go
@@ -2,6 +2,7 @@ package badgateway
import (
"bytes"
+ "context"
"fmt"
"io/ioutil"
"net"
@@ -12,19 +13,14 @@ import (
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
)
-// Values from http.DefaultTransport
-var DefaultDialer = &net.Dialer{
+// defaultDialer is configured to use the values from http.DefaultTransport
+var defaultDialer = &net.Dialer{
Timeout: 30 * time.Second,
KeepAlive: 30 * time.Second,
+ DualStack: true,
}
-var DefaultTransport = &http.Transport{
- Proxy: http.ProxyFromEnvironment, // from http.DefaultTransport
- Dial: DefaultDialer.Dial, // from http.DefaultTransport
- TLSHandshakeTimeout: 10 * time.Second, // from http.DefaultTransport
-}
-
-// Custom error for pretty Sentry 'issues'
+// Error is a custom error for pretty Sentry 'issues'
type Error struct{ error }
type RoundTripper struct {
@@ -36,24 +32,34 @@ func TestRoundTripper(backend *url.URL) *RoundTripper {
return NewRoundTripper(backend, "", 0, true)
}
+// NewRoundTripper returns a new RoundTripper instance using the provided values
func NewRoundTripper(backend *url.URL, socket string, proxyHeadersTimeout time.Duration, developmentMode bool) *RoundTripper {
- tr := *DefaultTransport
+ // Copied from the definition of http.DefaultTransport. We can't literally copy http.DefaultTransport because of its hidden internal state.
+ tr := &http.Transport{
+ Proxy: http.ProxyFromEnvironment,
+ DialContext: defaultDialer.DialContext,
+ MaxIdleConns: 100,
+ IdleConnTimeout: 90 * time.Second,
+ TLSHandshakeTimeout: 10 * time.Second,
+ ExpectContinueTimeout: 1 * time.Second,
+ }
+
tr.ResponseHeaderTimeout = proxyHeadersTimeout
if backend != nil && socket == "" {
address := mustParseAddress(backend.Host, backend.Scheme)
- tr.Dial = func(_, _ string) (net.Conn, error) {
- return DefaultDialer.Dial("tcp", address)
+ tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+ return defaultDialer.DialContext(ctx, "tcp", address)
}
} else if socket != "" {
- tr.Dial = func(_, _ string) (net.Conn, error) {
- return DefaultDialer.Dial("unix", socket)
+ tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+ return defaultDialer.DialContext(ctx, "unix", socket)
}
} else {
panic("backend is nil and socket is empty")
}
- return &RoundTripper{Transport: &tr, developmentMode: developmentMode}
+ return &RoundTripper{Transport: tr, developmentMode: developmentMode}
}
func mustParseAddress(address, scheme string) string {
diff --git a/internal/builds/register.go b/internal/builds/register.go
index b5ff4fd367c21..8ef922daabc09 100644
--- a/internal/builds/register.go
+++ b/internal/builds/register.go
@@ -49,7 +49,6 @@ var (
)
type largeBodyError struct{ error }
-type watchError struct{ error }
type WatchKeyHandler func(key, value string, timeout time.Duration) (redis.WatchKeyStatus, error)
diff --git a/internal/git/command.go b/internal/git/command.go
index 7d41a3e04b6b1..2d581f75ffe41 100644
--- a/internal/git/command.go
+++ b/internal/git/command.go
@@ -2,10 +2,11 @@ package git
import (
"fmt"
- "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"os"
"os/exec"
"syscall"
+
+ "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
)
var execCommand = exec.Command
diff --git a/internal/git/git-http.go b/internal/git/git-http.go
index 933524df4b2f5..5d194d4418aa5 100644
--- a/internal/git/git-http.go
+++ b/internal/git/git-http.go
@@ -41,12 +41,12 @@ func gitConfigOptions(a *api.Response) []string {
return out
}
-func postRPCHandler(a *api.API, name string, handler func(*GitHttpResponseWriter, *http.Request, *api.Response) error) http.Handler {
+func postRPCHandler(a *api.API, name string, handler func(*HttpResponseWriter, *http.Request, *api.Response) error) http.Handler {
return repoPreAuthorizeHandler(a, func(rw http.ResponseWriter, r *http.Request, ar *api.Response) {
cr := &countReadCloser{ReadCloser: r.Body}
r.Body = cr
- w := NewGitHttpResponseWriter(rw)
+ w := NewHttpResponseWriter(rw)
defer func() {
w.Log(r, cr.Count())
}()
diff --git a/internal/git/git-http_test.go b/internal/git/git-http_test.go
index 24268849593c5..e324eb8667d5a 100644
--- a/internal/git/git-http_test.go
+++ b/internal/git/git-http_test.go
@@ -39,7 +39,7 @@ func TestHandleReceivePack(t *testing.T) {
testHandlePostRpc(t, "git-receive-pack", handleReceivePack)
}
-func testHandlePostRpc(t *testing.T, action string, handler func(*GitHttpResponseWriter, *http.Request, *api.Response) error) {
+func testHandlePostRpc(t *testing.T, action string, handler func(*HttpResponseWriter, *http.Request, *api.Response) error) {
defer func(oldTesting bool) {
Testing = oldTesting
}(Testing)
@@ -60,7 +60,7 @@ func testHandlePostRpc(t *testing.T, action string, handler func(*GitHttpRespons
resp := &api.Response{GL_ID: GL_ID}
rr := httptest.NewRecorder()
- handler(NewGitHttpResponseWriter(rr), req, resp)
+ handler(NewHttpResponseWriter(rr), req, resp)
// Check HTTP status code
if status := rr.Code; status != http.StatusOK {
diff --git a/internal/git/info-refs.go b/internal/git/info-refs.go
index daca32bcaf89f..b3cdebf3eb83b 100644
--- a/internal/git/info-refs.go
+++ b/internal/git/info-refs.go
@@ -21,7 +21,7 @@ func GetInfoRefsHandler(a *api.API) http.Handler {
}
func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response) {
- w := NewGitHttpResponseWriter(rw)
+ w := NewHttpResponseWriter(rw)
// Log 0 bytes in because we ignore the request body (and there usually is none anyway).
defer w.Log(r, 0)
diff --git a/internal/git/pktline_test.go b/internal/git/pktline_test.go
index 63fecd4c137d9..d4be863453865 100644
--- a/internal/git/pktline_test.go
+++ b/internal/git/pktline_test.go
@@ -32,7 +32,7 @@ func TestFailedScanDeepen(t *testing.T) {
}
for _, example := range examples {
- if scanDeepen(bytes.NewReader([]byte(example))) == true {
+ if scanDeepen(bytes.NewReader([]byte(example))) {
t.Fatalf("scanDeepen %q: expected result to be false, got true", example)
}
}
diff --git a/internal/git/receive-pack.go b/internal/git/receive-pack.go
index 073fcd5201b5c..a2783fc39dc1e 100644
--- a/internal/git/receive-pack.go
+++ b/internal/git/receive-pack.go
@@ -13,7 +13,7 @@ import (
// Will not return a non-nil error after the response body has been
// written to.
-func handleReceivePack(w *GitHttpResponseWriter, r *http.Request, a *api.Response) error {
+func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response) error {
action := getService(r)
writePostRPCHeader(w, action)
diff --git a/internal/git/responsewriter.go b/internal/git/responsewriter.go
index 15765aad9f466..32bf6cef54577 100644
--- a/internal/git/responsewriter.go
+++ b/internal/git/responsewriter.go
@@ -43,18 +43,18 @@ func init() {
prometheus.MustRegister(gitHTTPBytes)
}
-type GitHttpResponseWriter struct {
+type HttpResponseWriter struct {
helper.CountingResponseWriter
}
-func NewGitHttpResponseWriter(rw http.ResponseWriter) *GitHttpResponseWriter {
+func NewHttpResponseWriter(rw http.ResponseWriter) *HttpResponseWriter {
gitHTTPSessionsActive.Inc()
- return &GitHttpResponseWriter{
+ return &HttpResponseWriter{
CountingResponseWriter: helper.NewCountingResponseWriter(rw),
}
}
-func (w *GitHttpResponseWriter) Log(r *http.Request, writtenIn int64) {
+func (w *HttpResponseWriter) Log(r *http.Request, writtenIn int64) {
service := getService(r)
agent := getRequestAgent(r)
diff --git a/internal/git/snapshot.go b/internal/git/snapshot.go
index 9c97a44da33fb..da0b506195579 100644
--- a/internal/git/snapshot.go
+++ b/internal/git/snapshot.go
@@ -62,6 +62,4 @@ func (s *snapshot) Inject(w http.ResponseWriter, r *http.Request, sendData strin
if _, err := io.Copy(w, reader); err != nil {
helper.LogError(r, fmt.Errorf("SendSnapshot: copy gitaly output: %v", err))
}
-
- return
}
diff --git a/internal/git/upload-pack.go b/internal/git/upload-pack.go
index 6617b3be34e87..595cf5a40a78d 100644
--- a/internal/git/upload-pack.go
+++ b/internal/git/upload-pack.go
@@ -14,7 +14,7 @@ import (
// Will not return a non-nil error after the response body has been
// written to.
-func handleUploadPack(w *GitHttpResponseWriter, r *http.Request, a *api.Response) error {
+func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) error {
// The body will consist almost entirely of 'have XXX' and 'want XXX'
// lines; these are about 50 bytes long. With a limit of 10MB the client
// can send over 200,000 have/want lines.
diff --git a/internal/objectstore/prometheus.go b/internal/objectstore/prometheus.go
index aee62ecc49465..bfc2beddf34c0 100644
--- a/internal/objectstore/prometheus.go
+++ b/internal/objectstore/prometheus.go
@@ -29,11 +29,8 @@ var (
Buckets: objectStorageUploadTimeBuckets,
})
- objectStorageUploadRequestsFileFailed = objectStorageUploadRequests.WithLabelValues("file-failed")
- objectStorageUploadRequestsRequestFailed = objectStorageUploadRequests.WithLabelValues("request-failed")
- objectStorageUploadRequestsInvalidStatus = objectStorageUploadRequests.WithLabelValues("invalid-status")
- objectStorageUploadRequestsSucceeded = objectStorageUploadRequests.WithLabelValues("succeeded")
- objectStorageUploadRequestsMultipleUploads = objectStorageUploadRequests.WithLabelValues("multiple-uploads")
+ objectStorageUploadRequestsRequestFailed = objectStorageUploadRequests.WithLabelValues("request-failed")
+ objectStorageUploadRequestsInvalidStatus = objectStorageUploadRequests.WithLabelValues("invalid-status")
objectStorageUploadTimeBuckets = []float64{.1, .25, .5, 1, 2.5, 5, 10, 25, 50, 100}
)
diff --git a/internal/redis/keywatcher.go b/internal/redis/keywatcher.go
index cb0c7a57290c2..f7e911618e73d 100644
--- a/internal/redis/keywatcher.go
+++ b/internal/redis/keywatcher.go
@@ -47,9 +47,7 @@ func init() {
}
const (
- keySubChannel = "workhorse:notifications"
- promStatusMiss = "miss"
- promStatusHit = "hit"
+ keySubChannel = "workhorse:notifications"
)
// KeyChan holds a key and a channel
diff --git a/internal/redis/redis.go b/internal/redis/redis.go
index c6718f2706d6e..c42f751900e47 100644
--- a/internal/redis/redis.go
+++ b/internal/redis/redis.go
@@ -89,7 +89,7 @@ func sentinelConn(master string, urls []config.TomlURL) *sentinel.Sentinel {
// For every address it should try to connect to the Sentinel,
// using a short timeout (in the order of a few hundreds of milliseconds).
timeout := 500 * time.Millisecond
- c, err := redis.DialTimeout("tcp", addr, timeout, timeout, timeout)
+ c, err := redis.Dial("tcp", addr, redis.DialConnectTimeout(timeout), redis.DialReadTimeout(timeout), redis.DialWriteTimeout(timeout))
if err != nil {
errorCounter.WithLabelValues("dial", "sentinel").Inc()
return nil, err
diff --git a/internal/sendfile/sendfile.go b/internal/sendfile/sendfile.go
index f536d56cf5f6d..a9310a231823f 100644
--- a/internal/sendfile/sendfile.go
+++ b/internal/sendfile/sendfile.go
@@ -100,7 +100,6 @@ func (s *sendFileResponseWriter) WriteHeader(status int) {
}
s.rw.WriteHeader(s.status)
- return
}
func sendFileFromDisk(w http.ResponseWriter, r *http.Request, file string) {
diff --git a/internal/staticpages/servefile_test.go b/internal/staticpages/servefile_test.go
index b587af7cf504e..9619cba30e3ae 100644
--- a/internal/staticpages/servefile_test.go
+++ b/internal/staticpages/servefile_test.go
@@ -111,7 +111,7 @@ func testServingThePregzippedFile(t *testing.T, enableGzip bool) {
testhelper.AssertResponseCode(t, w, 200)
if enableGzip {
testhelper.AssertResponseWriterHeader(t, w, "Content-Encoding", "gzip")
- if bytes.Compare(w.Body.Bytes(), fileGzipContent.Bytes()) != 0 {
+ if !bytes.Equal(w.Body.Bytes(), fileGzipContent.Bytes()) {
t.Error("We should serve the pregzipped file")
}
} else {
diff --git a/internal/terminal/auth_checker.go b/internal/terminal/auth_checker.go
index f785577a52eed..4f88f00214ca2 100644
--- a/internal/terminal/auth_checker.go
+++ b/internal/terminal/auth_checker.go
@@ -20,7 +20,7 @@ type AuthChecker struct {
Count int64
}
-var ErrAuthChanged = errors.New("Connection closed: authentication changed or endpoint unavailable.")
+var ErrAuthChanged = errors.New("connection closed: authentication changed or endpoint unavailable")
func NewAuthChecker(f AuthCheckerFunc, template *api.TerminalSettings, stopCh chan error) *AuthChecker {
return &AuthChecker{
diff --git a/internal/terminal/terminal.go b/internal/terminal/terminal.go
index 6fafd1882941d..641838a1282a5 100644
--- a/internal/terminal/terminal.go
+++ b/internal/terminal/terminal.go
@@ -1,7 +1,6 @@
package terminal
import (
- "errors"
"fmt"
"net/http"
"time"
@@ -125,10 +124,8 @@ func closeAfterMaxTime(proxy *Proxy, maxSessionTime int) {
}
<-time.After(time.Duration(maxSessionTime) * time.Second)
- proxy.StopCh <- errors.New(
- fmt.Sprintf(
- "Connection closed: session time greater than maximum time allowed - %v seconds",
- maxSessionTime,
- ),
+ proxy.StopCh <- fmt.Errorf(
+ "connection closed: session time greater than maximum time allowed - %v seconds",
+ maxSessionTime,
)
}
diff --git a/internal/terminal/wrappers_test.go b/internal/terminal/wrappers_test.go
index ee139ad79cbfc..7abe08fd83d54 100644
--- a/internal/terminal/wrappers_test.go
+++ b/internal/terminal/wrappers_test.go
@@ -67,7 +67,7 @@ func assertEqual(t *testing.T, expected, actual *fakeConn, msg string, args ...i
t.Fatalf(msg, args...)
}
- if bytes.Compare(expected.data, actual.data) != 0 {
+ if !bytes.Equal(expected.data, actual.data) {
t.Logf("data expected to be %q but was %q: ", expected.data, actual.data)
t.Fatalf(msg, args...)
}
diff --git a/internal/testhelper/gitaly.go b/internal/testhelper/gitaly.go
index db54d439a2da5..1f695d85d9e81 100644
--- a/internal/testhelper/gitaly.go
+++ b/internal/testhelper/gitaly.go
@@ -15,8 +15,8 @@ import (
pb "gitlab.com/gitlab-org/gitaly-proto/go"
"golang.org/x/net/context"
- "google.golang.org/grpc"
"google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
)
type GitalyTestServer struct {
@@ -158,7 +158,7 @@ func (s *GitalyTestServer) PostReceivePack(stream pb.SmartHTTPService_PostReceiv
data = append(data, req.GetData()...)
}
- nSends, err := sendBytes(data, 100, func(p []byte) error {
+ nSends, _ := sendBytes(data, 100, func(p []byte) error {
return stream.Send(&pb.PostReceivePackResponse{Data: p})
})
@@ -204,7 +204,7 @@ func (s *GitalyTestServer) PostUploadPack(stream pb.SmartHTTPService_PostUploadP
data = append(data, req.GetData()...)
}
- nSends, err := sendBytes(data, 100, func(p []byte) error {
+ nSends, _ := sendBytes(data, 100, func(p []byte) error {
return stream.Send(&pb.PostUploadPackResponse{Data: p})
})
@@ -354,7 +354,7 @@ func sendBytes(data []byte, chunkSize int, sender func([]byte) error) (int, erro
func (s *GitalyTestServer) finalError() error {
if code := s.finalMessageCode; code != codes.OK {
- return grpc.Errorf(code, "error as specified by test")
+ return status.Errorf(code, "error as specified by test")
}
return nil
diff --git a/internal/upload/accelerate.go b/internal/upload/accelerate.go
index b2ffd27550ca9..f7613e84f9296 100644
--- a/internal/upload/accelerate.go
+++ b/internal/upload/accelerate.go
@@ -40,7 +40,7 @@ func (s *savedFileTracker) ProcessFile(_ context.Context, fieldName string, file
return nil
}
-func (_ *savedFileTracker) ProcessField(_ context.Context, _ string, _ *multipart.Writer) error {
+func (s *savedFileTracker) ProcessField(_ context.Context, _ string, _ *multipart.Writer) error {
return nil
}
diff --git a/internal/upstream/metrics.go b/internal/upstream/metrics.go
index ef8d856ea4956..f5bc276f403f0 100644
--- a/internal/upstream/metrics.go
+++ b/internal/upstream/metrics.go
@@ -111,9 +111,7 @@ func init() {
}
func instrumentRoute(next http.Handler, method string, regexpStr string) http.Handler {
- var handler http.Handler
-
- handler = next
+ handler := next
handler = promhttp.InstrumentHandlerCounter(httpRequestsTotal.MustCurryWith(map[string]string{"route": regexpStr}), handler)
handler = promhttp.InstrumentHandlerDuration(httpRequestDurationSeconds.MustCurryWith(map[string]string{"route": regexpStr}), handler)
diff --git a/internal/upstream/notfoundunless.go b/internal/upstream/notfoundunless.go
index 60cadceef69d9..3bbe3e873a46c 100644
--- a/internal/upstream/notfoundunless.go
+++ b/internal/upstream/notfoundunless.go
@@ -5,7 +5,7 @@ import "net/http"
func NotFoundUnless(pass bool, handler http.Handler) http.Handler {
if pass {
return handler
- } else {
- return http.HandlerFunc(http.NotFound)
}
+
+ return http.HandlerFunc(http.NotFound)
}
diff --git a/internal/upstream/routes.go b/internal/upstream/routes.go
index fd0f14e2034d9..83275d7699d1c 100644
--- a/internal/upstream/routes.go
+++ b/internal/upstream/routes.go
@@ -98,13 +98,13 @@ func (ro *routeEntry) isMatch(cleanedPath string, req *http.Request) bool {
// We match against URI not containing the relativeUrlRoot:
// see upstream.ServeHTTP
-func (u *Upstream) configureRoutes() {
+func (u *upstream) configureRoutes() {
api := apipkg.NewAPI(
u.Backend,
u.Version,
u.RoundTripper,
)
- static := &staticpages.Static{u.DocumentRoot}
+ static := &staticpages.Static{DocumentRoot: u.DocumentRoot}
proxy := senddata.SendData(
sendfile.SendFile(
apipkg.Block(
diff --git a/internal/upstream/upstream.go b/internal/upstream/upstream.go
index 93f5ef7b1847e..3b4b342f34a5c 100644
--- a/internal/upstream/upstream.go
+++ b/internal/upstream/upstream.go
@@ -27,15 +27,15 @@ var (
}
)
-type Upstream struct {
+type upstream struct {
config.Config
URLPrefix urlprefix.Prefix
Routes []routeEntry
RoundTripper *badgateway.RoundTripper
}
-func NewUpstream(cfg config.Config) *Upstream {
- up := Upstream{
+func NewUpstream(cfg config.Config) http.Handler {
+ up := upstream{
Config: cfg,
}
if up.Backend == nil {
@@ -47,7 +47,7 @@ func NewUpstream(cfg config.Config) *Upstream {
return &up
}
-func (u *Upstream) configureURLPrefix() {
+func (u *upstream) configureURLPrefix() {
relativeURLRoot := u.Backend.Path
if !strings.HasSuffix(relativeURLRoot, "/") {
relativeURLRoot += "/"
@@ -55,7 +55,7 @@ func (u *Upstream) configureURLPrefix() {
u.URLPrefix = urlprefix.Prefix(relativeURLRoot)
}
-func (u *Upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) {
+func (u *upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) {
// Automatic quasi-intelligent X-Forwarded-For parsing
r.RemoteAddr = xff.GetRemoteAddr(r)
diff --git a/internal/zipartifacts/metadata.go b/internal/zipartifacts/metadata.go
index 50267a2e2baa2..1ecf52deafbf9 100644
--- a/internal/zipartifacts/metadata.go
+++ b/internal/zipartifacts/metadata.go
@@ -29,6 +29,7 @@ func newMetadata(file *zip.File) metadata {
}
return metadata{
+ //lint:ignore SA1019 Remove this once the minimum supported version is go 1.10 (go 1.9 and down do not support an alternative)
Modified: file.ModTime().Unix(),
Mode: strconv.FormatUint(uint64(file.Mode().Perm()), 8),
CRC: file.CRC32,
@@ -89,7 +90,7 @@ func GenerateZipMetadata(w io.Writer, archive *zip.Reader) error {
// Sort paths
sortedPaths := make([]string, 0, len(zipMap))
- for path, _ := range zipMap {
+ for path := range zipMap {
sortedPaths = append(sortedPaths, path)
}
sort.Strings(sortedPaths)
diff --git a/main_test.go b/main_test.go
index d4fe4c15a5b42..8ee0957e671c9 100644
--- a/main_test.go
+++ b/main_test.go
@@ -377,8 +377,6 @@ func sendDataResponder(command string, literalJSON string) *httptest.Server {
if _, err := fmt.Fprintf(w, "gibberish"); err != nil {
panic(err)
}
-
- return
}
return testhelper.TestServerWithHandler(regexp.MustCompile(`.`), handler)
@@ -569,10 +567,7 @@ func setupStaticFile(fpath, content string) error {
return err
}
staticFile := path.Join(*documentRoot, fpath)
- if err := ioutil.WriteFile(staticFile, []byte(content), 0666); err != nil {
- return err
- }
- return nil
+ return ioutil.WriteFile(staticFile, []byte(content), 0666)
}
func prepareDownloadDir(t *testing.T) {
diff --git a/sendfile_test.go b/sendfile_test.go
index 7095e3a7ea752..c3bc79f18359d 100644
--- a/sendfile_test.go
+++ b/sendfile_test.go
@@ -65,7 +65,7 @@ func allowedXSendfileDownload(t *testing.T, contentFilename string, filePath str
if err != nil {
t.Fatal(err)
}
- if bytes.Compare(actual, contentBytes) != 0 {
+ if !bytes.Equal(actual, contentBytes) {
t.Fatal("Unexpected file contents in download")
}
}
@@ -95,7 +95,7 @@ func deniedXSendfileDownload(t *testing.T, contentFilename string, filePath stri
if err != nil {
t.Fatal(err)
}
- if bytes.Compare(actual, []byte("Denied")) != 0 {
+ if !bytes.Equal(actual, []byte("Denied")) {
t.Fatal("Unexpected file contents in download")
}
}
--
GitLab