ci: Use golangci's github action for linting (#3794)

* ci: Use golangci's github action for linting

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix most of the staticcheck lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the prealloc lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the misspell lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the varcheck lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the errcheck lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the bodyclose lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the deadcode lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the unused lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the gosec lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the gosimple lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the ineffassign lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Fix the staticcheck lint errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Revert the misspell change, use a neutral English

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Remove broken golangci-lint CI job

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Re-add errantly-removed weakrand initialization

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* don't break the loop and return

* Removing extra handling for null rootKey

* unignore RegisterModule/RegisterAdapter

Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* single-line log message

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

* Fix lint after a1808b0dbf209c615e438a496d257ce5e3acdce2 was merged

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Revert ticker change, ignore it instead

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Ignore some of the write errors

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Remove blank line

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Use lifetime

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* close immediately

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

* Preallocate configVals

Signed-off-by: Dave Henderson <dhenderson@gmail.com>

* Update modules/caddytls/distributedstek/distributedstek.go

Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

.github/workflows/ci.yml

@@ -148,20 +148,6 @@ jobs:
         env:
           SSH_KEY: ${{ secrets.S390X_SSH_KEY }}
 
-  # From https://github.com/reviewdog/action-golangci-lint
-  golangci-lint:
-    name: runner / golangci-lint
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout code into the Go module directory
-        uses: actions/checkout@v2
-
-      - name: Run golangci-lint
-        uses: reviewdog/action-golangci-lint@v1
-        # uses: docker://reviewdog/action-golangci-lint:v1 # pre-build docker image
-        with:
-          github_token: ${{ secrets.github_token }}
-
   goreleaser-check:
     runs-on: ubuntu-latest
     steps:

.github/workflows/lint.yml

@@ -0,0 +1,23 @@
+name: Lint
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  # From https://github.com/golangci/golangci-lint-action
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v2
+        with:
+          version: v1.31
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true

.golangci.yml

@@ -1,21 +1,68 @@
 linters-settings:
   errcheck:
-    ignore: fmt:.*,io/ioutil:^Read.*,github.com/caddyserver/caddy/v2/caddyconfig:RegisterAdapter,github.com/caddyserver/caddy/v2:RegisterModule
+    ignore: fmt:.*,io/ioutil:^Read.*,go.uber.org/zap/zapcore:^Add.*
     ignoretests: true
-  misspell:
-    locale: US
 
 linters:
+  disable-all: true
   enable:
     - bodyclose
-    - prealloc
-    - unconvert
+    - deadcode
     - errcheck
     - gofmt
     - goimports
     - gosec
+    - gosimple
+    - govet
     - ineffassign
     - misspell
+    - prealloc
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unused
+    - varcheck
+  # these are implicitly disabled:
+  # - asciicheck
+  # - depguard
+  # - dogsled
+  # - dupl
+  # - exhaustive
+  # - exportloopref
+  # - funlen
+  # - gci
+  # - gochecknoglobals
+  # - gochecknoinits
+  # - gocognit
+  # - goconst
+  # - gocritic
+  # - gocyclo
+  # - godot
+  # - godox
+  # - goerr113
+  # - gofumpt
+  # - goheader
+  # - golint
+  # - gomnd
+  # - gomodguard
+  # - goprintffuncname
+  # - interfacer
+  # - lll
+  # - maligned
+  # - nakedret
+  # - nestif
+  # - nlreturn
+  # - noctx
+  # - nolintlint
+  # - rowserrcheck
+  # - scopelint
+  # - sqlclosecheck
+  # - stylecheck
+  # - testpackage
+  # - unparam
+  # - whitespace
+  # - wsl
 
 run:
   # default concurrency is a available CPU number.

admin.go

@@ -398,7 +398,10 @@ func (h adminHandler) handleError(w http.ResponseWriter, r *http.Request, err er
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(apiErr.Code)
-	json.NewEncoder(w).Encode(apiErr)
+	encErr := json.NewEncoder(w).Encode(apiErr)
+	if encErr != nil {
+		Log().Named("admin.api").Error("failed to encode error response", zap.Error(encErr))
+	}
 }
 
 // checkHost returns a handler that wraps next such that
@@ -838,7 +841,7 @@ var (
 // will get deleted before the process gracefully exits.
 func PIDFile(filename string) error {
 	pid := []byte(strconv.Itoa(os.Getpid()) + "\n")
-	err := ioutil.WriteFile(filename, pid, 0644)
+	err := ioutil.WriteFile(filename, pid, 0600)
 	if err != nil {
 		return err
 	}

caddyconfig/httpcaddyfile/builtins.go

@@ -356,7 +356,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 	}
 
 	// begin building the final config values
-	var configVals []ConfigValue
+	configVals := []ConfigValue{}
 
 	// certificate loaders
 	if len(fileLoader) > 0 {

caddytest/caddytest.go

@@ -124,10 +124,10 @@ func (tc *Tester) initServer(rawConfig string, configType string) error {
 				return
 			}
 			defer res.Body.Close()
-			body, err := ioutil.ReadAll(res.Body)
+			body, _ := ioutil.ReadAll(res.Body)
 
 			var out bytes.Buffer
-			json.Indent(&out, body, "", "  ")
+			_ = json.Indent(&out, body, "", "  ")
 			tc.t.Logf("----------- failed with config -----------\n%s", out.String())
 		}
 	})
@@ -221,10 +221,11 @@ func isCaddyAdminRunning() error {
 	client := &http.Client{
 		Timeout: Default.LoadRequestTimeout,
 	}
-	_, err := client.Get(fmt.Sprintf("http://localhost:%d/config/", Default.AdminPort))
+	resp, err := client.Get(fmt.Sprintf("http://localhost:%d/config/", Default.AdminPort))
 	if err != nil {
 		return errors.New("caddy integration test caddy server not running. Expected to be listening on localhost:2019")
 	}
+	resp.Body.Close()
 
 	return nil
 }
@@ -272,7 +273,7 @@ func CreateTestingTransport() *http.Transport {
 		IdleConnTimeout:       90 * time.Second,
 		TLSHandshakeTimeout:   5 * time.Second,
 		ExpectContinueTimeout: 1 * time.Second,
-		TLSClientConfig:       &tls.Config{InsecureSkipVerify: true},
+		TLSClientConfig:       &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
 	}
 }
 

cmd/commandfuncs.go

@@ -96,7 +96,7 @@ func cmdStart(fl Flags) (int, error) {
 	// started yet, and writing synchronously would result
 	// in a deadlock
 	go func() {
-		stdinpipe.Write(expect)
+		_, _ = stdinpipe.Write(expect)
 		stdinpipe.Close()
 	}()
 

cmd/main.go

@@ -236,6 +236,7 @@ func watchConfigFile(filename, adapterName string) {
 	}
 
 	// begin poller
+	//nolint:staticcheck
 	for range time.Tick(1 * time.Second) {
 		// get the file info
 		info, err := os.Stat(filename)

cmd/proc_posix.go

@@ -1,37 +0,0 @@
-// Copyright 2015 Matthew Holt and The Caddy Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// +build !windows
-
-package caddycmd
-
-import (
-	"fmt"
-	"os"
-	"path/filepath"
-	"syscall"
-)
-
-func gracefullyStopProcess(pid int) error {
-	fmt.Print("Graceful stop... ")
-	err := syscall.Kill(pid, syscall.SIGINT)
-	if err != nil {
-		return fmt.Errorf("kill: %v", err)
-	}
-	return nil
-}
-
-func getProcessName() string {
-	return filepath.Base(os.Args[0])
-}

cmd/proc_windows.go

@@ -1,44 +0,0 @@
-// Copyright 2015 Matthew Holt and The Caddy Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package caddycmd
-
-import (
-	"fmt"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"strconv"
-)
-
-func gracefullyStopProcess(pid int) error {
-	fmt.Print("Forceful stop... ")
-	// process on windows will not stop unless forced with /f
-	cmd := exec.Command("taskkill", "/pid", strconv.Itoa(pid), "/f")
-	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("taskkill: %v", err)
-	}
-	return nil
-}
-
-// On Windows the app name passed in os.Args[0] will match how
-// caddy was started eg will match caddy or caddy.exe.
-// So return appname with .exe for consistency
-func getProcessName() string {
-	base := filepath.Base(os.Args[0])
-	if filepath.Ext(base) == "" {
-		return base + ".exe"
-	}
-	return base
-}

go.sum

@@ -354,6 +354,8 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/libdns/libdns v0.1.0 h1:0ctCOrVJsVzj53mop1angHp/pE3hmAhP7KiHvR0HD04=
 github.com/libdns/libdns v0.1.0/go.mod h1:yQCXzk1lEZmmCPa857bnk4TsOiqYasqpyOEeSObbb40=
 github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4=
+github.com/lucas-clemente/quic-go v0.19.2 h1:w8BBYUx5Z+kNpeaOeQW/KzcNsKWhh4O6PeQhb0nURPg=
+github.com/lucas-clemente/quic-go v0.19.2/go.mod h1:ZUygOqIoai0ASXXLJ92LTnKdbqh9MHCLTX6Nr1jUrK0=
 github.com/lunixbochs/vtclean v1.0.0 h1:xu2sLAri4lGiovBDQKxl5mrXyESr3gUr5m5SM5+LVb8=
 github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm/+2c2E2WMI=
 github.com/magiconair/properties v1.7.6/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=

listeners.go

@@ -129,9 +129,9 @@ func (fcl *fakeCloseListener) Accept() (net.Conn, error) {
 	if *fcl.deadline {
 		switch ln := fcl.Listener.(type) {
 		case *net.TCPListener:
-			ln.SetDeadline(time.Time{})
+			_ = ln.SetDeadline(time.Time{})
 		case *net.UnixListener:
-			ln.SetDeadline(time.Time{})
+			_ = ln.SetDeadline(time.Time{})
 		}
 		*fcl.deadline = false
 	}
@@ -167,9 +167,9 @@ func (fcl *fakeCloseListener) Close() error {
 		if !*fcl.deadline {
 			switch ln := fcl.Listener.(type) {
 			case *net.TCPListener:
-				ln.SetDeadline(time.Now().Add(-1 * time.Minute))
+				_ = ln.SetDeadline(time.Now().Add(-1 * time.Minute))
 			case *net.UnixListener:
-				ln.SetDeadline(time.Now().Add(-1 * time.Minute))
+				_ = ln.SetDeadline(time.Now().Add(-1 * time.Minute))
 			}
 			*fcl.deadline = true
 		}

logging.go

@@ -458,7 +458,7 @@ func (cl *CustomLog) buildCore() {
 		if cl.Sampling.Thereafter == 0 {
 			cl.Sampling.Thereafter = 100
 		}
-		c = zapcore.NewSampler(c, cl.Sampling.Interval,
+		c = zapcore.NewSamplerWithOptions(c, cl.Sampling.Interval,
 			cl.Sampling.First, cl.Sampling.Thereafter)
 	}
 	cl.core = c

modules/caddyhttp/app.go

@@ -363,6 +363,7 @@ func (app *App) Start() error {
 								ErrorLog:  serverLogger,
 							},
 						}
+						//nolint:errcheck
 						go h3srv.Serve(h3ln)
 						app.h3servers = append(app.h3servers, h3srv)
 						app.h3listeners = append(app.h3listeners, h3ln)
@@ -391,6 +392,7 @@ func (app *App) Start() error {
 					zap.Bool("tls", useTLS),
 				)
 
+				//nolint:errcheck
 				go s.Serve(ln)
 				app.servers = append(app.servers, s)
 			}

modules/caddyhttp/autohttps.go

@@ -523,7 +523,10 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri
 		// our base/catch-all policy - this will serve the
 		// public-looking names as well as any other names
 		// that don't match any other policy
-		app.tlsApp.AddAutomationPolicy(basePolicy)
+		err := app.tlsApp.AddAutomationPolicy(basePolicy)
+		if err != nil {
+			return err
+		}
 	} else {
 		// a base policy already existed; we might have
 		// changed it, so re-provision it

modules/caddyhttp/caddyauth/basicauth.go

@@ -240,6 +240,7 @@ func (c *Cache) makeRoom() {
 		// map with less code, this is a heavily skewed eviction
 		// strategy; generating random numbers is cheap and
 		// ensures a much better distribution.
+		//nolint:gosec
 		rnd := weakrand.Intn(len(c.cache))
 		i := 0
 		for key := range c.cache {

modules/caddyhttp/caddyhttp.go

@@ -18,18 +18,14 @@ import (
 	"bytes"
 	"encoding/json"
 	"io"
-	weakrand "math/rand"
 	"net"
 	"net/http"
 	"strconv"
-	"time"
 
 	"github.com/caddyserver/caddy/v2"
 )
 
 func init() {
-	weakrand.Seed(time.Now().UnixNano())
-
 	caddy.RegisterModule(tlsPlaceholderWrapper{})
 }
 

modules/caddyhttp/encode/encode.go

@@ -262,7 +262,7 @@ func acceptedEncodings(r *http.Request) []string {
 		return []string{}
 	}
 
-	var prefs []encodingPreference
+	prefs := []encodingPreference{}
 
 	for _, accepted := range strings.Split(acceptEncHeader, ",") {
 		parts := strings.Split(accepted, ";")

modules/caddyhttp/errors.go

@@ -16,14 +16,19 @@ package caddyhttp
 
 import (
 	"fmt"
-	mathrand "math/rand"
+	weakrand "math/rand"
 	"path"
 	"runtime"
 	"strings"
+	"time"
 
 	"github.com/caddyserver/caddy/v2"
 )
 
+func init() {
+	weakrand.Seed(time.Now().UnixNano())
+}
+
 // Error is a convenient way for a Handler to populate the
 // essential fields of a HandlerError. If err is itself a
 // HandlerError, then any essential fields that are not
@@ -92,7 +97,8 @@ func randString(n int, sameCase bool) string {
 	}
 	b := make([]byte, n)
 	for i := range b {
-		b[i] = dict[mathrand.Int63()%int64(len(dict))]
+		//nolint:gosec
+		b[i] = dict[weakrand.Int63()%int64(len(dict))]
 	}
 	return string(b)
 }

modules/caddyhttp/fileserver/browse.go

@@ -82,7 +82,7 @@ func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *ht
 		w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	}
 
-	buf.WriteTo(w)
+	_, _ = buf.WriteTo(w)
 
 	return nil
 }

modules/caddyhttp/fileserver/browselisting.go

@@ -30,10 +30,8 @@ import (
 func (fsrv *FileServer) directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, repl *caddy.Replacer) browseListing {
 	filesToHide := fsrv.transformHidePaths(repl)
 
-	var (
-		fileInfos           []fileInfo
-		dirCount, fileCount int
-	)
+	var dirCount, fileCount int
+	fileInfos := []fileInfo{}
 
 	for _, f := range files {
 		name := f.Name()
@@ -109,10 +107,8 @@ type browseListing struct {
 // Breadcrumbs returns l.Path where every element maps
 // the link to the text to display.
 func (l browseListing) Breadcrumbs() []crumb {
-	var result []crumb
-
 	if len(l.Path) == 0 {
-		return result
+		return []crumb{}
 	}
 
 	// skip trailing slash
@@ -122,13 +118,13 @@ func (l browseListing) Breadcrumbs() []crumb {
 	}
 
 	parts := strings.Split(lpath, "/")
-	for i := range parts {
-		txt := parts[i]
-		if i == 0 && parts[i] == "" {
-			txt = "/"
+	result := make([]crumb, len(parts))
+	for i, p := range parts {
+		if i == 0 && p == "" {
+			p = "/"
 		}
 		lnk := strings.Repeat("../", len(parts)-i-1)
-		result = append(result, crumb{Link: lnk, Text: txt})
+		result[i] = crumb{Link: lnk, Text: p}
 	}
 
 	return result

modules/caddyhttp/fileserver/browselisting_test.go

@@ -0,0 +1,40 @@
+package fileserver
+
+import (
+	"testing"
+)
+
+func TestBreadcrumbs(t *testing.T) {
+	testdata := []struct {
+		path     string
+		expected []crumb
+	}{
+		{"", []crumb{}},
+		{"/", []crumb{{Text: "/"}}},
+		{"foo/bar/baz", []crumb{
+			{Link: "../../", Text: "foo"},
+			{Link: "../", Text: "bar"},
+			{Link: "", Text: "baz"},
+		}},
+		{"/qux/quux/corge/", []crumb{
+			{Link: "../../../", Text: "/"},
+			{Link: "../../", Text: "qux"},
+			{Link: "../", Text: "quux"},
+			{Link: "", Text: "corge"},
+		}},
+	}
+
+	for _, d := range testdata {
+		l := browseListing{Path: d.path}
+		actual := l.Breadcrumbs()
+		if len(actual) != len(d.expected) {
+			t.Errorf("wrong size output, got %d elements but expected %d", len(actual), len(d.expected))
+			continue
+		}
+		for i, c := range actual {
+			if c != d.expected[i] {
+				t.Errorf("got %#v but expected %#v at index %d", c, d.expected[i], i)
+			}
+		}
+	}
+}

modules/caddyhttp/fileserver/staticfiles.go

@@ -249,7 +249,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
 		}
 		w.WriteHeader(statusCode)
 		if r.Method != http.MethodHead {
-			io.Copy(w, file)
+			_, _ = io.Copy(w, file)
 		}
 		return nil
 	}
@@ -278,6 +278,7 @@ func (fsrv *FileServer) openFile(filename string, w http.ResponseWriter) (*os.Fi
 		}
 		// maybe the server is under load and ran out of file descriptors?
 		// have client wait arbitrary seconds to help prevent a stampede
+		//nolint:gosec
 		backoff := weakrand.Intn(maxBackoff-minBackoff) + minBackoff
 		w.Header().Set("Retry-After", strconv.Itoa(backoff))
 		return nil, caddyhttp.Error(http.StatusServiceUnavailable, err)

modules/caddyhttp/marshalers.go

@@ -75,7 +75,8 @@ func (t LoggableTLSConnState) MarshalLogObject(enc zapcore.ObjectEncoder) error
 	enc.AddUint16("version", t.Version)
 	enc.AddUint16("cipher_suite", t.CipherSuite)
 	enc.AddString("proto", t.NegotiatedProtocol)
-	enc.AddBool("proto_mutual", t.NegotiatedProtocolIsMutual)
+	// NegotiatedProtocolIsMutual is deprecated - it's always true
+	enc.AddBool("proto_mutual", true)
 	enc.AddString("server_name", t.ServerName)
 	if len(t.PeerCertificates) > 0 {
 		enc.AddString("client_common_name", t.PeerCertificates[0].Subject.CommonName)

modules/caddyhttp/replacer.go

@@ -362,7 +362,8 @@ func getReqTLSReplacement(req *http.Request, key string) (interface{}, bool) {
 	case "proto":
 		return req.TLS.NegotiatedProtocol, true
 	case "proto_mutual":
-		return req.TLS.NegotiatedProtocolIsMutual, true
+		// req.TLS.NegotiatedProtocolIsMutual is deprecated - it's always true.
+		return true, true
 	case "server_name":
 		return req.TLS.ServerName, true
 	}

modules/caddyhttp/reverseproxy/fastcgi/client.go

@@ -242,13 +242,6 @@ func (c *FCGIClient) writeBeginRequest(role uint16, flags uint8) error {
 	return c.writeRecord(BeginRequest, b[:])
 }
 
-func (c *FCGIClient) writeEndRequest(appStatus int, protocolStatus uint8) error {
-	b := make([]byte, 8)
-	binary.BigEndian.PutUint32(b, uint32(appStatus))
-	b[4] = protocolStatus
-	return c.writeRecord(EndRequest, b)
-}
-
 func (c *FCGIClient) writePairs(recType uint8, pairs map[string]string) error {
 	w := newWriter(c, recType)
 	b := make([]byte, 8)

modules/caddyhttp/reverseproxy/healthchecks.go

@@ -263,7 +263,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, host H
 	}
 	defer func() {
 		// drain any remaining body so connection could be re-used
-		io.Copy(ioutil.Discard, body)
+		_, _ = io.Copy(ioutil.Discard, body)
 		resp.Body.Close()
 	}()
 

modules/caddyhttp/reverseproxy/httptransport.go

@@ -173,6 +173,7 @@ func (h *HTTPTransport) NewTransport(ctx caddy.Context) (*http.Transport, error)
 		dialer.Resolver = &net.Resolver{
 			PreferGo: true,
 			Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {
+				//nolint:gosec
 				addr := h.Resolver.netAddrs[weakrand.Intn(len(h.Resolver.netAddrs))]
 				return d.DialContext(ctx, addr.Network, addr.JoinHostPort(0))
 			},

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -314,7 +314,7 @@ func (h *Handler) Cleanup() error {
 
 	// remove hosts from our config from the pool
 	for _, upstream := range h.Upstreams {
-		hosts.Delete(upstream.String())
+		_, _ = hosts.Delete(upstream.String())
 	}
 
 	return nil
@@ -339,7 +339,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht
 		buf := bufPool.Get().(*bytes.Buffer)
 		buf.Reset()
 		defer bufPool.Put(buf)
-		io.Copy(buf, r.Body)
+		_, _ = io.Copy(buf, r.Body)
 		r.Body.Close()
 		r.Body = ioutil.NopCloser(buf)
 	}
@@ -518,7 +518,8 @@ func (h Handler) prepareRequest(req *http.Request) error {
 // (This method is mostly the beginning of what was borrowed from the net/http/httputil package in the
 // Go standard library which was used as the foundation.)
 func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, di DialInfo, next caddyhttp.Handler) error {
-	di.Upstream.Host.CountRequest(1)
+	_ = di.Upstream.Host.CountRequest(1)
+	//nolint:errcheck
 	defer di.Upstream.Host.CountRequest(-1)
 
 	// point the request to this upstream
@@ -742,16 +743,6 @@ func copyHeader(dst, src http.Header) {
 	}
 }
 
-func cloneHeader(h http.Header) http.Header {
-	h2 := make(http.Header, len(h))
-	for k, vv := range h {
-		vv2 := make([]string, len(vv))
-		copy(vv2, vv)
-		h2[k] = vv2
-	}
-	return h2
-}
-
 func upgradeType(h http.Header) string {
 	if !httpguts.HeaderValuesContainsToken(h["Connection"], "Upgrade") {
 		return ""
@@ -759,18 +750,6 @@ func upgradeType(h http.Header) string {
 	return strings.ToLower(h.Get("Upgrade"))
 }
 
-func singleJoiningSlash(a, b string) string {
-	aslash := strings.HasSuffix(a, "/")
-	bslash := strings.HasPrefix(b, "/")
-	switch {
-	case aslash && bslash:
-		return a + b[1:]
-	case !aslash && !bslash:
-		return a + "/" + b
-	}
-	return a + b
-}
-
 // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h.
 // See RFC 7230, section 6.1
 func removeConnectionHeaders(h http.Header) {

modules/caddyhttp/reverseproxy/selectionpolicies.go

@@ -536,7 +536,7 @@ func hostByHashing(pool []*Upstream, s string) *Upstream {
 // hash calculates a fast hash based on s.
 func hash(s string) uint32 {
 	h := fnv.New32a()
-	h.Write([]byte(s))
+	_, _ = h.Write([]byte(s))
 	return h.Sum32()
 }
 

modules/caddyhttp/reverseproxy/streaming.go

@@ -138,9 +138,9 @@ func (h Handler) copyResponse(dst io.Writer, src io.Reader, flushInterval time.D
 		}
 	}
 
-	buf := streamingBufPool.Get().([]byte)
+	buf := streamingBufPool.Get().(*[]byte)
 	defer streamingBufPool.Put(buf)
-	_, err := h.copyBuffer(dst, src, buf)
+	_, err := h.copyBuffer(dst, src, *buf)
 	return err
 }
 
@@ -255,7 +255,12 @@ func (c switchProtocolCopier) copyToBackend(errc chan<- error) {
 
 var streamingBufPool = sync.Pool{
 	New: func() interface{} {
-		return make([]byte, defaultBufferSize)
+		// The Pool's New function should generally only return pointer
+		// types, since a pointer can be put into the return interface
+		// value without an allocation
+		// - (from the package docs)
+		b := make([]byte, defaultBufferSize)
+		return &b
 	},
 }
 

modules/caddyhttp/reverseproxy/streaming_test.go

@@ -0,0 +1,30 @@
+package reverseproxy
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+)
+
+func TestHandlerCopyResponse(t *testing.T) {
+	h := Handler{}
+	testdata := []string{
+		"",
+		strings.Repeat("a", defaultBufferSize),
+		strings.Repeat("123456789 123456789 123456789 12", 3000),
+	}
+	dst := bytes.NewBuffer(nil)
+
+	for _, d := range testdata {
+		src := bytes.NewBuffer([]byte(d))
+		dst.Reset()
+		err := h.copyResponse(dst, src, 0)
+		if err != nil {
+			t.Errorf("failed with error: %v", err)
+		}
+		out := dst.String()
+		if out != d {
+			t.Errorf("bad read: got %q", out)
+		}
+	}
+}

modules/caddyhttp/templates/tplcontext.go

@@ -270,7 +270,10 @@ func (templateContext) funcMarkdown(input interface{}) (string, error) {
 	buf.Reset()
 	defer bufPool.Put(buf)
 
-	md.Convert([]byte(inputStr), buf)
+	err := md.Convert([]byte(inputStr), buf)
+	if err != nil {
+		return "", err
+	}
 
 	return buf.String(), nil
 }

modules/caddypki/acmeserver/acmeserver.go

@@ -132,11 +132,11 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 		return err
 	}
 
-	acmeAuth, err := acme.NewAuthority(
-		auth.GetDatabase().(nosql.DB),     // stores all the server state
-		ash.Host,                          // used for directory links; TODO: not needed
-		strings.Trim(ash.PathPrefix, "/"), // used for directory links
-		auth)                              // configures the signing authority
+	acmeAuth, err := acme.New(auth, acme.AuthorityOptions{
+		DB:     auth.GetDatabase().(nosql.DB),     // stores all the server state
+		DNS:    ash.Host,                          // used for directory links; TODO: not needed
+		Prefix: strings.Trim(ash.PathPrefix, "/"), // used for directory links
+	})
 	if err != nil {
 		return err
 	}

modules/caddypki/ca.go

@@ -309,14 +309,6 @@ func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey interface
 func (ca CA) genIntermediate(rootCert *x509.Certificate, rootKey interface{}) (interCert *x509.Certificate, interKey interface{}, err error) {
 	repl := ca.newReplacer()
 
-	rootKeyPEM, err := ca.storage.Load(ca.storageKeyRootKey())
-	if err != nil {
-		return nil, nil, fmt.Errorf("loading root key to sign new intermediate: %v", err)
-	}
-	rootKey, err = pemDecodePrivateKey(rootKeyPEM)
-	if err != nil {
-		return nil, nil, fmt.Errorf("decoding root key: %v", err)
-	}
 	interCert, interKey, err = generateIntermediate(repl.ReplaceAll(ca.IntermediateCommonName, ""), rootCert, rootKey)
 	if err != nil {
 		return nil, nil, fmt.Errorf("generating CA intermediate: %v", err)

modules/caddytls/connpolicy.go

@@ -80,6 +80,7 @@ func (cp ConnectionPolicies) TLSConfig(ctx caddy.Context) *tls.Config {
 	}
 
 	return &tls.Config{
+		MinVersion: tls.VersionTLS12,
 		GetConfigForClient: func(hello *tls.ClientHelloInfo) (*tls.Config, error) {
 			// filter policies by SNI first, if possible, to speed things up
 			// when there may be lots of policies

modules/caddytls/distributedstek/distributedstek.go

@@ -145,7 +145,12 @@ func (s *Provider) storeSTEK(dstek distributedSTEK) error {
 // current STEK is outdated (NextRotation time is in the past),
 // then it is rotated and persisted. The resulting STEK is returned.
 func (s *Provider) getSTEK() (distributedSTEK, error) {
-	s.storage.Lock(s.ctx, stekLockName)
+	err := s.storage.Lock(s.ctx, stekLockName)
+	if err != nil {
+		return distributedSTEK{}, fmt.Errorf("failed to acquire storage lock: %v", err)
+	}
+
+	//nolint:errcheck
 	defer s.storage.Unlock(stekLockName)
 
 	// load the current STEKs from storage

modules/caddytls/folderloader.go

@@ -97,26 +97,38 @@ func x509CertFromCertAndKeyPEMFile(fpath string) (tls.Certificate, error) {
 
 		if derBlock.Type == "CERTIFICATE" {
 			// Re-encode certificate as PEM, appending to certificate chain
-			pem.Encode(certBuilder, derBlock)
+			err = pem.Encode(certBuilder, derBlock)
+			if err != nil {
+				return tls.Certificate{}, err
+			}
 		} else if derBlock.Type == "EC PARAMETERS" {
 			// EC keys generated from openssl can be composed of two blocks:
 			// parameters and key (parameter block should come first)
 			if !foundKey {
 				// Encode parameters
-				pem.Encode(keyBuilder, derBlock)
+				err = pem.Encode(keyBuilder, derBlock)
+				if err != nil {
+					return tls.Certificate{}, err
+				}
 
 				// Key must immediately follow
 				derBlock, bundle = pem.Decode(bundle)
 				if derBlock == nil || derBlock.Type != "EC PRIVATE KEY" {
 					return tls.Certificate{}, fmt.Errorf("%s: expected elliptic private key to immediately follow EC parameters", fpath)
 				}
-				pem.Encode(keyBuilder, derBlock)
+				err = pem.Encode(keyBuilder, derBlock)
+				if err != nil {
+					return tls.Certificate{}, err
+				}
 				foundKey = true
 			}
 		} else if derBlock.Type == "PRIVATE KEY" || strings.HasSuffix(derBlock.Type, " PRIVATE KEY") {
 			// RSA key
 			if !foundKey {
-				pem.Encode(keyBuilder, derBlock)
+				err = pem.Encode(keyBuilder, derBlock)
+				if err != nil {
+					return tls.Certificate{}, err
+				}
 				foundKey = true
 			}
 		} else {

modules/caddytls/internalissuer.go

@@ -27,6 +27,7 @@ import (
 	"github.com/caddyserver/caddy/v2/modules/caddypki"
 	"github.com/caddyserver/certmagic"
 	"github.com/smallstep/certificates/authority/provisioner"
+	"go.uber.org/zap"
 )
 
 func init() {
@@ -52,6 +53,7 @@ type InternalIssuer struct {
 	SignWithRoot bool `json:"sign_with_root,omitempty"`
 
 	ca     *caddypki.CA
+	logger *zap.Logger
 }
 
 // CaddyModule returns the Caddy module information.
@@ -64,6 +66,8 @@ func (InternalIssuer) CaddyModule() caddy.ModuleInfo {
 
 // Provision sets up the issuer.
 func (iss *InternalIssuer) Provision(ctx caddy.Context) error {
+	iss.logger = ctx.Logger(iss)
+
 	// get a reference to the configured CA
 	appModule, err := ctx.App("pki")
 	if err != nil {
@@ -115,11 +119,15 @@ func (iss InternalIssuer) Issue(ctx context.Context, csr *x509.CertificateReques
 	// ensure issued certificate does not expire later than its issuer
 	lifetime := time.Duration(iss.Lifetime)
 	if time.Now().Add(lifetime).After(issuerCert.NotAfter) {
-		// TODO: log this
-		lifetime = issuerCert.NotAfter.Sub(time.Now())
+		lifetime = time.Until(issuerCert.NotAfter)
+		iss.logger.Warn("cert lifetime would exceed issuer NotAfter, clamping lifetime",
+			zap.Duration("orig_lifetime", time.Duration(iss.Lifetime)),
+			zap.Duration("lifetime", lifetime),
+			zap.Time("not_after", issuerCert.NotAfter),
+		)
 	}
 
-	certChain, err := auth.Sign(csr, provisioner.SignOptions{}, customCertLifetime(iss.Lifetime))
+	certChain, err := auth.Sign(csr, provisioner.SignOptions{}, customCertLifetime(caddy.Duration(lifetime)))
 	if err != nil {
 		return nil, err
 	}

modules/caddytls/tls.go

@@ -498,8 +498,6 @@ var (
 	storageCleanMu sync.Mutex
 )
 
-const automateKey = "automate"
-
 // Interface guards
 var (
 	_ caddy.App          = (*TLS)(nil)

modules/caddytls/values.go

@@ -122,6 +122,7 @@ var SupportedProtocols = map[string]uint16{
 // unsupportedProtocols is a map of unsupported protocols.
 // Used for logging only, not enforcement.
 var unsupportedProtocols = map[string]uint16{
+	//nolint:staticcheck
 	"ssl3.0": tls.VersionSSL30,
 	"tls1.0": tls.VersionTLS10,
 	"tls1.1": tls.VersionTLS11,