github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-01 07:47:56 +00:00
Code Issues Projects Releases Wiki Activity

golangci-lint: remove "strict" checking

Browse Source 
The corresponding "pull-kubernetes-verify-lint" job was already removed
earlier. Manual strict checking was still possible, but doesn't really make
sense for the same reasons why the job was removed (e.g. the decisions which
checks should be "strict" were too arbitrary).

The explanations for "hints" no longer end with "In general please prefer to
fix the error, ..." because that was misleading and only really applied to the
checks for existing code. For those checks we prefer to fix errors instead of
suppressing them, but not for hints.
This commit is contained in:
Patrick Ohly 2025-01-29 09:39:53 +01:00
parent 6e3546228d
commit 949385731f
6 changed files with 7 additions and 299 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
hack/golangci-hints.yaml
View File

@ -2,8 +2,6 @@
# 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

269
hack/golangci-strict.yaml
View File

@ -1,269 +0,0 @@
# 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
# The default excludes disable the "should have comment or be unexported" check from revive.
# We want that to be enabled, therefore we have to disable all default excludes and
# add those back one-by-one that we want. See https://github.com/golangci/golangci-lint/issues/456#issuecomment-617470264
exclude-use-default: false
exclude:
# staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
- ineffective break statement. Did you mean to break out of the outer loop
# 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$
# Adding unversioned feature gates is allowed in tests
- linters:
- forbidigo
text: should not use MutableFeatureGate.Add, use AddVersioned instead
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
- revive
text: "(ST1003: should not use underscores in Go names; func (Convert_.*_To_.*|SetDefaults_)|exported: exported function (Convert|SetDefaults)_.* should be of the form)"
# 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"
# Only packages listed here opt into the strict "exported symbols must be documented".
#
# Exclude texts from https://github.com/golangci/golangci-lint/blob/ab3c3cd69e602ff53bb4c3e2c188f0caeb80305d/pkg/config/issues.go#L11-L103
- linters:
- golint
- revive
- stylecheck
text: comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form|exported (.+) should have comment( \(or a comment on this block\))? or be unexported|package comment should be of the form "(.+)...|comment on exported (.+) should be of the form "(.+)...|should have a package comment
path-except: cmd/kubeadm
# The unused linter that comes from staticcheck currently does not handle types which implement
# a generic interface. The linter incorrectly reports the implementations of unexported
# interface methods as unused. See https://github.com/dominikh/go-tools/issues/1294.
# Rather than exporting the interface methods, which makes the error go away but changes the
# semantics of the code, we ignore this error for affected files.
# This can be removed when the staticcheck implementation of this rule is fixed, which may
# depend on https://github.com/golang/go/issues/63982.
- linters:
- unused
path: staging/src/k8s.io/client-go/util/workqueue/metrics.go
# 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"
# Didn't make it into https://github.com/kubernetes/kubernetes/issues/117288.
# Discussion on Slack concluded that "it's hard to have a universal policy for all
# functions marked deprecated" and thus this can only be a hint which must
# be considered on a case-by-case basis.
- linters:
- staticcheck
text: "SA1019: .*is deprecated"
# 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
enable: # please keep this alphabetized
- depguard
- forbidigo
- ginkgolinter
- gocritic
- govet
- ineffassign
- logcheck
- revive
- staticcheck
- stylecheck
- testifylint
- unused
- usestdlibvars
disable:
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008359
- errcheck
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
settings:
config: |
# hack/logcheck.conf contains regular expressions that are matched against <pkg>/<file>,
# for example k8s.io/cmd/kube-scheduler/app/config/config.go.
#
# By default, structured logging call parameters are checked, but usage of
# those calls is not required. That is changed on a per-file basis.
#
# Remember to clean the golangci-lint cache when changing the configuration and
# running the verify-golangci-lint.sh script multiple times, otherwise
# golangci-lint will report stale results:
# _output/local/bin/golangci-lint cache clean
# At this point we don't enforce the usage structured logging calls except in
# those packages that were migrated. This disables the check for other files.
-structured .*
# Now enable it again for migrated packages.
structured k8s.io/kubernetes/cmd/kubelet/.*
structured k8s.io/kubernetes/pkg/kubelet/.*
structured k8s.io/kubernetes/pkg/proxy/.*
structured k8s.io/kms/.*
structured k8s.io/apiserver/pkg/storage/value/.*
structured k8s.io/apiserver/pkg/server/options/encryptionconfig/.*
# The following packages have been migrated to contextual logging.
# Packages matched here do not have to be listed above because
# "contextual" implies "structured".
contextual k8s.io/api/.*
contextual k8s.io/apimachinery/pkg/util/runtime/.*
contextual k8s.io/client-go/metadata/.*
contextual k8s.io/client-go/rest/.*
contextual k8s.io/client-go/tools/cache/.*
contextual k8s.io/client-go/tools/events/.*
contextual k8s.io/client-go/tools/record/.*
contextual k8s.io/component-helpers/.*
contextual k8s.io/cri-api/.*
contextual k8s.io/cri-client/.*
contextual k8s.io/csi-translation-lib/.*
contextual k8s.io/dynamic-resource-allocation/.*
contextual k8s.io/endpointslice/.*
contextual k8s.io/kms/.*
contextual k8s.io/kube-controller-manager/.*
contextual k8s.io/kube-proxy/.*
contextual k8s.io/kube-scheduler/.*
contextual k8s.io/sample-apiserver/.*
contextual k8s.io/sample-cli-plugin/.*
contextual k8s.io/sample-controller/.*
contextual k8s.io/kubernetes/cmd/kube-proxy/.*
contextual k8s.io/kubernetes/cmd/kube-scheduler/.*
contextual k8s.io/kubernetes/pkg/controller/.*
contextual k8s.io/kubernetes/pkg/scheduler/.*
contextual k8s.io/kubernetes/test/e2e/dra/.*
contextual k8s.io/kubernetes/pkg/kubelet/cm/dra/.*
contextual k8s.io/kubernetes/pkg/kubelet/pleg/.*
contextual k8s.io/kubernetes/pkg/kubelet/clustertrustbundle/.*
contextual k8s.io/kubernetes/pkg/kubelet/token/.*
contextual k8s.io/kubernetes/pkg/kubelet/cadvisor/.*
contextual k8s.io/kubernetes/pkg/kubelet/oom/.*
contextual k8s.io/kubernetes/pkg/kubelet/sysctl/.*
# As long as contextual logging is alpha or beta, all WithName, WithValues,
# NewContext calls have to go through klog. Once it is GA, we can lift
# this restriction. Whether we then do a global search/replace remains
# to be decided.
with-helpers .*
depguard:
rules:
main:
files:
- $all
- "!$test"
- "!**/test/**"
deny:
- pkg: "github.com/google/go-cmp/cmp"
desc: "cmp is allowed only in test files"
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
- p: \.Add$
pkg: ^k8s\.io/component-base/featuregate$
type: ^MutableFeatureGate$
msg: should not use MutableFeatureGate.Add, use AddVersioned instead
revive:
# Only these rules are enabled.
rules:
- name: exported
arguments:
- disableStutteringCheck
staticcheck:
checks:
- "all"
testifylint:
enable-all: true
disable: # TODO: remove each disabled rule and fix it
- require-error

2
hack/golangci.yaml
View File

@ -2,8 +2,6 @@
# 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

7
hack/golangci.yaml.in
View File

@ -2,8 +2,6 @@
# 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
@ -171,11 +169,6 @@ linters:
{{- if not .Base }}
- usestdlibvars
{{- end}}
{{- if .Strict}}
disable:
# https://github.com/kubernetes/kubernetes/issues/117288#issuecomment-1507008359
- errcheck
{{- end}}
linters-settings: # please keep this alphabetized
custom:

1
hack/update-golangci-lint-config.sh
View File

@ -40,5 +40,4 @@ generate () {
# Regenerate.
generate hack/golangci.yaml Base=1
generate hack/golangci-strict.yaml Strict=1
generate hack/golangci-hints.yaml Hints=1

25
hack/verify-golangci-lint.sh
View File

@ -25,7 +25,6 @@ Usage: $0 [-r <revision>|-a] [-s] [-c none|<config>] [-- <golangci-lint run flag
-r <revision>: only report issues in code added since that revision
-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 or may not
be useful
-g <github action file>: also write results with --out-format=github-actions
@ -53,7 +52,6 @@ invocation=(./hack/verify-golangci-lint.sh "$@")
golangci=("${GOBIN}/golangci-lint" run)
golangci_config="${KUBE_ROOT}/hack/golangci.yaml"
base=
strict=
hints=
githubactions=
while getopts "ar:sng:c:" o; do
@ -69,10 +67,6 @@ while getopts "ar:sng:c:" o; do
usage
fi
;;
s)
golangci_config="${KUBE_ROOT}/hack/golangci-strict.yaml"
strict=1
;;
n)
golangci_config="${KUBE_ROOT}/hack/golangci-hints.yaml"
hints=1
@ -185,18 +179,12 @@ else
echo "Please review the above warnings. You can test via \"${invocation[*]}\""
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
echo 'golangci-strict.yaml was used as configuration. Warnings must be fixed in'
echo 'new or modified code.'
elif [ "$hints" ]; then
if [ "$hints" ]; then
echo
echo 'golangci-hints.yaml was used as configuration. Some of the reported issues may'
echo 'have to be fixed while others can be ignored, depending on the circumstances'
echo 'and/or personal preferences. To determine which issues have to be fixed, check'
echo 'the report that uses golangci-strict.yaml (= pull-kubernetes-verify-lint).'
fi
if [ "$strict" ] || [ "$hints" ]; then
echo 'the report that uses golangci.yaml (= pull-kubernetes-verify).'
echo
echo 'If you feel that this warns about issues that should be ignored by default,'
echo 'then please discuss with your reviewer and propose'
@ -208,12 +196,13 @@ else
echo 'Instead, propose to fix certain linter issues in an issue first and'
echo 'discuss there with maintainers. PRs are welcome if they address a real'
echo 'problem, which then needs to be explained in the PR.'
else
echo
echo 'In general please prefer to fix the error, we have already disabled specific lints'
echo ' that the project chooses to ignore.'
echo 'See: https://golangci-lint.run/usage/false-positives/'
fi
echo
echo 'In general please prefer to fix the error, we have already disabled specific lints'
echo ' that the project chooses to ignore.'
echo 'See: https://golangci-lint.run/usage/false-positives/'
echo
} >&2
exit 1
fi