Merge branch 'an-linters' into 'master'

Lint Workhorse

See merge request gitlab-org/gitlab-workhorse!293

.gitlab-ci.yml

 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

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

_support/lint.sh

+#!/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
+

_support/validate-formatting.sh

+#!/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

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

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 {

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)
 

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

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

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 {

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)
 

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

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)
 

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)
 

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
 }

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.

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

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

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

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) {

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 {

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{

internal/terminal/terminal.go

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

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

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

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
 }
 

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)

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

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(

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)
 

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)

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) {

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