diff --git a/cmd/gotemplate/OWNERS b/cmd/gotemplate/OWNERS new file mode 100644 index 00000000000..e634ba5e022 --- /dev/null +++ b/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 diff --git a/cmd/gotemplate/gotemplate.go b/cmd/gotemplate/gotemplate.go new file mode 100644 index 00000000000..afd033c203c --- /dev/null +++ b/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 = pairs which can be used as {{.}} to inject +// the for that key. +// +// Besides the default functions (https://pkg.go.dev/text/template#hdr-Functions), +// gotemplate also implements: +// - include : returns the content of that file as string +// - indent : replace each newline with "newline + spaces", indent the newline at the end +// - trim : 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 =, 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) +} diff --git a/cmd/gotemplate/gotemplate_test.go b/cmd/gotemplate/gotemplate_test.go new file mode 100644 index 00000000000..3322702ad96 --- /dev/null +++ b/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)) + }) + } +} diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml new file mode 100644 index 00000000000..527f267bff1 --- /dev/null +++ b/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" diff --git a/hack/golangci-strict.yaml b/hack/golangci-strict.yaml index 2515eb81ec8..57880634914 100644 --- a/hack/golangci-strict.yaml +++ b/hack/golangci-strict.yaml @@ -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: diff --git a/hack/golangci.yaml b/hack/golangci.yaml index 448374acf09..78b5f4b8326 100644 --- a/hack/golangci.yaml +++ b/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 - 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 isn’t 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 isn’t 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 diff --git a/hack/golangci.yaml.in b/hack/golangci.yaml.in new file mode 100644 index 00000000000..ebdcf426c3a --- /dev/null +++ b/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}} diff --git a/hack/lib/verify-generated.sh b/hack/lib/verify-generated.sh new file mode 100755 index 00000000000..4770f99ca5c --- /dev/null +++ b/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 +} diff --git a/hack/logcheck.yaml b/hack/logcheck.yaml new file mode 100644 index 00000000000..66047a693c1 --- /dev/null +++ b/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 diff --git a/hack/update-golangci-lint-config.sh b/hack/update-golangci-lint-config.sh new file mode 100755 index 00000000000..172ab070018 --- /dev/null +++ b/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 "${out}" "$@" +} + +# Regenerate. +generate hack/golangci.yaml Base=1 +generate hack/golangci-strict.yaml Strict=1 +generate hack/golangci-hints.yaml Hints=1 diff --git a/hack/verify-codegen.sh b/hack/verify-codegen.sh index 2442e2a6d3a..2072e87b62e 100755 --- a/hack/verify-codegen.sh +++ b/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 -# specific sub-projects. The sub-projects are listed below this script(the -# line that starts with `CODEGEN_PKG`). -# Usage: `hack/verify-codegen.sh`. +# This script verifies whether a code update is needed. +# 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 -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" +kube::verify::generated "Generated files need to be updated" "Please run 'hack/update-codegen.sh'" hack/update-codegen.sh "$@" diff --git a/hack/verify-golangci-lint-config.sh b/hack/verify-golangci-lint-config.sh index 93954551fad..236d3e589e5 100755 --- a/hack/verify-golangci-lint-config.sh +++ b/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 -# 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 <|-a] [-s] [-c none|] [-- : also write results with --out-format=github-actions to a separate file -c : 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.' diff --git a/hack/verify-internal-modules.sh b/hack/verify-internal-modules.sh index 9b2bef3824e..ed2af762167 100755 --- a/hack/verify-internal-modules.sh +++ b/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 -# 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 +kube::verify::generated "" "Run ./hack/update-internal-modules.sh" ./hack/update-internal-modules.sh diff --git a/hack/verify-mocks.sh b/hack/verify-mocks.sh index 4fa0db12237..13528756687 100755 --- a/hack/verify-mocks.sh +++ b/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 -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 +kube::verify::generated "Mock files are out of date" "Please run 'hack/update-mocks.sh'" hack/update-mocks.sh diff --git a/hack/verify-yamlfmt.sh b/hack/verify-yamlfmt.sh index 284b318fe24..a6545b5b0e1 100755 --- a/hack/verify-yamlfmt.sh +++ b/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 -# 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 +kube::verify::generated "YAML files need to be formatted" "Please run 'hack/update-yamlfmt.sh'" hack/update-yamlfmt.sh