Merge pull request #118502 from pohly/golangci-lint-hints

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

cmd/gotemplate/OWNERS

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

cmd/gotemplate/gotemplate.go

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

cmd/gotemplate/gotemplate_test.go

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

hack/golangci-hints.yaml

@@ -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"

hack/golangci-strict.yaml

@@ -1,13 +1,28 @@
-# This file configures checks that all new code for Kubernetes is meant to
+# golangci-lint is used in Kubernetes with different configurations that
-# pass, in contrast to .golangci.yaml which defines checks that also the
+# enable an increasing amount of checks:
-# existing code passes.
+# - 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
+        - gocritic
-      text: should not be used because managedFields was removed
+      text: "ifElseChain: rewrite if-else to switch statement"
-      path: _test.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"
+
+    # 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
+  disable-all: false
-  enable: # please keep this alphabetized and in sync with golangci.yaml
+  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:

hack/golangci.yaml

@@ -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
+    - path: (pkg/volume/*|test/*|azure/*|pkg/cmd/wait*|request/bearertoken/*|metrics/*|filters/*)
-      linters:                                                                                    # not in golangci-strict.yaml
+      linters:
-        - gocritic                                                                                # not in golangci-strict.yaml
+        - 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
+        - gocritic
-      text: should not be used because managedFields was removed
+      text: "ifElseChain: rewrite if-else to switch statement"
-      path: _test.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"
+
+    # 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
+  disable-all: true
-  enable: # please keep this alphabetized and in sync with golangci-strict.yaml
+  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
+    enabled-checks:
-      - equalFold               # not limited in golangci-strict.yaml
+      - equalFold
-      - boolExprSimplify        # not limited in golangci-strict.yaml
+      - boolExprSimplify
   staticcheck:
     checks:
       - "all"
-      - "-SA1019"  # TODO(fix) Using a deprecated function, variable, constant or field - 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 isn’t allowed - enabled in golangci-strict.yaml
+      - "-SA2002"  # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isn’t allowed
   stylecheck:
-    checks:        # golangci-strict.yaml uses the default checks.
+    checks:
-      - "ST1019"   # Importing the same package multiple times - enabled in golangci-strict.yaml.
+      - "ST1019"   # Importing the same package multiple times

hack/golangci.yaml.in

@@ -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 isn’t allowed
+      {{- end}}
+  {{- if .Base }}
+  stylecheck:
+    checks:
+      - "ST1019"   # Importing the same package multiple times
+  {{- end}}

hack/lib/verify-generated.sh

@@ -0,0 +1,68 @@
+#!/usr/bin/env bash
+
+# Copyright 2014 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.
+
+# Short-circuit if verify-generated.sh has already been sourced.
+[[ $(type -t kube::verify::generated::loaded) == function ]] && return 0
+
+source "${KUBE_ROOT}/hack/lib/init.sh"
+
+# This function verifies whether generated files are up-to-date. The first two
+# parameters are messages that get printed to stderr when changes are found,
+# the rest are the function or command and its parameters for generating files
+# in the work tree.
+#
+# Example: kube::verify::generated "Mock files are out of date" "Please run 'hack/update-mocks.sh'" hack/update-mocks.sh
+kube::verify::generated() {
+  ( # a subshell prevents environment changes from leaking out of this function
+    local failure_header=$1
+    shift
+    local failure_tail=$1
+    shift
+
+    kube::util::ensure_clean_working_dir
+
+    # This sets up the environment, like GOCACHE, which keeps the worktree cleaner.
+    kube::golang::setup_env
+
+    _tmpdir="$(kube::realpath "$(mktemp -d -t "verify-generated-$(basename "$1").XXXXXX")")"
+    git worktree add -f -q "${_tmpdir}" HEAD
+    kube::util::trap_add "git worktree remove -f ${_tmpdir}" EXIT
+    cd "${_tmpdir}"
+
+    # Update generated files.
+    "$@"
+
+    # Test for diffs
+    diffs=$(git status --porcelain | wc -l)
+    if [[ ${diffs} -gt 0 ]]; then
+      if [[ -n "${failure_header}" ]]; then
+        echo "${failure_header}" >&2
+      fi
+      git status >&2
+      git diff >&2
+      if [[ -n "${failure_tail}" ]]; then
+        echo "" >&2
+        echo "${failure_tail}" >&2
+      fi
+      return 1
+    fi
+  )
+}
+
+# Marker function to indicate verify-generated.sh has been fully sourced.
+kube::verify::generated::loaded() {
+  return 0
+}

hack/logcheck.yaml

@@ -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

hack/update-golangci-lint-config.sh

@@ -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

hack/verify-codegen.sh

@@ -14,39 +14,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# This script verifies whether code update is needed or not against the
+# This script verifies whether a code update is needed.
-# specific sub-projects. The sub-projects are listed below this script(the
+# Usage: `hack/verify-codegen.sh <parameters for update-codegen.sh>`.
-# line that starts with `CODEGEN_PKG`).
-# Usage: `hack/verify-codegen.sh`.
 
 set -o errexit
 set -o nounset
 set -o pipefail
 
 KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
-source "${KUBE_ROOT}/hack/lib/init.sh"
+source "${KUBE_ROOT}/hack/lib/verify-generated.sh"
 
-kube::util::ensure_clean_working_dir
-
-# This sets up the environment, like GOCACHE, which keeps the worktree cleaner.
-kube::golang::setup_env
-
-_tmpdir="$(kube::realpath "$(mktemp -d -t "$(basename "$0").XXXXXX")")"
-git worktree add -f -q "${_tmpdir}" HEAD
-kube::util::trap_add "git worktree remove -f ${_tmpdir}" EXIT
-cd "${_tmpdir}"
-
-# Update generated code
 export UPDATE_API_KNOWN_VIOLATIONS=true
-hack/update-codegen.sh "$@"
 
-# Test for diffs
+kube::verify::generated "Generated files need to be updated" "Please run 'hack/update-codegen.sh'" hack/update-codegen.sh "$@"
-diffs=$(git status --porcelain | wc -l)
-if [[ ${diffs} -gt 0 ]]; then
-  git status >&2
-  git diff >&2
-  echo "Generated files need to be updated" >&2
-  echo "Please run 'hack/update-codegen.sh'" >&2
-  exit 1
-fi
-echo "Generated files are up to date"

hack/verify-golangci-lint-config.sh

@@ -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
+# This script checks that all generated golangci-lint configurations
-# sync. Lines that are intentionally different must have a comment which
+# are up-to-date.
-# 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/verify-generated.sh"
 
-if differences=$(diff --context --ignore-blank-lines \
+kube::verify::generated "" "Please run 'hack/update-golangci-lint-config.sh'" hack/update-golangci-lint-config.sh
-                      --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

hack/verify-golangci-lint.sh

@@ -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 'The more strict golangci-strict.yaml was used.'
-        echo 'that should be ignored by default, then please discuss with your reviewer and propose'
+    elif [ "$hints" ]; then
-        echo 'a change for hack/golangci-strict.yaml as part of your PR.'
+        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.'

hack/verify-internal-modules.sh

@@ -19,30 +19,6 @@ set -o nounset
 set -o pipefail
 
 KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
-source "${KUBE_ROOT}/hack/lib/init.sh"
+source "${KUBE_ROOT}/hack/lib/verify-generated.sh"
 
-kube::util::ensure_clean_working_dir
+kube::verify::generated "" "Run ./hack/update-internal-modules.sh" ./hack/update-internal-modules.sh
-# This sets up the environment, like GOCACHE, which keeps the worktree cleaner.
-kube::golang::setup_env
-
-_tmpdir="$(kube::realpath "$(mktemp -d -t verify-internal-modules.XXXXXX)")"
-kube::util::trap_add "rm -rf ${_tmpdir:?}" EXIT
-
-_tmp_gopath="${_tmpdir}/go"
-_tmp_kuberoot="${_tmp_gopath}/src/k8s.io/kubernetes"
-git worktree add -f "${_tmp_kuberoot}" HEAD
-kube::util::trap_add "git worktree remove -f ${_tmp_kuberoot}" EXIT
-
-pushd "${_tmp_kuberoot}" >/dev/null
-./hack/update-internal-modules.sh
-popd
-
-git -C "${_tmp_kuberoot}" add -N .
-diff=$(git -C "${_tmp_kuberoot}" diff HEAD || true)
-
-if [[ -n "${diff}" ]]; then
-  echo "${diff}" >&2
-  echo >&2
-  echo "Run ./hack/update-internal-modules.sh" >&2
-  exit 1
-fi

hack/verify-mocks.sh

@@ -25,28 +25,6 @@ set -o nounset
 set -o pipefail
 
 KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
-source "${KUBE_ROOT}/hack/lib/init.sh"
+source "${KUBE_ROOT}/hack/lib/verify-generated.sh"
 
-# Explicitly opt into go modules, even though we're inside a GOPATH directory
+kube::verify::generated "Mock files are out of date" "Please run 'hack/update-mocks.sh'" hack/update-mocks.sh
-export GO111MODULE=on
-
-kube::util::ensure_clean_working_dir
-# This sets up the environment, like GOCACHE, which keeps the worktree cleaner.
-kube::golang::setup_env
-
-_tmpdir="$(kube::realpath "$(mktemp -d -t "$(basename "$0").XXXXXX")")"
-git worktree add -f -q "${_tmpdir}" HEAD
-kube::util::trap_add "git worktree remove -f ${_tmpdir}" EXIT
-cd "${_tmpdir}"
-
-# Update the mocks in ${_tmpdir}
-hack/update-mocks.sh
-
-# Test for diffs
-diffs=$(git status --porcelain | wc -l)
-if [[ ${diffs} -gt 0 ]]; then
-  echo "Mock files are out of date" >&2
-  git diff
-  echo "Please run 'hack/update-mocks.sh'" >&2
-  exit 1
-fi

hack/verify-yamlfmt.sh

@@ -25,25 +25,6 @@ set -o nounset
 set -o pipefail
 
 KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
-source "${KUBE_ROOT}/hack/lib/init.sh"
+source "${KUBE_ROOT}/hack/lib/verify-generated.sh"
 
-kube::util::ensure_clean_working_dir
+kube::verify::generated "YAML files need to be formatted" "Please run 'hack/update-yamlfmt.sh'" hack/update-yamlfmt.sh
-# This sets up the environment, like GOCACHE, which keeps the worktree cleaner.
-kube::golang::setup_env
-
-_tmpdir="$(kube::realpath "$(mktemp -d -t "$(basename "$0").XXXXXX")")"
-git worktree add -f -q "${_tmpdir}" HEAD
-kube::util::trap_add "git worktree remove -f ${_tmpdir}" EXIT
-cd "${_tmpdir}"
-
-# Format YAML files
-hack/update-yamlfmt.sh
-
-# Test for diffs
-diffs=$(git status --porcelain | wc -l)
-if [[ ${diffs} -gt 0 ]]; then
-  echo "YAML files need to be formatted" >&2
-  git diff
-  echo "Please run 'hack/update-yamlfmt.sh'" >&2
-  exit 1
-fi