refactor: improve color management in cli test (#5609)

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

cmd/cli/kubectl-kyverno/test/output.go

@@ -0,0 +1,40 @@
+package test
+
+import (
+	"os"
+
+	"github.com/fatih/color"
+	"github.com/kataras/tablewriter"
+	"github.com/lensesio/tableprinter"
+)
+
+var (
+	boldGreen  = color.New(color.FgGreen).Add(color.Bold)
+	boldRed    = color.New(color.FgRed).Add(color.Bold)
+	boldYellow = color.New(color.FgYellow).Add(color.Bold)
+	boldFgCyan = color.New(color.FgCyan).Add(color.Bold)
+)
+
+func colorize(noColor bool, color *color.Color, format string, a ...interface{}) string {
+	if noColor {
+		return format
+	}
+	return color.Sprintf(format, a...)
+}
+
+func newTablePrinter(noColor bool) *tableprinter.Printer {
+	printer := tableprinter.New(os.Stdout)
+	printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true
+	printer.CenterSeparator = "│"
+	printer.ColumnSeparator = "│"
+	printer.RowSeparator = "─"
+	printer.RowCharLimit = 300
+	printer.RowLengthTitle = func(rowsLength int) bool {
+		return rowsLength > 10
+	}
+	if !noColor {
+		printer.HeaderBgColor = tablewriter.BgBlackColor
+		printer.HeaderFgColor = tablewriter.FgGreenColor
+	}
+	return printer
+}

cmd/cli/kubectl-kyverno/test/test_command.go

@@ -13,10 +13,8 @@ import (
 	"strings"
 	"time"
 
-	"github.com/fatih/color"
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/memfs"
-	"github.com/kataras/tablewriter"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	"github.com/kyverno/kyverno/api/kyverno/v1beta1"
 	policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2"
@@ -32,7 +30,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/openapi"
 	policy2 "github.com/kyverno/kyverno/pkg/policy"
 	gitutils "github.com/kyverno/kyverno/pkg/utils/git"
-	"github.com/lensesio/tableprinter"
 	"github.com/spf13/cobra"
 	"golang.org/x/exp/slices"
 	corev1 "k8s.io/api/core/v1"
@@ -377,7 +374,7 @@ func testCommandExecute(dirPath []string, fileName string, gitBranch string, tes
 	fmt.Printf("\n")
 
 	if rc.Fail > 0 && !failOnly {
-		printFailedTestResult()
+		printFailedTestResult(removeColor)
 		os.Exit(1)
 	}
 	os.Exit(0)
@@ -951,12 +948,8 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, isGit bool,
 }
 
 func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, testResults []api.TestResults, rc *resultCounts, failOnly, removeColor bool) error {
-	printer := tableprinter.New(os.Stdout)
+	printer := newTablePrinter(removeColor)
 	table := []Table{}
-	boldGreen := color.New(color.FgGreen).Add(color.Bold)
-	boldRed := color.New(color.FgRed).Add(color.Bold)
-	boldYellow := color.New(color.FgYellow).Add(color.Bold)
-	boldFgCyan := color.New(color.FgCyan).Add(color.Bold)
 
 	var countDeprecatedResource int
 	testCount := 1
@@ -966,23 +959,14 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 		if v.Resources == nil {
 			testCount++
 		}
-		if !removeColor {
+		res.Policy = colorize(removeColor, boldFgCyan, v.Policy)
-			res.Policy = boldFgCyan.Sprintf(v.Policy)
+		res.Rule = colorize(removeColor, boldFgCyan, v.Rule)
-			res.Rule = boldFgCyan.Sprintf(v.Rule)
-		} else {
-			res.Policy = v.Policy
-			res.Rule = v.Rule
-		}
 
 		if v.Resources != nil {
 			for _, resource := range v.Resources {
 				res.ID = testCount
 				testCount++
-				if !removeColor {
+				res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, resource)
-					res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(resource)
-				} else {
-					res.Resource = v.Namespace + "/" + v.Kind + "/" + resource
-				}
 				var ruleNameInResultKey string
 				if v.AutoGeneratedRule != "" {
 					ruleNameInResultKey = fmt.Sprintf("%s-%s", v.AutoGeneratedRule, v.Rule)
@@ -998,19 +982,10 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 					resultKey = fmt.Sprintf("%s-%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, resource)
 				} else if found {
 					resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Kind, resource)
-					if !removeColor {
+					res.Policy = colorize(removeColor, boldFgCyan, ns) + "/" + colorize(removeColor, boldFgCyan, v.Policy)
-						res.Policy = boldFgCyan.Sprintf(ns) + "/" + boldFgCyan.Sprintf(v.Policy)
+					res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, resource)
-						res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(resource)
-					} else {
-						res.Policy = ns + "/" + v.Policy
-						res.Resource = v.Namespace + "/" + v.Kind + "/" + resource
-					}
 				} else if v.Namespace != "" {
-					if !removeColor {
+					res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, resource)
-						res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(resource)
-					} else {
-						res.Resource = v.Namespace + "/" + v.Kind + "/" + resource
-					}
 					resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, resource)
 				}
 
@@ -1019,11 +994,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 					testRes = val
 				} else {
 					log.Log.V(2).Info("result not found", "key", resultKey)
-					if !removeColor {
+					res.Result = colorize(removeColor, boldYellow, "Not found")
-						res.Result = boldYellow.Sprintf("Not found")
-					} else {
-						res.Result = "Not found"
-					}
 					rc.Fail++
 					table = append(table, *res)
 					ftable = append(ftable, *res)
@@ -1035,11 +1006,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 				}
 
 				if testRes.Result == v.Result {
-					if !removeColor {
+					res.Result = colorize(removeColor, boldGreen, "Pass")
-						res.Result = boldGreen.Sprintf("Pass")
-					} else {
-						res.Result = "Pass"
-					}
 					if testRes.Result == policyreportv1alpha2.StatusSkip {
 						rc.Skip++
 					} else {
@@ -1047,11 +1014,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 					}
 				} else {
 					log.Log.V(2).Info("result mismatch", "expected", v.Result, "received", testRes.Result, "key", resultKey)
-					if !removeColor {
+					res.Result = colorize(removeColor, boldRed, "Fail")
-						res.Result = boldRed.Sprintf("Fail")
-					} else {
-						res.Result = "Fail"
-					}
 					rc.Fail++
 					ftable = append(ftable, *res)
 				}
@@ -1066,11 +1029,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 			}
 		} else if v.Resource != "" {
 			countDeprecatedResource++
-			if !removeColor {
+			res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, v.Resource)
-				res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(v.Resource)
-			} else {
-				res.Resource = v.Namespace + "/" + v.Kind + "/" + v.Resource
-			}
 			var ruleNameInResultKey string
 			if v.AutoGeneratedRule != "" {
 				ruleNameInResultKey = fmt.Sprintf("%s-%s", v.AutoGeneratedRule, v.Rule)
@@ -1086,19 +1045,10 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 				resultKey = fmt.Sprintf("%s-%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, v.Resource)
 			} else if found {
 				resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Kind, v.Resource)
-				if !removeColor {
+				res.Policy = colorize(removeColor, boldFgCyan, ns) + "/" + colorize(removeColor, boldFgCyan, v.Policy)
-					res.Policy = boldFgCyan.Sprintf(ns) + "/" + boldFgCyan.Sprintf(v.Policy)
+				res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, v.Resource)
-					res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(v.Resource)
-				} else {
-					res.Policy = ns + "/" + v.Policy
-					res.Resource = v.Namespace + "/" + v.Kind + "/" + v.Resource
-				}
 			} else if v.Namespace != "" {
-				if !removeColor {
+				res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, v.Resource)
-					res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(v.Resource)
-				} else {
-					res.Resource = v.Namespace + "/" + v.Kind + "/" + v.Resource
-				}
 				resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, v.Resource)
 			}
 
@@ -1107,11 +1057,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 				testRes = val
 			} else {
 				log.Log.V(2).Info("result not found", "key", resultKey)
-				if !removeColor {
+				res.Result = colorize(removeColor, boldYellow, "Not found")
-					res.Result = boldYellow.Sprintf("Not found")
-				} else {
-					res.Result = "Not found"
-				}
 				rc.Fail++
 				table = append(table, *res)
 				ftable = append(ftable, *res)
@@ -1123,11 +1069,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 			}
 
 			if testRes.Result == v.Result {
-				if !removeColor {
+				res.Result = colorize(removeColor, boldGreen, "Pass")
-					res.Result = boldGreen.Sprintf("Pass")
-				} else {
-					res.Result = "Pass"
-				}
 				if testRes.Result == policyreportv1alpha2.StatusSkip {
 					rc.Skip++
 				} else {
@@ -1135,11 +1077,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 				}
 			} else {
 				log.Log.V(2).Info("result mismatch", "expected", v.Result, "received", testRes.Result, "key", resultKey)
-				if !removeColor {
+				res.Result = colorize(removeColor, boldRed, "Fail")
-					res.Result = boldRed.Sprintf("Fail")
-				} else {
-					res.Result = "Fail"
-				}
 				rc.Fail++
 				ftable = append(ftable, *res)
 			}
@@ -1153,41 +1091,17 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t
 			}
 		}
 	}
-
-	printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true
-	printer.CenterSeparator = "│"
-	printer.ColumnSeparator = "│"
-	printer.RowSeparator = "─"
-	printer.RowCharLimit = 300
-	printer.RowLengthTitle = func(rowsLength int) bool {
-		return rowsLength > 10
-	}
-	if !removeColor {
-		printer.HeaderBgColor = tablewriter.BgBlackColor
-		printer.HeaderFgColor = tablewriter.FgGreenColor
-	}
 	fmt.Printf("\n")
 	printer.Print(table)
 	return nil
 }
 
-func printFailedTestResult() {
+func printFailedTestResult(removeColor bool) {
-	printer := tableprinter.New(os.Stdout)
+	printer := newTablePrinter(removeColor)
 	for i, v := range ftable {
 		v.ID = i + 1
 	}
 	fmt.Printf("Aggregated Failed Test Cases : ")
-	printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true
-	printer.CenterSeparator = "│"
-	printer.ColumnSeparator = "│"
-	printer.RowSeparator = "─"
-	printer.RowCharLimit = 300
-	printer.RowLengthTitle = func(rowsLength int) bool {
-		return rowsLength > 10
-	}
-
-	printer.HeaderBgColor = tablewriter.BgBlackColor
-	printer.HeaderFgColor = tablewriter.FgGreenColor
 	fmt.Printf("\n")
 	printer.Print(ftable)
 }