chore: upgrade .golangci.yml and workflow to v2 (#6924)

* chore: upgrade .golangci.yml and workflow to v2

run `golangci-lint fmt`

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* run `golangci-lint run --fix`

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* more lint fixes

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* bring back comments to .golangci.yml

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* appease the linter some more

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* oops

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* use embedded structs

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* use embedded structs where they were used before

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* disable rule  `-QF1006`

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* missed a spot

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

---------

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
pull/7044/head
Mohammed Al Sahaf 2025-06-03 02:24:32 +03:00 committed by GitHub
parent 5b2eb66418
commit e039a5bb5c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
33 changed files with 148 additions and 213 deletions

View File

@ -51,7 +51,7 @@ jobs:
check-latest: true
- name: golangci-lint
uses: golangci/golangci-lint-action@v6
uses: golangci/golangci-lint-action@v7
with:
version: latest

View File

@ -1,27 +1,15 @@
linters-settings:
errcheck:
exclude-functions:
- fmt.*
- (go.uber.org/zap/zapcore.ObjectEncoder).AddObject
- (go.uber.org/zap/zapcore.ObjectEncoder).AddArray
gci:
sections:
- standard # Standard section: captures all standard packages.
- default # Default section: contains all imports that could not be matched to another section type.
- prefix(github.com/caddyserver/caddy/v2/cmd) # ensure that this is always at the top and always has a line break.
- prefix(github.com/caddyserver/caddy) # Custom section: groups all imports with the specified Prefix.
# Skip generated files.
# Default: true
skip-generated: true
# Enable custom order of sections.
# If `true`, make the section order the same as the order of `sections`.
# Default: false
custom-order: true
exhaustive:
ignore-enum-types: reflect.Kind|svc.Cmd
version: "2"
run:
issues-exit-code: 1
tests: false
output:
formats:
text:
path: stdout
print-linter-name: true
print-issued-lines: true
linters:
disable-all: true
default: none
enable:
- asasalint
- asciicheck
@ -35,148 +23,96 @@ linters:
- errcheck
- errname
- exhaustive
- gci
- gofmt
- goimports
- gofumpt
- gosec
- gosimple
- govet
- ineffassign
- importas
- ineffassign
- misspell
- prealloc
- promlinter
- sloglint
- sqlclosecheck
- staticcheck
- tenv
- testableexamples
- testifylint
- tparallel
- typecheck
- unconvert
- unused
- wastedassign
- whitespace
- zerologlint
# these are implicitly disabled:
# - containedctx
# - contextcheck
# - cyclop
# - depguard
# - errchkjson
# - errorlint
# - exhaustruct
# - execinquery
# - exhaustruct
# - forbidigo
# - forcetypeassert
# - funlen
# - ginkgolinter
# - gocheckcompilerdirectives
# - gochecknoglobals
# - gochecknoinits
# - gochecksumtype
# - gocognit
# - goconst
# - gocritic
# - gocyclo
# - godot
# - godox
# - goerr113
# - goheader
# - gomnd
# - gomoddirectives
# - gomodguard
# - goprintffuncname
# - gosmopolitan
# - grouper
# - inamedparam
# - interfacebloat
# - ireturn
# - lll
# - loggercheck
# - maintidx
# - makezero
# - mirror
# - musttag
# - nakedret
# - nestif
# - nilerr
# - nilnil
# - nlreturn
# - noctx
# - nolintlint
# - nonamedreturns
# - nosprintfhostport
# - paralleltest
# - perfsprint
# - predeclared
# - protogetter
# - reassign
# - revive
# - rowserrcheck
# - stylecheck
# - tagalign
# - tagliatelle
# - testpackage
# - thelper
# - unparam
# - usestdlibvars
# - varnamelen
# - wrapcheck
# - wsl
run:
# default concurrency is a available CPU number.
# concurrency: 4 # explicitly omit this value to fully utilize available resources.
timeout: 5m
issues-exit-code: 1
tests: false
# output configuration options
output:
formats:
- format: 'colored-line-number'
print-issued-lines: true
print-linter-name: true
issues:
exclude-rules:
- text: 'G115' # TODO: Either we should fix the issues or nuke the linter if it's bad
linters:
- gosec
# we aren't calling unknown URL
- text: 'G107' # G107: Url provided to HTTP request as taint input
linters:
- gosec
# as a web server that's expected to handle any template, this is totally in the hands of the user.
- text: 'G203' # G203: Use of unescaped data in HTML templates
linters:
- gosec
# we're shelling out to known commands, not relying on user-defined input.
- text: 'G204' # G204: Audit use of command execution
linters:
- gosec
# the choice of weakrand is deliberate, hence the named import "weakrand"
- path: modules/caddyhttp/reverseproxy/selectionpolicies.go
text: 'G404' # G404: Insecure random number source (rand)
linters:
- gosec
- path: modules/caddyhttp/reverseproxy/streaming.go
text: 'G404' # G404: Insecure random number source (rand)
linters:
- gosec
- path: modules/logging/filters.go
linters:
- dupl
- path: modules/caddyhttp/matchers.go
linters:
- dupl
- path: modules/caddyhttp/vars.go
linters:
- dupl
- path: _test\.go
linters:
- errcheck
settings:
staticcheck:
checks: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-QF1006", "-QF1008"] # default, and exclude 1 more undesired check
errcheck:
exclude-functions:
- fmt.*
- (go.uber.org/zap/zapcore.ObjectEncoder).AddObject
- (go.uber.org/zap/zapcore.ObjectEncoder).AddArray
exhaustive:
ignore-enum-types: reflect.Kind|svc.Cmd
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
rules:
- linters:
- gosec
text: G115 # TODO: Either we should fix the issues or nuke the linter if it's bad
- linters:
- gosec
text: G107 # we aren't calling unknown URL
- linters:
- gosec
text: G203 # as a web server that's expected to handle any template, this is totally in the hands of the user.
- linters:
- gosec
text: G204 # we're shelling out to known commands, not relying on user-defined input.
- linters:
- gosec
# the choice of weakrand is deliberate, hence the named import "weakrand"
path: modules/caddyhttp/reverseproxy/selectionpolicies.go
text: G404
- linters:
- gosec
path: modules/caddyhttp/reverseproxy/streaming.go
text: G404
- linters:
- dupl
path: modules/logging/filters.go
- linters:
- dupl
path: modules/caddyhttp/matchers.go
- linters:
- dupl
path: modules/caddyhttp/vars.go
- linters:
- errcheck
path: _test\.go
paths:
- third_party$
- builtin$
- examples$
formatters:
enable:
- gci
- gofmt
- gofumpt
- goimports
settings:
gci:
sections:
- standard # Standard section: captures all standard packages.
- default # Default section: contains all imports that could not be matched to another section type.
- prefix(github.com/caddyserver/caddy/v2/cmd) # ensure that this is always at the top and always has a line break.
- prefix(github.com/caddyserver/caddy) # Custom section: groups all imports with the specified Prefix.
custom-order: true
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$

View File

@ -68,7 +68,7 @@ func (a Adapter) Adapt(body []byte, options map[string]any) ([]byte, []caddyconf
// TODO: also perform this check on imported files
func FormattingDifference(filename string, body []byte) (caddyconfig.Warning, bool) {
// replace windows-style newlines to normalize comparison
normalizedBody := bytes.Replace(body, []byte("\r\n"), []byte("\n"), -1)
normalizedBody := bytes.ReplaceAll(body, []byte("\r\n"), []byte("\n"))
formatted := Format(normalizedBody)
if bytes.Equal(formatted, normalizedBody) {

View File

@ -94,7 +94,7 @@ func Format(input []byte) []byte {
}
// detect whether we have the start of a heredoc
if !quoted && !(heredoc != heredocClosed || heredocEscaped) &&
if !quoted && (heredoc == heredocClosed && !heredocEscaped) &&
space && last == '<' && ch == '<' {
write(ch)
heredoc = heredocOpening

View File

@ -137,7 +137,7 @@ func (l *lexer) next() (bool, error) {
}
// detect whether we have the start of a heredoc
if !(quoted || btQuoted) && !(inHeredoc || heredocEscaped) &&
if (!quoted && !btQuoted) && (!inHeredoc && !heredocEscaped) &&
len(val) > 1 && string(val[:2]) == "<<" {
// a space means it's just a regular token and not a heredoc
if ch == ' ' {

View File

@ -174,10 +174,12 @@ func RegisterDirectiveOrder(dir string, position Positional, standardDir string)
if d != standardDir {
continue
}
if position == Before {
switch position {
case Before:
newOrder = append(newOrder[:i], append([]string{dir}, newOrder[i:]...)...)
} else if position == After {
case After:
newOrder = append(newOrder[:i+1], append([]string{dir}, newOrder[i+1:]...)...)
case First, Last:
}
break
}

View File

@ -281,7 +281,7 @@ func validateTestPrerequisites(tc *Tester) error {
tc.t.Cleanup(func() {
os.Remove(f.Name())
})
if _, err := f.WriteString(fmt.Sprintf(initConfig, tc.config.AdminPort)); err != nil {
if _, err := fmt.Fprintf(f, initConfig, tc.config.AdminPort); err != nil {
return err
}

View File

@ -12,13 +12,14 @@ import (
"strings"
"testing"
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddytest"
"github.com/mholt/acmez/v3"
"github.com/mholt/acmez/v3/acme"
smallstepacme "github.com/smallstep/certificates/acme"
"go.uber.org/zap"
"go.uber.org/zap/exp/zapslog"
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddytest"
)
const acmeChallengePort = 9081

View File

@ -9,11 +9,12 @@ import (
"strings"
"testing"
"github.com/caddyserver/caddy/v2/caddytest"
"github.com/mholt/acmez/v3"
"github.com/mholt/acmez/v3/acme"
"go.uber.org/zap"
"go.uber.org/zap/exp/zapslog"
"github.com/caddyserver/caddy/v2/caddytest"
)
func TestACMEServerDirectory(t *testing.T) {

View File

@ -10,7 +10,6 @@ import (
"testing"
"github.com/caddyserver/caddy/v2/caddytest"
_ "github.com/caddyserver/caddy/v2/internal/testmocks"
)

View File

@ -615,7 +615,6 @@ func TestReplaceWithReplacementPlaceholder(t *testing.T) {
respond "{query}"`, "caddyfile")
tester.AssertGetResponse("http://localhost:9080/endpoint?placeholder=baz&foo=bar", 200, "foo=baz&placeholder=baz")
}
func TestReplaceWithKeyPlaceholder(t *testing.T) {

View File

@ -3,10 +3,11 @@ package integration
import (
"context"
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
"github.com/caddyserver/certmagic"
"github.com/libdns/libdns"
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
)
func init() {
@ -55,7 +56,9 @@ func (MockDNSProvider) SetRecords(ctx context.Context, zone string, recs []libdn
}
// Interface guard
var _ caddyfile.Unmarshaler = (*MockDNSProvider)(nil)
var _ certmagic.DNSProvider = (*MockDNSProvider)(nil)
var _ caddy.Provisioner = (*MockDNSProvider)(nil)
var _ caddy.Module = (*MockDNSProvider)(nil)
var (
_ caddyfile.Unmarshaler = (*MockDNSProvider)(nil)
_ certmagic.DNSProvider = (*MockDNSProvider)(nil)
_ caddy.Provisioner = (*MockDNSProvider)(nil)
_ caddy.Module = (*MockDNSProvider)(nil)
)

View File

@ -13,9 +13,10 @@ import (
"testing"
"time"
"github.com/caddyserver/caddy/v2/caddytest"
"golang.org/x/net/http2"
"golang.org/x/net/http2/h2c"
"github.com/caddyserver/caddy/v2/caddytest"
)
// (see https://github.com/caddyserver/caddy/issues/3556 for use case)

View File

@ -418,7 +418,7 @@ func parseEnvFile(envInput io.Reader) (map[string]string, error) {
// quoted value: support newlines
if strings.HasPrefix(val, `"`) || strings.HasPrefix(val, "'") {
quote := string(val[0])
for !(strings.HasSuffix(line, quote) && !strings.HasSuffix(line, `\`+quote)) {
for !strings.HasSuffix(line, quote) || strings.HasSuffix(line, `\`+quote) {
val = strings.ReplaceAll(val, `\`+quote, quote)
if !scanner.Scan() {
break

View File

@ -235,7 +235,6 @@ func Test_isCaddyfile(t *testing.T) {
wantErr: false,
},
{
name: "json is not caddyfile but not error",
args: args{
configFile: "./Caddyfile.json",
@ -245,7 +244,6 @@ func Test_isCaddyfile(t *testing.T) {
wantErr: false,
},
{
name: "prefix of Caddyfile and ./ with any extension is Caddyfile",
args: args{
configFile: "./Caddyfile.prd",
@ -255,7 +253,6 @@ func Test_isCaddyfile(t *testing.T) {
wantErr: false,
},
{
name: "prefix of Caddyfile without ./ with any extension is Caddyfile",
args: args{
configFile: "Caddyfile.prd",

View File

@ -84,7 +84,7 @@ func cmdAddPackage(fl Flags) (int, error) {
return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid module name: %v", err)
}
// only allow a version to be specified if it's different from the existing version
if _, ok := pluginPkgs[module]; ok && !(version != "" && pluginPkgs[module].Version != version) {
if _, ok := pluginPkgs[module]; ok && (version == "" || pluginPkgs[module].Version == version) {
return caddy.ExitCodeFailedStartup, fmt.Errorf("package is already added")
}
pluginPkgs[module] = pluginPackage{Version: version, Path: module}

View File

@ -30,7 +30,7 @@ func TestSplitNetworkAddress(t *testing.T) {
expectErr bool
}{
{
input: "",
input: "",
expectHost: "",
},
{
@ -41,7 +41,7 @@ func TestSplitNetworkAddress(t *testing.T) {
input: ":", // empty host & empty port
},
{
input: "::",
input: "::",
expectHost: "::",
},
{
@ -184,9 +184,8 @@ func TestParseNetworkAddress(t *testing.T) {
expectErr bool
}{
{
input: "",
expectAddr: NetworkAddress{
},
input: "",
expectAddr: NetworkAddress{},
},
{
input: ":",
@ -311,9 +310,8 @@ func TestParseNetworkAddressWithDefaults(t *testing.T) {
expectErr bool
}{
{
input: "",
expectAddr: NetworkAddress{
},
input: "",
expectAddr: NetworkAddress{},
},
{
input: ":",

View File

@ -343,7 +343,7 @@ uniqueDomainsLoop:
// match on known domain names, unless it's our special case of a
// catch-all which is an empty string (common among catch-all sites
// that enable on-demand TLS for yet-unknown domain names)
if !(len(domains) == 1 && domains[0] == "") {
if len(domains) != 1 || domains[0] != "" {
matcherSet = append(matcherSet, MatchHost(domains))
}

View File

@ -252,7 +252,7 @@ func celFileMatcherMacroExpander() parser.MacroExpander {
}
for _, arg := range args {
if !(isCELStringLiteral(arg) || isCELCaddyPlaceholderCall(arg)) {
if !isCELStringLiteral(arg) && !isCELCaddyPlaceholderCall(arg) {
return nil, &common.Error{
Location: eh.OffsetLocation(arg.ID()),
Message: "matcher only supports repeated string literal arguments",
@ -616,15 +616,16 @@ func isCELTryFilesLiteral(e ast.Expr) bool {
return false
}
mapKeyStr := mapKey.AsLiteral().ConvertToType(types.StringType).Value()
if mapKeyStr == "try_files" || mapKeyStr == "split_path" {
switch mapKeyStr {
case "try_files", "split_path":
if !isCELStringListLiteral(mapVal) {
return false
}
} else if mapKeyStr == "try_policy" || mapKeyStr == "root" {
case "try_policy", "root":
if !(isCELStringExpr(mapVal)) {
return false
}
} else {
default:
return false
}
}

View File

@ -552,7 +552,6 @@ func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) b
if iPattern >= len(matchPath) || iPath >= len(escapedPath) {
break
}
// get the next character from the request path
pathCh := string(escapedPath[iPath])

View File

@ -9,8 +9,9 @@ import (
"sync"
"testing"
"github.com/caddyserver/caddy/v2"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/caddyserver/caddy/v2"
)
func TestServerNameFromContext(t *testing.T) {

View File

@ -363,13 +363,13 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo
}
}
switch {
case key == "http.shutting_down":
switch key {
case "http.shutting_down":
server := req.Context().Value(ServerCtxKey).(*Server)
server.shutdownAtMu.RLock()
defer server.shutdownAtMu.RUnlock()
return !server.shutdownAt.IsZero(), true
case key == "http.time_until_shutdown":
case "http.time_until_shutdown":
server := req.Context().Value(ServerCtxKey).(*Server)
server.shutdownAtMu.RLock()
defer server.shutdownAtMu.RUnlock()

View File

@ -665,9 +665,10 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
if d.NextArg() {
return d.ArgErr()
}
if subdir == "request_buffers" {
switch subdir {
case "request_buffers":
h.RequestBuffers = size
} else if subdir == "response_buffers" {
case "response_buffers":
h.ResponseBuffers = size
}

View File

@ -122,9 +122,10 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) {
}
}
if fromAddr.Port == "" {
if fromAddr.Scheme == "http" {
switch fromAddr.Scheme {
case "http":
fromAddr.Port = httpPort
} else if fromAddr.Scheme == "https" {
case "https":
fromAddr.Port = httpsPort
}
}

View File

@ -484,7 +484,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ
markHealthy := func() {
// increment passes and then check if it has reached the threshold to be healthy
err := upstream.Host.countHealthPass(1)
err := upstream.countHealthPass(1)
if err != nil {
if c := h.HealthChecks.Active.logger.Check(zapcore.ErrorLevel, "could not count active health pass"); c != nil {
c.Write(

View File

@ -1150,7 +1150,7 @@ func (lb LoadBalancing) tryAgain(ctx caddy.Context, start time.Time, retries int
// we have to assume the upstream received the request, and
// retries need to be carefully decided, because some requests
// are not idempotent
if !isDialError && !(isHandlerError && errors.Is(herr, errNoUpstream)) {
if !isDialError && (!isHandlerError || !errors.Is(herr, errNoUpstream)) {
if lb.RetryMatch == nil && req.Method != "GET" {
// by default, don't retry requests if they aren't GET
return false

View File

@ -808,7 +808,7 @@ func leastRequests(upstreams []*Upstream) *Upstream {
return nil
}
var best []*Upstream
var bestReqs int = -1
bestReqs := -1
for _, upstream := range upstreams {
if upstream == nil {
continue

View File

@ -52,5 +52,4 @@ func TestResolveIpVersion(t *testing.T) {
t.Errorf("resolveIpVersion(): Expected %s got %s", test.expectedIpVersion, ipVersion)
}
}
}

View File

@ -377,11 +377,7 @@ func buildQueryString(qs string, repl *caddy.Replacer) string {
// performed in normalized/unescaped space.
func trimPathPrefix(escapedPath, prefix string) string {
var iPath, iPrefix int
for {
if iPath >= len(escapedPath) || iPrefix >= len(prefix) {
break
}
for iPath < len(escapedPath) && iPrefix < len(prefix) {
prefixCh := prefix[iPrefix]
ch := string(escapedPath[iPath])

View File

@ -171,6 +171,7 @@ func BenchmarkServer_LogRequest_WithTrace(b *testing.B) {
s.logRequest(accLog, req, wrec, &duration, repl, bodyReader, false)
}
}
func TestServer_TrustedRealClientIP_NoTrustedHeaders(t *testing.T) {
req := httptest.NewRequest("GET", "/", nil)
req.RemoteAddr = "192.0.2.1:12345"

View File

@ -388,10 +388,8 @@ func (ap *AutomationPolicy) onlyInternalIssuer() bool {
// isWildcardOrDefault determines if the subjects include any wildcard domains,
// or is the "default" policy (i.e. no subjects) which is unbounded.
func (ap *AutomationPolicy) isWildcardOrDefault() bool {
isWildcardOrDefault := false
if len(ap.subjects) == 0 {
isWildcardOrDefault = true
}
isWildcardOrDefault := len(ap.subjects) == 0
for _, sub := range ap.subjects {
if strings.HasPrefix(sub, "*") {
isWildcardOrDefault = true

View File

@ -317,7 +317,7 @@ func TestFileModeToJSON(t *testing.T) {
}{
{
name: "none zero",
mode: 0644,
mode: 0o644,
want: `"0644"`,
wantErr: false,
},
@ -358,7 +358,7 @@ func TestFileModeModification(t *testing.T) {
defer os.RemoveAll(dir)
fpath := path.Join(dir, "test.log")
f_tmp, err := os.OpenFile(fpath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, os.FileMode(0600))
f_tmp, err := os.OpenFile(fpath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, os.FileMode(0o600))
if err != nil {
t.Fatalf("failed to create test file: %v", err)
}

View File

@ -3,9 +3,10 @@ package logging
import (
"testing"
"go.uber.org/zap/zapcore"
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
"go.uber.org/zap/zapcore"
)
func TestIPMaskSingleValue(t *testing.T) {