From 901efbc74c8928315382c5dfa490fabc982d2297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 14 Sep 2023 13:45:18 +0200 Subject: [PATCH] fix: cli output improvements (#8398) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .../kubectl-kyverno/commands/apply/command.go | 2 +- .../kubectl-kyverno/commands/apply/table.go | 6 +++-- .../kubectl-kyverno/commands/test/command.go | 1 + .../kubectl-kyverno/commands/test/output.go | 5 ++-- cmd/cli/kubectl-kyverno/commands/test/test.go | 8 +++--- .../kubectl-kyverno/output/table/printer.go | 6 ++--- .../output/table/printer_test.go | 3 ++- .../processor/policy_processor.go | 4 +-- cmd/cli/kubectl-kyverno/processor/result.go | 25 +++---------------- 9 files changed, 24 insertions(+), 36 deletions(-) diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index 10ba9564e7..8a8955b8c9 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -87,7 +87,7 @@ func Command() *cobra.Command { if applyCommandConfig.PolicyReport { printReport(out, responses, applyCommandConfig.AuditWarn) } else if table { - printTable(detailedResults, applyCommandConfig.AuditWarn, responses...) + printTable(out, detailedResults, applyCommandConfig.AuditWarn, responses...) } else { printViolations(out, rc) } diff --git a/cmd/cli/kubectl-kyverno/commands/apply/table.go b/cmd/cli/kubectl-kyverno/commands/apply/table.go index fa6b742c8f..168dc3091e 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/table.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/table.go @@ -1,13 +1,15 @@ package apply import ( + "io" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/color" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/table" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/policy/annotations" engineapi "github.com/kyverno/kyverno/pkg/engine/api" ) -func printTable(compact, auditWarn bool, engineResponses ...engineapi.EngineResponse) { +func printTable(out io.Writer, compact, auditWarn bool, engineResponses ...engineapi.EngineResponse) { var resultsTable table.Table id := 1 for _, engineResponse := range engineResponses { @@ -49,6 +51,6 @@ func printTable(compact, auditWarn bool, engineResponses ...engineapi.EngineResp resultsTable.Add(row) } } - printer := table.NewTablePrinter() + printer := table.NewTablePrinter(out) printer.Print(resultsTable.Rows(compact)) } diff --git a/cmd/cli/kubectl-kyverno/commands/test/command.go b/cmd/cli/kubectl-kyverno/commands/test/command.go index 26bcdc5a28..488fd47abb 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/command.go +++ b/cmd/cli/kubectl-kyverno/commands/test/command.go @@ -126,6 +126,7 @@ func testCommandExecute( if err != nil { return fmt.Errorf("failed to run test (%w)", err) } + fmt.Fprintln(out, " Checking results ...") t, err := printTestResult(out, filteredResults, responses, rc, failOnly, detailedResults, test.Fs, resourcePath) if err != nil { return fmt.Errorf("failed to print test result (%w)", err) diff --git a/cmd/cli/kubectl-kyverno/commands/test/output.go b/cmd/cli/kubectl-kyverno/commands/test/output.go index bb6c2496d2..c29c4baf6b 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/output.go +++ b/cmd/cli/kubectl-kyverno/commands/test/output.go @@ -22,7 +22,7 @@ func printTestResult( fs billy.Filesystem, resoucePath string, ) (table.Table, error) { - printer := table.NewTablePrinter() + printer := table.NewTablePrinter(out) var resultsTable table.Table var countDeprecatedResource int testCount := 1 @@ -99,11 +99,12 @@ func printTestResult( } fmt.Fprintln(out) printer.Print(resultsTable.Rows(detailedResults)) + fmt.Fprintln(out) return resultsTable, nil } func printFailedTestResult(out io.Writer, resultsTable table.Table, detailedResults bool) { - printer := table.NewTablePrinter() + printer := table.NewTablePrinter(out) for i := range resultsTable.RawRows { resultsTable.RawRows[i].ID = i + 1 } diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go index 73d8be33df..e069b09fa4 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/test.go +++ b/cmd/cli/kubectl-kyverno/commands/test/test.go @@ -78,6 +78,7 @@ func runTest(out io.Writer, openApiManager openapi.Manager, testCase test.TestCa if vars != nil { vars.SetInStore() } + fmt.Fprintln(out, " Applying", len(policies), pluralize.Pluralize(len(policies), "policy", "policies"), "to", len(uniques), pluralize.Pluralize(len(uniques), "resource", "resources"), "...") // TODO document the code below ruleToCloneSourceResource := map[string]string{} for _, policy := range policies { @@ -90,12 +91,12 @@ func runTest(out io.Writer, openApiManager openapi.Manager, testCase test.TestCa if rule.HasGenerate() { ruleUnstr, err := generate.GetUnstrRule(rule.Generation.DeepCopy()) if err != nil { - fmt.Fprintf(out, "Error: failed to get unstructured rule\nCause: %s\n", err) + fmt.Fprintf(out, " Error: failed to get unstructured rule (%s)\n", err) break } genClone, _, err := unstructured.NestedMap(ruleUnstr.Object, "clone") if err != nil { - fmt.Fprintf(out, "Error: failed to read data\nCause: %s\n", err) + fmt.Fprintf(out, " Error: failed to read data (%s)\n", err) break } if len(genClone) != 0 { @@ -128,14 +129,13 @@ func runTest(out io.Writer, openApiManager openapi.Manager, testCase test.TestCa if !vars.HasVariables() && variables.NeedsVariables(matches...) { // check policy in variable file if !vars.HasPolicyVariables(pol.GetName()) { - fmt.Fprintf(out, "test skipped for policy %v (as required variables are not provided by the users) \n \n", pol.GetName()) + fmt.Fprintln(out, " test skipped for policy", pol.GetName(), "(as required variables are not provided by the users)") // continue } } validPolicies = append(validPolicies, pol) } // execute engine - fmt.Fprintln(out, " Applying", len(policies), pluralize.Pluralize(len(policies), "policy", "policies"), "to", len(uniques), pluralize.Pluralize(len(uniques), "resource", "resources"), "...") var engineResponses []engineapi.EngineResponse var resultCounts processor.ResultCounts diff --git a/cmd/cli/kubectl-kyverno/output/table/printer.go b/cmd/cli/kubectl-kyverno/output/table/printer.go index c36964f638..ddb516aa7f 100644 --- a/cmd/cli/kubectl-kyverno/output/table/printer.go +++ b/cmd/cli/kubectl-kyverno/output/table/printer.go @@ -1,7 +1,7 @@ package table import ( - "os" + "io" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/color" "github.com/lensesio/tableprinter" @@ -11,8 +11,8 @@ func rowsLength(length int) bool { return length > 10 } -func NewTablePrinter() *tableprinter.Printer { - printer := tableprinter.New(os.Stdout) +func NewTablePrinter(out io.Writer) *tableprinter.Printer { + printer := tableprinter.New(out) printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true printer.CenterSeparator = "│" printer.ColumnSeparator = "│" diff --git a/cmd/cli/kubectl-kyverno/output/table/printer_test.go b/cmd/cli/kubectl-kyverno/output/table/printer_test.go index 0547640e89..2d74d7732c 100644 --- a/cmd/cli/kubectl-kyverno/output/table/printer_test.go +++ b/cmd/cli/kubectl-kyverno/output/table/printer_test.go @@ -1,11 +1,12 @@ package table import ( + "os" "testing" ) func TestNewTablePrinter(t *testing.T) { - if got := NewTablePrinter(); got == nil { + if got := NewTablePrinter(os.Stdout); got == nil { t.Errorf("NewTablePrinter() return nill") } } diff --git a/cmd/cli/kubectl-kyverno/processor/policy_processor.go b/cmd/cli/kubectl-kyverno/processor/policy_processor.go index ede0995d63..35b62dcae4 100644 --- a/cmd/cli/kubectl-kyverno/processor/policy_processor.go +++ b/cmd/cli/kubectl-kyverno/processor/policy_processor.go @@ -176,7 +176,7 @@ func (p *PolicyProcessor) ApplyPoliciesOnResource() ([]engineapi.EngineResponse, } responses = append(responses, generateResponse) } - p.Rc.addGenerateResponse(p.Out, p.AuditWarn, resPath, generateResponse) + p.Rc.addGenerateResponse(p.AuditWarn, resPath, generateResponse) } } p.Rc.addEngineResponses(p.AuditWarn, responses...) @@ -307,7 +307,7 @@ func (p *PolicyProcessor) makePolicyContext( } func (p *PolicyProcessor) processMutateEngineResponse(response engineapi.EngineResponse, resourcePath string) error { - printMutatedRes := p.Rc.addMutateResponse(p.Out, resourcePath, response) + printMutatedRes := p.Rc.addMutateResponse(resourcePath, response) if printMutatedRes && p.PrintPatchResource { yamlEncodedResource, err := yamlv2.Marshal(response.PatchedResource.Object) if err != nil { diff --git a/cmd/cli/kubectl-kyverno/processor/result.go b/cmd/cli/kubectl-kyverno/processor/result.go index fb6ad12db5..c49d6c2667 100644 --- a/cmd/cli/kubectl-kyverno/processor/result.go +++ b/cmd/cli/kubectl-kyverno/processor/result.go @@ -1,9 +1,6 @@ package processor import ( - "fmt" - "io" - kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/policy/annotations" "github.com/kyverno/kyverno/pkg/autogen" @@ -75,26 +72,20 @@ func (rc *ResultCounts) addEngineResponse(auditWarn bool, response engineapi.Eng } } -func (rc *ResultCounts) addGenerateResponse(out io.Writer, auditWarn bool, resPath string, response engineapi.EngineResponse) { +func (rc *ResultCounts) addGenerateResponse(auditWarn bool, resPath string, response engineapi.EngineResponse) { genericPolicy := response.Policy() if polType := genericPolicy.GetType(); polType == engineapi.ValidatingAdmissionPolicyType { return } policy := genericPolicy.GetPolicy().(kyvernov1.PolicyInterface) - printCount := 0 for _, policyRule := range autogen.ComputeRules(policy) { ruleFoundInEngineResponse := false - for i, ruleResponse := range response.PolicyResponse.Rules { + for _, ruleResponse := range response.PolicyResponse.Rules { if policyRule.Name == ruleResponse.Name() { ruleFoundInEngineResponse = true if ruleResponse.Status() == engineapi.RuleStatusPass { rc.pass++ } else { - if printCount < 1 { - fmt.Fprintln(out, "\ninvalid resource", "policy", policy.GetName(), "resource", resPath) - printCount++ - } - fmt.Fprintf(out, "%d. %s - %s\n", i+1, ruleResponse.Name(), ruleResponse.Message()) if auditWarn && response.GetValidationFailureAction().Audit() { rc.warn++ } else { @@ -110,7 +101,7 @@ func (rc *ResultCounts) addGenerateResponse(out io.Writer, auditWarn bool, resPa } } -func (rc *ResultCounts) addMutateResponse(out io.Writer, resourcePath string, response engineapi.EngineResponse) bool { +func (rc *ResultCounts) addMutateResponse(resourcePath string, response engineapi.EngineResponse) bool { genericPolicy := response.Policy() if polType := genericPolicy.GetType(); polType == engineapi.ValidatingAdmissionPolicyType { return false @@ -125,28 +116,20 @@ func (rc *ResultCounts) addMutateResponse(out io.Writer, resourcePath string, re if !policyHasMutate { return false } - printCount := 0 printMutatedRes := false for _, policyRule := range autogen.ComputeRules(policy) { ruleFoundInEngineResponse := false - for i, mutateResponseRule := range response.PolicyResponse.Rules { + for _, mutateResponseRule := range response.PolicyResponse.Rules { if policyRule.Name == mutateResponseRule.Name() { ruleFoundInEngineResponse = true if mutateResponseRule.Status() == engineapi.RuleStatusPass { rc.pass++ printMutatedRes = true } else if mutateResponseRule.Status() == engineapi.RuleStatusSkip { - fmt.Fprintf(out, "\nskipped mutate policy %s -> resource %s", policy.GetName(), resourcePath) rc.skip++ } else if mutateResponseRule.Status() == engineapi.RuleStatusError { - fmt.Fprintf(out, "\nerror while applying mutate policy %s -> resource %s\nerror: %s", policy.GetName(), resourcePath, mutateResponseRule.Message()) rc.err++ } else { - if printCount < 1 { - fmt.Fprintf(out, "\nfailed to apply mutate policy %s -> resource %s", policy.GetName(), resourcePath) - printCount++ - } - fmt.Fprintf(out, "%d. %s - %s \n", i+1, mutateResponseRule.Name(), mutateResponseRule.Message()) rc.fail++ } continue