Skip to content

Write a custom lint rule to disable use of logrus package #6357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"os"
"strings"

"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"k8s.io/kubectl/pkg/util/templates"
Expand Down Expand Up @@ -198,7 +197,7 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
rootCmd.AddCommand(NewCmdInspect())

templates.ActsAsRootCommand(rootCmd, nil, groups...)
rootCmd.PersistentFlags().StringVarP(&v, "verbosity", "v", constants.DefaultLogLevel.String(), fmt.Sprintf("Log level: one of %v", logrus.AllLevels))
rootCmd.PersistentFlags().StringVarP(&v, "verbosity", "v", log.DefaultLogLevel.String(), fmt.Sprintf("Log level: one of %v", log.AllLevels))
rootCmd.PersistentFlags().IntVar(&defaultColor, "color", int(output.DefaultColorCode), "Specify the default output color in ANSI escape codes")
rootCmd.PersistentFlags().BoolVar(&forceColors, "force-colors", false, "Always print color codes (hidden)")
rootCmd.PersistentFlags().BoolVar(&interactive, "interactive", true, "Allow user prompts for more information")
Expand Down Expand Up @@ -254,17 +253,7 @@ func FlagToEnvVarName(f *pflag.Flag) string {
}

func setUpLogs(stdErr io.Writer, level string, timestamp bool) error {
logrus.SetOutput(stdErr)
lvl, err := logrus.ParseLevel(level)
if err != nil {
return fmt.Errorf("parsing log level: %w", err)
}
logrus.SetLevel(lvl)
logrus.SetFormatter(&logrus.TextFormatter{
FullTimestamp: timestamp,
})
logrus.AddHook(event.NewLogHook())
return nil
return log.SetupLogs(stdErr, level, timestamp, event.NewLogHook())
}

// alwaysSucceedWhenCancelled returns nil if the context was cancelled.
Expand Down
15 changes: 15 additions & 0 deletions hack/golangci-lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,18 @@ fi

${BIN}/golangci-lint run ${FLAGS} --exclude=SA1019 -c ${DIR}/golangci.yml \
| awk '/out of memory/ || /Timeout exceeded/ {failed = 1}; {print}; END {exit failed}'


# Install and run custom linter to detect usage for logrus package.
# Currently, we can't run private custom linter in golangcl-lint due to abandoned issue
# https://github.com/golangci/golangci-lint/issues/1276
if ! [ -x "$(command -v ${BIN}/logrus-analyzer)" ] ; then
echo >&2 'Installing custom logrus linter'
cd "${DIR}/tools"
GO111MODULE=on go build -o ${BIN}/logrus-analyzer logrus_analyzer.go
cd -
fi
# This analyzer doesn't support any flags currently, so we don't include ${FLAGS}
${BIN}/logrus-analyzer github.com/GoogleContainerTools/skaffold{/pkg,/cmd,/diag}...


1 change: 1 addition & 0 deletions hack/tools/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ go 1.14
require (
github.com/corneliusweig/release-notes v0.0.0-20191014214505-0be5c7c66752
github.com/rakyll/statik v0.1.6
golang.org/x/tools v0.1.5
)
32 changes: 27 additions & 5 deletions hack/tools/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANyt
github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk=
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
Expand All @@ -46,24 +45,47 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 h1:4nGaVu0QrbjT/AK2PRLuQfQuh6DJve+pELhqTdAj3x0=
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6 h1:bjcUS9ztw9kFmmIxJInhon/0Is3p+EHBKNgquIzo1OI=
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190422165155-953cdadca894 h1:Cz4ceDQGXuKRnVBDTS23GTn/pU5OE2C0WrNTOYK1Uuc=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=
google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
Expand Down
71 changes: 71 additions & 0 deletions hack/tools/linters/logrus.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Copyright 2021 The Skaffold 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 linters

import (
"go/ast"
"strings"

"golang.org/x/tools/go/analysis"
)

// Ignore files
var ignoreFileSuffixes = []string{
// always ignore test files
"_test.go",
"pkg/skaffold/output/log/log.go",
"pkg/skaffold/event/v2/logger.go",
"pkg/skaffold/build/buildpacks/logger.go",
}
var LogrusAnalyzer = &analysis.Analyzer{
Name: "logruslinter",
Doc: "find usage of logrus",
Run: run,
}

func run(pass *analysis.Pass) (interface{}, error) {
for _, file := range pass.Files {
pos := pass.Fset.PositionFor(file.Pos(), false)
// Ignore logrus usage in test files
if ignore(pos.Filename) {
continue
}
ast.Inspect(file, func(n ast.Node) bool {
if importSpec, ok := n.(*ast.ImportSpec); ok {
if importSpec.Path != nil && strings.Contains(importSpec.Path.Value, "github.com/sirupsen/logrus") {
pass.Report(analysis.Diagnostic{
Pos: importSpec.Pos(),
End: 0,
Category: "logrus-analyzer",
Message: "Do not use github.com/sirupsen/logrus package, use output.log.Entry instead.",
})
}
}
return true
})
}
return nil, nil
}

func ignore(f string) bool {
for _, v := range ignoreFileSuffixes {
if strings.HasSuffix(f, v) {
return true
}
}
return false
}
27 changes: 27 additions & 0 deletions hack/tools/logrus_analyzer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
Copyright 2021 The Skaffold 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 (
"golang.org/x/tools/go/analysis/singlechecker"
"tools/linters"
)

func main() {
// TODO:(marlon) to add allowList of files which can import logrus.
singlechecker.Main(linters.LogrusAnalyzer)
}
13 changes: 0 additions & 13 deletions pkg/skaffold/build/cluster/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,13 @@ import (
"sync/atomic"
"time"

"github.com/sirupsen/logrus"
v1 "k8s.io/api/core/v1"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/kaniko"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
)

// logLevel makes sure kaniko logs at least at Info level and at most Debug level (trace doesn't work with Kaniko)
func logLevel() logrus.Level {
level := logrus.GetLevel()
if level < logrus.InfoLevel {
return logrus.InfoLevel
}
if level > logrus.DebugLevel {
return logrus.DebugLevel
}
return level
}

func streamLogs(ctx context.Context, out io.Writer, name string, pods corev1.PodInterface) func() {
var wg sync.WaitGroup
wg.Add(1)
Expand Down
48 changes: 0 additions & 48 deletions pkg/skaffold/build/cluster/logs_test.go

This file was deleted.

5 changes: 0 additions & 5 deletions pkg/skaffold/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package constants

import (
"github.com/sirupsen/logrus"

latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
)

Expand All @@ -36,9 +34,6 @@ const (
DevInit = Phase("DevInit")
Cleanup = Phase("Cleanup")

// DefaultLogLevel is the default global verbosity
DefaultLogLevel = logrus.WarnLevel

// DefaultDockerfilePath is the dockerfile path is given relative to the
// context directory
DefaultDockerfilePath = "Dockerfile"
Expand Down
3 changes: 1 addition & 2 deletions pkg/skaffold/kubernetes/debugging/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"strings"
"time"

"github.com/sirupsen/logrus"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -80,7 +79,7 @@ func applyDebuggingTransforms(l manifest.ManifestList, retriever debug.Configura
if err != nil {
return nil, fmt.Errorf("marshalling yaml: %w", err)
}
if logrus.IsLevelEnabled(logrus.DebugLevel) {
if log.IsDebugLevelEnabled() {
log.Entry(context.Background()).Debugln("Applied debugging transform:\n", string(manifest))
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/skaffold/kubernetes/portforward/kubectl_forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"sync/atomic"
"time"

"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -278,7 +277,7 @@ func findNewestPodForService(ctx context.Context, kubeContext, ns, serviceName s
}
sort.Slice(pods, newestPodsFirst(pods))

if logrus.IsLevelEnabled((logrus.TraceLevel)) {
if log.IsTraceLevelEnabled() {
var names []string
for _, p := range pods {
names = append(names, fmt.Sprintf("(pod:%q phase:%v created:%v)", p.Name, p.Status.Phase, p.CreationTimestamp))
Expand Down
Loading