From 4e4135c3dd8a0cea66d90503cebe53858a07ab12 Mon Sep 17 00:00:00 2001 From: "yingqi.ge" Date: Wed, 17 Mar 2021 12:12:28 +0800 Subject: [PATCH] Add check for InfoS & ErrorS parameters --- hack/tools/go.mod | 5 ++- hack/tools/go.sum | 22 +++++++++++- hack/tools/logcheck/main.go | 50 ++++++++++++++++++++++++++-- hack/tools/logcheck/testdata/data.go | 6 ++++ 4 files changed, 79 insertions(+), 4 deletions(-) diff --git a/hack/tools/go.mod b/hack/tools/go.mod index 23cf684c..3c94929c 100644 --- a/hack/tools/go.mod +++ b/hack/tools/go.mod @@ -2,4 +2,7 @@ module k8s.io/klog/hack/tools go 1.15 -require golang.org/x/tools v0.1.0 +require ( + golang.org/x/exp v0.0.0-20210220032938-85be41e4509f + golang.org/x/tools v0.1.0 +) diff --git a/hack/tools/go.sum b/hack/tools/go.sum index 21d696a6..e89e59cf 100644 --- a/hack/tools/go.sum +++ b/hack/tools/go.sum @@ -1,9 +1,25 @@ +dmitri.shuralyov.com/gpu/mtl v0.0.0-20201218220906-28db891af037/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= +github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= +github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= +golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56/go.mod h1:JhuoJpWY28nO4Vef9tZUw9qufEGTyX1+7lmHxV5q5G4= +golang.org/x/exp v0.0.0-20210220032938-85be41e4509f h1:GrkO5AtFUU9U/1f5ctbIBXtBGeSJbWwIYfIsTcFMaX4= +golang.org/x/exp v0.0.0-20210220032938-85be41e4509f/go.mod h1:I6l2HNBLBZEcrOoCpyKLdY2lHoRZ8lI4x60KMCQDft4= +golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= +golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE= +golang.org/x/mobile v0.0.0-20201217150744-e6ae53a27f4f/go.mod h1:skQtrUTUwhdJvXM/2KKJzY8pDgNr9I/FOMqDVRPBUS4= +golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY= +golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= +golang.org/x/mod v0.1.1-0.20191209134235-331c550502dd/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449 h1:xUIPaMhvROX9dhPvRCenIJtU78+lbEenGbgqB5hfHCQ= +golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +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-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= @@ -11,13 +27,17 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= 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-20190312151545-0bb0c0a6e846/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20200117012304-6edc0a871e69/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/hack/tools/logcheck/main.go b/hack/tools/logcheck/main.go index 496c5d67..c549d7ff 100644 --- a/hack/tools/logcheck/main.go +++ b/hack/tools/logcheck/main.go @@ -19,7 +19,9 @@ package main import ( "fmt" "go/ast" + "go/token" + "golang.org/x/exp/utf8string" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/singlechecker" ) @@ -48,7 +50,7 @@ func run(pass *analysis.Pass) (interface{}, error) { // passing all function calls to checkForFunctionExpr if fexpr, ok := n.(*ast.CallExpr); ok { - checkForFunctionExpr(fexpr.Fun, pass) + checkForFunctionExpr(fexpr.Fun, fexpr.Args, pass) } return true @@ -58,7 +60,7 @@ func run(pass *analysis.Pass) (interface{}, error) { } // checkForFunctionExpr checks for unstructured logging function, prints error if found any. -func checkForFunctionExpr(fun ast.Expr, pass *analysis.Pass) { +func checkForFunctionExpr(fun ast.Expr, args []ast.Expr, pass *analysis.Pass) { /* we are extracting external package function calls e.g. klog.Infof fmt.Printf and eliminating calls like setLocalHost() @@ -80,6 +82,13 @@ func checkForFunctionExpr(fun ast.Expr, pass *analysis.Pass) { // extracting package name pName, ok := selExpr.X.(*ast.Ident) + if ok && pName.Name == "klog" && !isUnstructured((fName)) { + if fName == "InfoS" { + isKeysValid(args[1:], fun, pass, fName) + } else if fName == "ErrorS" { + isKeysValid(args[2:], fun, pass, fName) + } + } // Matching if package name is klog and any unstructured logging function is used. if ok && pName.Name == "klog" && isUnstructured((fName)) { @@ -110,3 +119,40 @@ func isUnstructured(fName string) bool { return false } + +// isKeysValid check if all keys in keyAndValues is string type +func isKeysValid(keyValues []ast.Expr, fun ast.Expr, pass *analysis.Pass, funName string) { + if len(keyValues)%2 != 0 { + pass.Report(analysis.Diagnostic{ + Pos: fun.Pos(), + Message: fmt.Sprintf("Invalid Number of arguments for %s", funName), + }) + } + + for index, arg := range keyValues { + if index%2 != 0 { + continue + } + if lit, ok := arg.(*ast.BasicLit); ok { + if lit.Kind != token.STRING { + pass.Report(analysis.Diagnostic{ + Pos: fun.Pos(), + Message: fmt.Sprintf("Invalid value type for key %v", lit.Value), + }) + continue + } + isASCII := utf8string.NewString(lit.Value).IsASCII() + if !isASCII { + pass.Report(analysis.Diagnostic{ + Pos: fun.Pos(), + Message: fmt.Sprintf("Invalid value for key %v, it's must be ascii", lit.Value), + }) + } + } else { + pass.Report(analysis.Diagnostic{ + Pos: fun.Pos(), + Message: "Invalid value type for key", + }) + } + } +} diff --git a/hack/tools/logcheck/testdata/data.go b/hack/tools/logcheck/testdata/data.go index 1409d415..ec48dd8c 100644 --- a/hack/tools/logcheck/testdata/data.go +++ b/hack/tools/logcheck/testdata/data.go @@ -44,4 +44,10 @@ func printUnstructuredLog() { func printStructuredLog() { klog.InfoS("test log") klog.ErrorS(nil, "test log") + klog.InfoS("Starting container in a pod", "containerID", "containerID", "pod") // want `Invalid Number of arguments for InfoS` + klog.ErrorS(nil, "Starting container in a pod", "containerID", "containerID", "pod") // want `Invalid Number of arguments for ErrorS` + klog.InfoS("Starting container in a pod", "测试", "containerID") // want `Invalid value for key` + klog.ErrorS(nil, "Starting container in a pod", "测试", "containerID") // want `Invalid value for key` + klog.InfoS("Starting container in a pod", 7, "containerID") // want `Invalid value type for key 7` + klog.ErrorS(nil, "Starting container in a pod", 7, "containerID") // want `Invalid value type for key 7` }