hack: integrate logcheck into golangci-lint

Running logcheck as part of golangci-lint has several advantages:
- faster checking because finding files and parsing is shared
  with other linters
- gets rid of the complex and buggy
  hack/verify-structured-logging.sh (https://github.com/kubernetes/kubernetes/issues/106746)
- support for // nolint:logcheck
- works with Go 1.18
This commit is contained in:
Patrick Ohly 2022-01-25 11:02:09 +01:00
parent edffc700a4
commit 17e3c555c5
8 changed files with 45 additions and 118 deletions

View File

@ -21,10 +21,17 @@ linters:
# - structcheck
# - varcheck
- ineffassign
- logcheck
- staticcheck
- 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/klog/hack/tools
staticcheck:
go: "1.17"
checks: [

View File

@ -1,15 +0,0 @@
cmd/kube-proxy/...
cmd/kube-scheduler/app/config/...
cmd/kube-scheduler/app/testing/...
cmd/kubelet/...
pkg/kubelet/...
pkg/proxy/...
pkg/scheduler/apis/...
pkg/scheduler/framework/...
pkg/scheduler/internal/cache/fake/...
pkg/scheduler/internal/heap/...
pkg/scheduler/internal/queue/...
pkg/scheduler/metrics/...
pkg/scheduler/profile/...
pkg/scheduler/testing/...
pkg/scheduler/util/...

22
hack/logcheck.conf Normal file
View File

@ -0,0 +1,22 @@
# This file 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/kube-proxy/.*
structured k8s.io/kubernetes/cmd/kube-scheduler/.*
structured k8s.io/kubernetes/cmd/kubelet/.*
structured k8s.io/kubernetes/pkg/kubelet/.*
structured k8s.io/kubernetes/pkg/proxy/.*
structured k8s.io/kubernetes/pkg/scheduler/.*

View File

@ -35,7 +35,6 @@ EXCLUDED_PATTERNS=(
"verify-linkcheck.sh" # runs in separate Jenkins job once per day due to high network usage
"verify-*-dockerized.sh" # Don't run any scripts that intended to be run dockerized
"verify-govet-levee.sh" # Do not run levee analysis by default while KEP-1933 implementation is in alpha.
"verify-structured-logging.sh" # TODO(dims) Need to get this running with golang 1.18
)
# Exclude generated-files-remake in certain cases, if they're running in a separate job.

View File

@ -11,6 +11,6 @@ require (
github.com/google/go-flow-levee v0.1.5
gotest.tools/gotestsum v1.6.4
honnef.co/go/tools v0.2.2
k8s.io/klog/hack/tools v0.0.0-20210917071902-331d2323a192
k8s.io/klog/hack/tools v0.0.0-20220321210246-c697110cd8ac
sigs.k8s.io/zeitgeist v0.2.0
)

View File

@ -1521,8 +1521,8 @@ honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
honnef.co/go/tools v0.2.2 h1:MNh1AVMyVX23VUHE2O27jm6lNj3vjO5DexS4A1xvnzk=
honnef.co/go/tools v0.2.2/go.mod h1:lPVVZ2BS5TfnjLyizF7o7hv7j9/L+8cZY2hLyjP9cGY=
k8s.io/klog/hack/tools v0.0.0-20210917071902-331d2323a192 h1:u27Xm1of9MTDM1CZW3hg0Vv04ohywEG152G8mpr9n8Y=
k8s.io/klog/hack/tools v0.0.0-20210917071902-331d2323a192/go.mod h1:DXW3Mv8xqJvjXWiBSBHrK2O4mq5LMD0clqkv3b1g9HA=
k8s.io/klog/hack/tools v0.0.0-20220321210246-c697110cd8ac h1:c2NQfDLtcwrPdD9pnUcRPlMJ719zgRzdlufHULIW7mU=
k8s.io/klog/hack/tools v0.0.0-20220321210246-c697110cd8ac/go.mod h1:DXW3Mv8xqJvjXWiBSBHrK2O4mq5LMD0clqkv3b1g9HA=
k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
mvdan.cc/gofumpt v0.3.0 h1:kTojdZo9AcEYbQYhGuLf/zszYthRdhDNDUi2JKTxas4=

View File

@ -36,14 +36,25 @@ PATH="${GOBIN}:${PATH}"
export GO111MODULE=on
# Install golangci-lint
echo 'installing golangci-lint '
echo "installing golangci-lint and logcheck plugin from hack/tools into ${GOBIN}"
pushd "${KUBE_ROOT}/hack/tools" >/dev/null
go install github.com/golangci/golangci-lint/cmd/golangci-lint
go build -o "${GOBIN}/logcheck.so" -buildmode=plugin k8s.io/klog/hack/tools/logcheck/plugin
popd >/dev/null
cd "${KUBE_ROOT}"
# The config is in ${KUBE_ROOT}/.golangci.yaml
# The config is in ${KUBE_ROOT}/.golangci.yaml where it will be found
# even when golangci-lint is invoked in a sub-directory.
#
# The logcheck plugin currently has to be configured via env variables
# (https://github.com/golangci/golangci-lint/issues/1512).
#
# Remember to clean the golangci-lint cache when changing
# the configuration and running this script multiple times,
# otherwise golangci-lint will report stale results:
# _output/local/bin/golangci-lint cache clean
export LOGCHECK_CONFIG="${KUBE_ROOT}/hack/logcheck.conf"
echo 'running golangci-lint ' >&2
res=0
if [[ "$#" -gt 0 ]]; then

View File

@ -1,97 +0,0 @@
#!/usr/bin/env bash
# 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.
# This script is used to avoid regressions after a package is migrated
# to structured logging. once a package is completely migrated add
# it .structured_logging file to avoid any future regressions.
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/util.sh"
kube::golang::verify_go_version
# Ensure that we find the binaries we build before anything else.
export GOBIN="${KUBE_OUTPUT_BINPATH}"
PATH="${GOBIN}:${PATH}"
# Install logcheck
pushd "${KUBE_ROOT}/hack/tools" >/dev/null
GO111MODULE=on go install k8s.io/klog/hack/tools/logcheck
popd >/dev/null
# We store packages that are migrated in .structured_logging
migrated_packages_file="${KUBE_ROOT}/hack/.structured_logging"
# Ensure that file is sorted.
kube::util::check-file-in-alphabetical-order "${migrated_packages_file}"
migrated_packages=()
while IFS='' read -r line; do
migrated_packages+=("$KUBE_ROOT/$line")
done < <(cat "${migrated_packages_file}")
echo "Running structured logging static check on migrated packages"
ret=0
GOOS=linux logcheck "${migrated_packages[@]}" || ret=$?
GOOS=windows logcheck "${migrated_packages[@]}" || ret=$?
if [ $ret -eq 0 ]; then
echo "Structured logging static check passed on migrated packages :)"
else
echo "Please fix above failures. You can locally test via:"
echo "hack/verify-structured-logging.sh"
fi
# Ignore migrated packages as they are already tested
# Trim the trailing /... from given packages
ignore_packages=$(grep -oE '^[a-zA-Z0-9/]+[^/\.]' < "${KUBE_ROOT}"/hack/.structured_logging)
all_packages=()
# shellcheck disable=SC2010
for i in $(ls -d ./*/ | grep -v 'staging' | grep -v 'vendor' | grep -v 'hack')
do
all_packages+=("$(go list ./"$i"/... 2> /dev/null | sed 's/k8s.io\/kubernetes\///g')")
all_packages+=(" ")
done
# We exclude vendor/ except vendor/k8s.io
# This is because vendor/k8s.io is symlinked to staging
# and needs to be checked
all_packages+=("vendor/k8s.io")
# Packages to test = all_packages - ignored_packages
packages=()
while IFS='' read -r line; do
if [ -z "$line" ]; then continue; fi
packages+=("$KUBE_ROOT/$line/...")
done < <(echo "${all_packages[@]}" | tr " " "\n" | grep -v "$ignore_packages")
echo -e "\nRunning structured logging static check on all other packages"
GOOS=linux logcheck -allow-unstructured "${packages[@]}" || ret=$?
GOOS=windows logcheck -allow-unstructured "${packages[@]}" || ret=$?
if [ $ret -eq 0 ]; then
echo "Structured logging static check passed on all packages :)"
else
echo "Please fix above failures. You can locally test via:"
echo "hack/verify-structured-logging.sh"
fi
exit $ret