Skip to content

Commit

Permalink
Fix logcheck exit function
Browse files Browse the repository at this point in the history
  • Loading branch information
luyou86 committed Sep 16, 2021
1 parent f8e668d commit aa5eec7
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 3 deletions.
11 changes: 8 additions & 3 deletions hack/tools/logcheck/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func checkForFunctionExpr(fexpr *ast.CallExpr, pass *analysis.Pass, c *config) {
if !isUnstructured((fName)) {
// if format specifier is used, check for arg length will most probably fail
// so check for format specifier first and skip if found
if checkForFormatSpecifier(fexpr, pass) {
if checkForFormatSpecifier(fexpr, pass, c) {
return
}
if fName == "InfoS" {
Expand Down Expand Up @@ -179,11 +179,16 @@ func isKeysValid(keyValues []ast.Expr, fun ast.Expr, pass *analysis.Pass, funNam
}
}

func checkForFormatSpecifier(expr *ast.CallExpr, pass *analysis.Pass) bool {
func checkForFormatSpecifier(expr *ast.CallExpr, pass *analysis.Pass, c *config) bool {
if selExpr, ok := expr.Fun.(*ast.SelectorExpr); ok {
// extracting function Name like Infof
fName := selExpr.Sel.Name
if specifier, found := hasFormatSpecifier(expr.Args); found {
specifier, found := hasFormatSpecifier(expr.Args)
if found {
if (fName == "Exit" || fName == "Exitf" || fName == "Exitln" || fName == "ExitDepth") && c.allowUnstructured {
return true
}

msg := fmt.Sprintf("structured logging function %q should not use format specifier %q", fName, specifier)
pass.Report(analysis.Diagnostic{
Pos: expr.Fun.Pos(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ func allowUnstructuredLogs() {
klog.ErrorS(nil, "Starting container in a pod", testKey, "containerID") // want `Key positional arguments are expected to be inlined constant strings. `
klog.InfoS("test: %s", "testname") // want `structured logging function "InfoS" should not use format specifier "%s"`
klog.ErrorS(nil, "test no.: %d", 1) // want `structured logging function "ErrorS" should not use format specifier "%d"`
klog.Exit("test log")
klog.Exit("test: %s", "log")
klog.ExitDepth(1, "test log")
klog.ExitDepth(1, "test: %s", "log")
klog.Exitln("test log")
klog.Exitln("test: %s", "log")
klog.Exitf("%s", "test log")

// Unstructured logs
// Error is not expected as this package allows unstructured logging
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ func doNotAllowUnstructuredLogs() {
klog.ErrorS(nil, "Starting container in a pod", testKey, "containerID") // want `Key positional arguments are expected to be inlined constant strings. `
klog.InfoS("test: %s", "testname") // want `structured logging function "InfoS" should not use format specifier "%s"`
klog.ErrorS(nil, "test no.: %d", 1) // want `structured logging function "ErrorS" should not use format specifier "%d"`
klog.Exit("test log")
klog.ExitDepth(1, "test log")
klog.Exitln("test log")
klog.Exitf("test log")

// Unstructured logs
// Error is expected as this package does not allow unstructured logging
Expand Down
17 changes: 17 additions & 0 deletions hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,20 @@ func Fatalln(args ...interface{}) {
// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
func Fatalf(format string, args ...interface{}) {
}

func Exit(args ...interface{}) {
}

// ExitDepth acts as Exit but uses depth to determine which call frame to log.
// ExitDepth(0, "msg") is the same as Exit("msg").
func ExitDepth(depth int, args ...interface{}) {
}

// Exitln logs to the FATAL, ERROR, WARNING, and INFO logs, then calls os.Exit(1).
func Exitln(args ...interface{}) {
}

// Exitf logs to the FATAL, ERROR, WARNING, and INFO logs, then calls os.Exit(1).
// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
func Exitf(format string, args ...interface{}) {
}

0 comments on commit aa5eec7

Please sign in to comment.