golangci-lint: suppress one issue, demote others to "hints"

The voting in https://github.com/kubernetes/kubernetes/issues/117288 led to
one check that got rejected ("ifElseChain: rewrite if-else to switch
statement") and several that are "nice to know".

golangci-lint's support for issue "severity" is too limited to identify "nice
to know" issues in the output (filtering is only by linter without considering
the issue text; not part of text output). Therefore a third configuration gets
added which emits all issues (must fix and nits). The intention is to use
the "strict" configuration in pull-kubernetes-verify and the "hints"
configuration in a new non-blocking pull-kubernetes-linter-hints.

That way, "must fix" issues will block merging while issues that may be useful
will show up in a failed optional job. However, that job then also contains
"must fix" issues, partly because filtering out those would make the
configuration a lot larger and is likely to be unreliably (all "must fix"
issues would need to be identified and listed), partly because it may be useful
to have all issues in one place.

The previous approach of manually keeping two configs in sync with special
comments didn't scale to three configs. Now a single golangci.yaml.in with
text/template constructs contains the source for all three configs. A new
simple CLI frontend for text/template (cmd/gotemplate) is used by
hack/update-golangci-lint-config.sh to generate the three flavors.
This commit is contained in:
Patrick Ohly 2023-06-06 13:15:11 +02:00
parent 6a16c076e7
commit ce9e668a93
11 changed files with 696 additions and 51 deletions

11
cmd/gotemplate/OWNERS Normal file
View File

@ -0,0 +1,11 @@
# See the OWNERS docs at https://go.k8s.io/owners
reviewers:
- bentheelder
- dims
- pohly
approvers:
- dims
- pohly
labels:
- sig/testing

View File

@ -0,0 +1,97 @@
/*
Copyright 2023 The Kubernetes 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 main
import (
"bytes"
"fmt"
"io"
"os"
"strings"
"text/template"
)
// gotemplate is a simple CLI for text/template. It reads from stdin and writes to stdout.
// Optional arguments are <key>=<value> pairs which can be used as {{.<key>}} to inject
// the <value> for that key.
//
// Besides the default functions (https://pkg.go.dev/text/template#hdr-Functions),
// gotemplate also implements:
// - include <filename>: returns the content of that file as string
// - indent <number of spaces> <string>: replace each newline with "newline + spaces", indent the newline at the end
// - trim <string>: strip leading and trailing whitespace
func main() {
kvs := make(map[string]string)
for _, keyValue := range os.Args[1:] {
index := strings.Index(keyValue, "=")
if index <= 0 {
fmt.Fprintf(os.Stderr, "optional arguments must be of the form <key>=<value>, got instead: %q\n", keyValue)
os.Exit(1)
}
kvs[keyValue[0:index]] = keyValue[index+1:]
}
if err := generate(os.Stdin, os.Stdout, kvs); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
}
func generate(in io.Reader, out io.Writer, data interface{}) error {
var buf bytes.Buffer
if _, err := buf.ReadFrom(in); err != nil {
return fmt.Errorf("reading input: %v", err)
}
funcMap := template.FuncMap{
"include": include,
"indent": indent,
"trim": trim,
}
tmpl, err := template.New("").Funcs(funcMap).Parse(buf.String())
if err != nil {
return fmt.Errorf("parsing input as text template: %v", err)
}
if err := tmpl.Execute(out, data); err != nil {
return fmt.Errorf("generating result: %v", err)
}
return nil
}
func include(filename string) (string, error) {
content, err := os.ReadFile(filename)
if err != nil {
return "", err
}
return string(content), nil
}
func indent(numSpaces int, content string) string {
if content == "" {
return ""
}
prefix := strings.Repeat(" ", numSpaces)
return strings.ReplaceAll(content, "\n", "\n"+prefix)
}
func trim(content string) string {
return strings.TrimSpace(content)
}

View File

@ -0,0 +1,108 @@
/*
Copyright 2021 The Kubernetes 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 main
import (
"bytes"
"os"
"path"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestGenerate(t *testing.T) {
for name, tt := range map[string]struct {
in string
data map[string]string
files map[string]string
expected string
expectedErr string
}{
"missing-file": {
in: `{{include "no-such-file.txt"}}`,
expectedErr: "open no-such-file.txt: no such file or directory",
},
"data": {
in: `{{.Hello}} {{.World}}`,
data: map[string]string{"Hello": "world", "World": "hello"},
expected: "world hello",
},
"include": {
in: `{{include "test.txt" | indent 2}}`,
files: map[string]string{"test.txt": "hello\nworld"},
expected: "hello\n world",
},
} {
cwd, err := os.Getwd()
require.NoError(t, err)
t.Run(name, func(t *testing.T) {
tmp := t.TempDir()
for fileName, fileContent := range tt.files {
err := os.WriteFile(path.Join(tmp, fileName), []byte(fileContent), 0666)
require.NoError(t, err, "create input file")
}
defer os.Chdir(cwd)
require.NoError(t, os.Chdir(tmp), "change into tmp directory")
in := strings.NewReader(tt.in)
var out bytes.Buffer
err := generate(in, &out, tt.data)
if tt.expectedErr == "" {
require.NoError(t, err, "expand template")
require.Equal(t, tt.expected, out.String())
} else {
require.Contains(t, err.Error(), tt.expectedErr)
}
})
}
}
func TestIndent(t *testing.T) {
for name, tt := range map[string]struct {
numSpaces int
content string
expected string
}{
"empty": {
numSpaces: 10,
content: "",
expected: "",
},
"trailing-newline": {
numSpaces: 2,
content: "hello\nworld\n",
expected: "hello\n world\n ",
},
"no-trailing-newline": {
numSpaces: 1,
content: "hello\nworld",
expected: "hello\n world",
},
"zero-indent": {
numSpaces: 0,
content: "hello\nworld",
expected: "hello\nworld",
},
} {
t.Run(name, func(t *testing.T) {
assert.Equal(t, tt.expected, indent(tt.numSpaces, tt.content))
})
}
}

91
hack/golangci-hints.yaml Normal file
View File

@ -0,0 +1,91 @@
# golangci-lint is used in Kubernetes with different configurations that
# enable an increasing amount of checks:
# - golangci.yaml is the most permissive configuration. All existing code
# passed.
# - golangci-strict.yaml adds checks that all new code in pull requests
# must pass.
# - golangci-hints.yaml adds checks for code patterns where developer
# and reviewer may decide whether findings should get addressed before
# merging. Beware that the golangci-lint output includes also the
# issues that must be fixed and doesn't indicate how severe each issue
# is (https://gophers.slack.com/archives/CS0TBRKPC/p1685721815275349).
#
# All three flavors are generated from golangci.yaml.in with
# hack/update-golangci-lint-config.sh.
run:
timeout: 30m
skip-files:
- "^zz_generated.*"
output:
sort-results: true
issues:
max-issues-per-linter: 0
max-same-issues: 0
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
# exclude ineffassign linter for generated files for conversion
- path: conversion\.go
linters:
- ineffassign
# SSA Extract calls are allowed in tests.
- linters:
- forbidigo
text: should not be used because managedFields was removed
path: _test.go$
# The Kubernetes naming convention for conversion functions uses underscores
# and intentionally deviates from normal Go conventions to make those function
# names more readable. Same for SetDefaults_*.
#
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507028627
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
- linters:
- stylecheck
text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
# This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
- linters:
- ginkgolinter
text: use a function call in (Eventually|Consistently)
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507012435
- linters:
- gocritic
text: "ifElseChain: rewrite if-else to switch statement"
linters:
disable-all: false
enable: # please keep this alphabetized
- forbidigo
- ginkgolinter
- gocritic
- govet
- ineffassign
- logcheck
- staticcheck
- stylecheck
- unused
linters-settings: # please keep this alphabetized
custom:
logcheck:
# Installed there by hack/verify-golangci-lint.sh.
path: ../_output/local/bin/logcheck.so
description: structured logging checker
original-url: k8s.io/logtools/logcheck
forbidigo:
analyze-types: true
forbid:
- p: ^managedfields\.ExtractInto$
pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$
msg: should not be used because managedFields was removed
- p: \.Extract
pkg: ^k8s\.io/client-go/applyconfigurations/
msg: should not be used because managedFields was removed
staticcheck:
checks:
- "all"

View File

@ -1,13 +1,28 @@
# This file configures checks that all new code for Kubernetes is meant to
# pass, in contrast to .golangci.yaml which defines checks that also the
# existing code passes.
# golangci-lint is used in Kubernetes with different configurations that
# enable an increasing amount of checks:
# - golangci.yaml is the most permissive configuration. All existing code
# passed.
# - golangci-strict.yaml adds checks that all new code in pull requests
# must pass.
# - golangci-hints.yaml adds checks for code patterns where developer
# and reviewer may decide whether findings should get addressed before
# merging. Beware that the golangci-lint output includes also the
# issues that must be fixed and doesn't indicate how severe each issue
# is (https://gophers.slack.com/archives/CS0TBRKPC/p1685721815275349).
#
# All three flavors are generated from golangci.yaml.in with
# hack/update-golangci-lint-config.sh.
run:
timeout: 30m
skip-files:
- "^zz_generated.*"
output:
sort-results: true
issues:
max-issues-per-linter: 0
max-same-issues: 0
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
@ -15,25 +30,72 @@ issues:
- path: conversion\.go
linters:
- ineffassign
# SSA Extract calls are allowed in tests.
- linters:
- forbidigo
text: should not be used because managedFields was removed
path: _test.go$
# The Kubernetes naming convention for conversion functions uses underscores
# and intentionally deviates from normal Go conventions to make those function
# names more readable. Same for SetDefaults_*.
#
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507028627
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
- linters:
- stylecheck
text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
# This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
- linters:
- ginkgolinter
text: use a function call in (Eventually|Consistently)
# SSA Extract calls are allowed in tests.
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507012435
- linters:
- forbidigo
text: should not be used because managedFields was removed
path: _test.go$
- gocritic
text: "ifElseChain: rewrite if-else to switch statement"
# The following issues were deemed "might be worth fixing, needs to be
# decided on a case-by-case basis". This was initially decided by a
# majority of the developers who voted in
# https://github.com/kubernetes/kubernetes/issues/117288 and may evolve
# over time.
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008918
- linters:
- gocritic
text: "assignOp:"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507016854
- linters:
- gosimple
text: "S1002: should omit comparison to bool constant"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507023980
- linters:
- gosimple
text: "S1016: should convert opts .* instead of using struct literal"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507026758
- linters:
- gosimple
text: "S1033: unnecessary guard around call to delete"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507030071
- linters:
- stylecheck
text: "ST1012: error var .* should have name of the form ErrFoo"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507031224
- linters:
- stylecheck
text: "ST1023: should omit type .* from declaration; it will be inferred from the right-hand side"
linters:
disable-all: false # in contrast to golangci.yaml, the default set of linters remains enabled
enable: # please keep this alphabetized and in sync with golangci.yaml
disable-all: false
enable: # please keep this alphabetized
- forbidigo
- ginkgolinter
- gocritic
@ -43,6 +105,9 @@ linters:
- staticcheck
- stylecheck
- unused
disable:
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008359
- errcheck
linters-settings: # please keep this alphabetized
custom:
@ -60,8 +125,6 @@ linters-settings: # please keep this alphabetized
- p: \.Extract
pkg: ^k8s\.io/client-go/applyconfigurations/
msg: should not be used because managedFields was removed
gocritic:
staticcheck:
checks:
- "all"
stylecheck:

View File

@ -1,9 +1,28 @@
# golangci-lint is used in Kubernetes with different configurations that
# enable an increasing amount of checks:
# - golangci.yaml is the most permissive configuration. All existing code
# passed.
# - golangci-strict.yaml adds checks that all new code in pull requests
# must pass.
# - golangci-hints.yaml adds checks for code patterns where developer
# and reviewer may decide whether findings should get addressed before
# merging. Beware that the golangci-lint output includes also the
# issues that must be fixed and doesn't indicate how severe each issue
# is (https://gophers.slack.com/archives/CS0TBRKPC/p1685721815275349).
#
# All three flavors are generated from golangci.yaml.in with
# hack/update-golangci-lint-config.sh.
run:
timeout: 30m
skip-files:
- "^zz_generated.*"
output:
sort-results: true
issues:
max-issues-per-linter: 0
max-same-issues: 0
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
@ -11,30 +30,78 @@ issues:
- path: conversion\.go
linters:
- ineffassign
# SSA Extract calls are allowed in tests.
- linters:
- forbidigo
text: should not be used because managedFields was removed
path: _test.go$
# TODO(oscr) Remove these excluded directories and fix findings. Due to large amount of findings in different components
# with different owners it's hard to fix everything in a single pr. This will therefore be done in multiple prs.
- path: (pkg/volume/*|test/*|azure/*|pkg/cmd/wait*|request/bearertoken/*|metrics/*|filters/*) # not in golangci-strict.yaml
linters: # not in golangci-strict.yaml
- gocritic # not in golangci-strict.yaml
- path: (pkg/volume/*|test/*|azure/*|pkg/cmd/wait*|request/bearertoken/*|metrics/*|filters/*)
linters:
- gocritic
# The Kubernetes naming convention for conversion functions uses underscores
# and intentionally deviates from normal Go conventions to make those function
# names more readable. Same for SetDefaults_*.
#
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507028627
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
- linters:
- stylecheck
text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
# This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
- linters:
- ginkgolinter
text: use a function call in (Eventually|Consistently)
# SSA Extract calls are allowed in tests.
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507012435
- linters:
- forbidigo
text: should not be used because managedFields was removed
path: _test.go$
- gocritic
text: "ifElseChain: rewrite if-else to switch statement"
# The following issues were deemed "might be worth fixing, needs to be
# decided on a case-by-case basis". This was initially decided by a
# majority of the developers who voted in
# https://github.com/kubernetes/kubernetes/issues/117288 and may evolve
# over time.
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008918
- linters:
- gocritic
text: "assignOp:"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507016854
- linters:
- gosimple
text: "S1002: should omit comparison to bool constant"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507023980
- linters:
- gosimple
text: "S1016: should convert opts .* instead of using struct literal"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507026758
- linters:
- gosimple
text: "S1033: unnecessary guard around call to delete"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507030071
- linters:
- stylecheck
text: "ST1012: error var .* should have name of the form ErrFoo"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507031224
- linters:
- stylecheck
text: "ST1023: should omit type .* from declaration; it will be inferred from the right-hand side"
linters:
disable-all: true # not disabled in golangci-strict.yaml
enable: # please keep this alphabetized and in sync with golangci-strict.yaml
disable-all: true
enable: # please keep this alphabetized
- forbidigo
- ginkgolinter
- gocritic
@ -62,14 +129,14 @@ linters-settings: # please keep this alphabetized
pkg: ^k8s\.io/client-go/applyconfigurations/
msg: should not be used because managedFields was removed
gocritic:
enabled-checks: # not limited in golangci-strict.yaml
- equalFold # not limited in golangci-strict.yaml
- boolExprSimplify # not limited in golangci-strict.yaml
enabled-checks:
- equalFold
- boolExprSimplify
staticcheck:
checks:
- "all"
- "-SA1019" # TODO(fix) Using a deprecated function, variable, constant or field - enabled in golangci-strict.yaml
- "-SA2002" # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isnt allowed - enabled in golangci-strict.yaml
- "-SA1019" # TODO(fix) Using a deprecated function, variable, constant or field
- "-SA2002" # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isnt allowed
stylecheck:
checks: # golangci-strict.yaml uses the default checks.
- "ST1019" # Importing the same package multiple times - enabled in golangci-strict.yaml.
checks:
- "ST1019" # Importing the same package multiple times

159
hack/golangci.yaml.in Normal file
View File

@ -0,0 +1,159 @@
# golangci-lint is used in Kubernetes with different configurations that
# enable an increasing amount of checks:
# - golangci.yaml is the most permissive configuration. All existing code
# passed.
# - golangci-strict.yaml adds checks that all new code in pull requests
# must pass.
# - golangci-hints.yaml adds checks for code patterns where developer
# and reviewer may decide whether findings should get addressed before
# merging. Beware that the golangci-lint output includes also the
# issues that must be fixed and doesn't indicate how severe each issue
# is (https://gophers.slack.com/archives/CS0TBRKPC/p1685721815275349).
#
# All three flavors are generated from golangci.yaml.in with
# hack/update-golangci-lint-config.sh.
run:
timeout: 30m
skip-files:
- "^zz_generated.*"
output:
sort-results: true
issues:
max-issues-per-linter: 0
max-same-issues: 0
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
# exclude ineffassign linter for generated files for conversion
- path: conversion\.go
linters:
- ineffassign
# SSA Extract calls are allowed in tests.
- linters:
- forbidigo
text: should not be used because managedFields was removed
path: _test.go$
{{- if .Base}}
# TODO(oscr) Remove these excluded directories and fix findings. Due to large amount of findings in different components
# with different owners it's hard to fix everything in a single pr. This will therefore be done in multiple prs.
- path: (pkg/volume/*|test/*|azure/*|pkg/cmd/wait*|request/bearertoken/*|metrics/*|filters/*)
linters:
- gocritic
{{- end}}
# The Kubernetes naming convention for conversion functions uses underscores
# and intentionally deviates from normal Go conventions to make those function
# names more readable. Same for SetDefaults_*.
#
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507028627
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1514201592
- linters:
- stylecheck
text: "ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)"
# This check currently has some false positives (https://github.com/nunnatsa/ginkgolinter/issues/91).
- linters:
- ginkgolinter
text: use a function call in (Eventually|Consistently)
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507012435
- linters:
- gocritic
text: "ifElseChain: rewrite if-else to switch statement"
{{- if not .Hints}}
# The following issues were deemed "might be worth fixing, needs to be
# decided on a case-by-case basis". This was initially decided by a
# majority of the developers who voted in
# https://github.com/kubernetes/kubernetes/issues/117288 and may evolve
# over time.
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008918
- linters:
- gocritic
text: "assignOp:"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507016854
- linters:
- gosimple
text: "S1002: should omit comparison to bool constant"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507023980
- linters:
- gosimple
text: "S1016: should convert opts .* instead of using struct literal"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507026758
- linters:
- gosimple
text: "S1033: unnecessary guard around call to delete"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507030071
- linters:
- stylecheck
text: "ST1012: error var .* should have name of the form ErrFoo"
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507031224
- linters:
- stylecheck
text: "ST1023: should omit type .* from declaration; it will be inferred from the right-hand side"
{{- end}}
linters:
disable-all: {{if .Base -}} true {{- else -}} false {{- end}}
enable: # please keep this alphabetized
- forbidigo
- ginkgolinter
- gocritic
- govet
- ineffassign
- logcheck
- staticcheck
- stylecheck
- unused
{{- if .Strict}}
disable:
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008359
- errcheck
{{- end}}
linters-settings: # please keep this alphabetized
custom:
logcheck:
# Installed there by hack/verify-golangci-lint.sh.
path: ../_output/local/bin/logcheck.so
description: structured logging checker
original-url: k8s.io/logtools/logcheck
forbidigo:
analyze-types: true
forbid:
- p: ^managedfields\.ExtractInto$
pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$
msg: should not be used because managedFields was removed
- p: \.Extract
pkg: ^k8s\.io/client-go/applyconfigurations/
msg: should not be used because managedFields was removed
{{- if .Base }}
gocritic:
enabled-checks:
- equalFold
- boolExprSimplify
{{- end}}
staticcheck:
checks:
- "all"
{{- if .Base }}
- "-SA1019" # TODO(fix) Using a deprecated function, variable, constant or field
- "-SA2002" # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isnt allowed
{{- end}}
{{- if .Base }}
stylecheck:
checks:
- "ST1019" # Importing the same package multiple times
{{- end}}

6
hack/logcheck.yaml Normal file
View File

@ -0,0 +1,6 @@
# Installed there by hack/verify-golangci-lint.sh.
path: ../_output/local/bin/logcheck.so
description: structured logging checker
original-url: k8s.io/logtools/logcheck
settings:
config: /nvme/gopath/src/k8s.io/kubernetes/hack/logcheck.conf

View File

@ -0,0 +1,44 @@
#!/usr/bin/env bash
# Copyright 2023 The Kubernetes 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.
# This script checks that golangci-strict.yaml and golangci.yaml remain in
# sync. Lines that are intentionally different must have a comment which
# mentions golangci.yaml or golangci-lint.yaml.
set -o errexit
set -o nounset
set -o pipefail
KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${KUBE_ROOT}/hack/lib/init.sh"
# This sets up the environment, like GOCACHE, which keeps the worktree cleaner.
kube::golang::setup_env
# Remove all files, some of them might be obsolete.
rm -f hack/golangci*.yaml
generate () {
out="$1"
shift
echo "Generating $out from hack/golangci.yaml.in with ./cmd/gotemplate $*"
go run ./cmd/gotemplate <hack/golangci.yaml.in >"${out}" "$@"
}
# Regenerate.
generate hack/golangci.yaml Base=1
generate hack/golangci-strict.yaml Strict=1
generate hack/golangci-hints.yaml Hints=1

View File

@ -14,30 +14,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
# This script checks that golangci-strict.yaml and golangci.yaml remain in
# sync. Lines that are intentionally different must have a comment which
# mentions golangci.yaml or golangci-lint.yaml.
# This script checks that all generated golangci-lint configurations
# are up-to-date.
set -o errexit
set -o nounset
set -o pipefail
KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
source "${KUBE_ROOT}/hack/lib/verify-generated.sh"
if differences=$(diff --context --ignore-blank-lines \
--ignore-matching-lines='^ *#' \
--ignore-matching-lines='#.*golangci\(-strict\)*.yaml' \
"${KUBE_ROOT}/hack/golangci.yaml" "${KUBE_ROOT}/hack/golangci-strict.yaml" ); then
echo "hack/golangci.yaml and hack/golangci-strict.yaml are synchronized."
else
cat >&2 <<EOF
Unexpected differences between hack/golangci.yaml and hack/golangci-strict.yaml:
${differences}
If these differences are intentional, then add comments at the end of each
different line in both files that mention golangci-strict.yaml or
golangci.yaml.
EOF
exit 1
fi
kube::verify::generated "" "Please run 'hack/update-golangci-lint-config.sh'" hack/update-golangci-lint-config.sh

View File

@ -26,6 +26,8 @@ Usage: $0 [-r <revision>|-a] [-s] [-c none|<config>] [-- <golangci-lint run flag
-a: automatically select the common base of origin/master and HEAD
as revision
-s: select a strict configuration for new code
-n: in addition to strict checking, also enable hints (aka nits) that may are may not
be useful
-g <github action file>: also write results with --out-format=github-actions
to a separate file
-c <config|"none">: use the specified configuration or none instead of the default hack/golangci.yaml
@ -65,8 +67,9 @@ golangci=(env LOGCHECK_CONFIG="${KUBE_ROOT}/hack/logcheck.conf" "${GOBIN}/golang
golangci_config="${KUBE_ROOT}/hack/golangci.yaml"
base=
strict=
hints=
githubactions=
while getopts "ar:sg:c:" o; do
while getopts "ar:sng:c:" o; do
case "${o}" in
a)
base="$(git merge-base origin/master HEAD)"
@ -83,6 +86,10 @@ while getopts "ar:sg:c:" o; do
golangci_config="${KUBE_ROOT}/hack/golangci-strict.yaml"
strict=1
;;
n)
golangci_config="${KUBE_ROOT}/hack/golangci-hints.yaml"
hints=1
;;
g)
githubactions="${OPTARG}"
;;
@ -192,9 +199,17 @@ else
echo 'If the above warnings do not make sense, you can exempt this warning with a comment'
echo ' (if your reviewer is okay with it).'
if [ "$strict" ]; then
echo 'The more strict golangci-strict.yaml was used. If you feel that this warns about issues'
echo 'that should be ignored by default, then please discuss with your reviewer and propose'
echo 'a change for hack/golangci-strict.yaml as part of your PR.'
echo 'The more strict golangci-strict.yaml was used.'
elif [ "$hints" ]; then
echo 'The golangci-hints.yaml was used. Some of the reported issues may have to be fixed'
echo 'while others can be ignored, depending on the circumstances and/or personal'
echo 'preferences. To determine which issues have to be fixed, check the report that'
echo 'uses golangci-strict.yaml.'
fi
if [ "$strict" ] || [ "$hints" ]; then
echo 'If you feel that this warns about issues that should be ignored by default,'
echo 'then please discuss with your reviewer and propose'
echo 'a change for hack/golangci.yaml.in as part of your PR.'
fi
echo 'In general please prefer to fix the error, we have already disabled specific lints'
echo ' that the project chooses to ignore.'