From 59089682a954bbaa56b56a825e928c62b11125aa Mon Sep 17 00:00:00 2001 From: Felipe Amaral Date: Tue, 1 Oct 2024 12:42:12 +0100 Subject: [PATCH] feat: add labels flag as optional --- README.md | 2 +- cmd/kubent/main.go | 6 +- cmd/kubent/main_test.go | 15 ++-- fixtures/expected-json-output.json | 3 +- pkg/collector/file_test.go | 2 +- pkg/config/config.go | 29 ++++++- pkg/config/config_test.go | 6 +- pkg/judge/judge.go | 1 + pkg/judge/rego.go | 7 ++ pkg/judge/rego_test.go | 2 +- pkg/printer/csv.go | 23 +++-- pkg/printer/csv_test.go | 7 +- pkg/printer/json.go | 3 +- pkg/printer/json_test.go | 16 +++- pkg/printer/printer.go | 3 +- pkg/printer/printer_helper.go | 17 ++++ pkg/printer/printer_helper_test.go | 127 ++++++++++++++++++++++++++++ pkg/printer/text.go | 16 +++- pkg/printer/text_test.go | 7 +- pkg/rules/rego/deprecated-1-26.rego | 1 + pkg/rules/rules_test.go | 2 +- test/rules_custom_test.go | 2 +- 22 files changed, 258 insertions(+), 39 deletions(-) create mode 100644 pkg/printer/printer_helper.go create mode 100644 pkg/printer/printer_helper_test.go diff --git a/README.md b/README.md index d3a721a3..b0a78f7d 100644 --- a/README.md +++ b/README.md @@ -223,7 +223,7 @@ Otherwise there's `Makefile` ```sh $ make make -all Cean, build and pack +all Clean, build and pack help Prints list of tasks build Build binary generate Go generate diff --git a/cmd/kubent/main.go b/cmd/kubent/main.go index fc54bc0c..c269558a 100644 --- a/cmd/kubent/main.go +++ b/cmd/kubent/main.go @@ -156,7 +156,7 @@ func main() { log.Fatal().Err(err).Str("name", "Rego").Msg("Failed to filter results") } - err = outputResults(results, config.Output, config.OutputFile) + err = outputResults(results, config.Output, config.OutputFile, *config.OptionalFlags) if err != nil { log.Fatal().Err(err).Msgf("Failed to output results") } @@ -180,14 +180,14 @@ func configureGlobalLogging() { log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr}) } -func outputResults(results []judge.Result, outputType string, outputFile string) error { +func outputResults(results []judge.Result, outputType string, outputFile string, optionalFlags config.OptionalFlags) error { printer, err := printer.NewPrinter(outputType, outputFile) if err != nil { return fmt.Errorf("failed to create printer: %v", err) } defer printer.Close() - err = printer.Print(results) + err = printer.Print(results, optionalFlags) if err != nil { return fmt.Errorf("failed to print results: %v", err) } diff --git a/cmd/kubent/main_test.go b/cmd/kubent/main_test.go index fe81a8de..42630277 100644 --- a/cmd/kubent/main_test.go +++ b/cmd/kubent/main_test.go @@ -272,23 +272,24 @@ func Test_outputResults(t *testing.T) { ApiVersion: "1.2.3", RuleSet: "rs", ReplaceWith: "rep", Since: testVersion}} type args struct { - results []judge.Result - outputType string - outputFile string + results []judge.Result + outputType string + outputFile string + optionalFlags config.OptionalFlags } tests := []struct { name string args args wantErr bool }{ - {"good", args{testResults, "text", "-"}, false}, - {"bad-new-printer-type", args{testResults, "unknown", "-"}, true}, - {"bad-new-printer-file", args{testResults, "text", "/unlikely/to/exist/dir"}, true}, + {"good", args{testResults, "text", "-", config.OptionalFlags{Labels: false}}, false}, + {"bad-new-printer-type", args{testResults, "unknown", "-", config.OptionalFlags{Labels: false}}, true}, + {"bad-new-printer-file", args{testResults, "text", "/unlikely/to/exist/dir", config.OptionalFlags{Labels: false}}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile); (err != nil) != tt.wantErr { + if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile, tt.args.optionalFlags); (err != nil) != tt.wantErr { t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr) } }) diff --git a/fixtures/expected-json-output.json b/fixtures/expected-json-output.json index ffbfbae7..650b9692 100644 --- a/fixtures/expected-json-output.json +++ b/fixtures/expected-json-output.json @@ -6,6 +6,7 @@ "ApiVersion": "apps/v1beta1", "RuleSet": "Deprecated APIs removed in 1.16", "ReplaceWith": "apps/v1", - "Since": "1.9.0" + "Since": "1.9.0", + "Labels": {} } ] diff --git a/pkg/collector/file_test.go b/pkg/collector/file_test.go index bec0d265..310b218f 100644 --- a/pkg/collector/file_test.go +++ b/pkg/collector/file_test.go @@ -52,7 +52,7 @@ func TestFileCollectorGet(t *testing.T) { t.Errorf("Expected to get %d, got %d", len(tc.expected), len(manifests)) } - for i, _ := range manifests { + for i := range manifests { if manifests[i]["kind"] != tc.expected[i] { t.Errorf("Expected to get %s, instead got: %s", tc.expected[i], manifests[i]["kind"]) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 1608e96c..2a8701d6 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -10,13 +10,18 @@ import ( "unicode" "github.com/doitintl/kube-no-trouble/pkg/judge" - "github.com/doitintl/kube-no-trouble/pkg/printer" "k8s.io/client-go/tools/clientcmd" "github.com/rs/zerolog" flag "github.com/spf13/pflag" ) +const ( + JSON = "json" + TEXT = "text" + CSV = "csv" +) + type Config struct { AdditionalKinds []string AdditionalAnnotations []string @@ -31,12 +36,18 @@ type Config struct { OutputFile string TargetVersion *judge.Version KubentVersion bool + OptionalFlags *OptionalFlags +} + +type OptionalFlags struct { + Labels bool } func NewFromFlags() (*Config, error) { config := Config{ LogLevel: ZeroLogLevel(zerolog.InfoLevel), TargetVersion: &judge.Version{}, + OptionalFlags: &OptionalFlags{}, } flag.StringSliceVarP(&config.AdditionalKinds, "additional-kind", "a", []string{}, "additional kinds of resources to report in Kind.version.group.com format") @@ -52,11 +63,12 @@ func NewFromFlags() (*Config, error) { flag.StringVarP(&config.OutputFile, "output-file", "O", "-", "output file, use - for stdout") flag.VarP(&config.LogLevel, "log-level", "l", "set log level (trace, debug, info, warn, error, fatal, panic, disabled)") flag.VarP(config.TargetVersion, "target-version", "t", "target K8s version in SemVer format (autodetected by default)") + flag.BoolVar(&config.OptionalFlags.Labels, "labels", false, "print resource labels") flag.Parse() - if _, err := printer.ParsePrinter(config.Output); err != nil { - return nil, fmt.Errorf("failed to validate argument output: %w", err) + if !isValidOutputFormat(config.Output) { + return nil, fmt.Errorf("failed to validate argument output: %s", config.Output) } if err := validateOutputFile(config.OutputFile); err != nil { @@ -77,6 +89,17 @@ func NewFromFlags() (*Config, error) { return &config, nil } +// Previuosly this was handled by a printer.go ParsePrinter function +// but we need to avoid cycle imports in order to inject the additional flags +func isValidOutputFormat(format string) bool { + switch format { + case JSON, TEXT, CSV: + return true + default: + return false + } +} + // validateAdditionalResources check that all resources are provided in full form // resource.version.group.com. E.g. managedcertificate.v1beta1.networking.gke.io func validateAdditionalResources(resources []string) error { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 654bac03..cd6e0089 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -72,9 +72,9 @@ func TestValidateAdditionalResources(t *testing.T) { func TestValidateAdditionalResourcesFail(t *testing.T) { testCases := [][]string{ - []string{"abcdef"}, - []string{""}, - []string{"test.v1.com"}, + {"abcdef"}, + {""}, + {"test.v1.com"}, } for _, tc := range testCases { diff --git a/pkg/judge/judge.go b/pkg/judge/judge.go index 6cdbe035..8f018c47 100644 --- a/pkg/judge/judge.go +++ b/pkg/judge/judge.go @@ -8,6 +8,7 @@ type Result struct { RuleSet string ReplaceWith string Since *Version + Labels map[string]interface{} } type Judge interface { diff --git a/pkg/judge/rego.go b/pkg/judge/rego.go index 7baa52eb..0edb2d92 100644 --- a/pkg/judge/rego.go +++ b/pkg/judge/rego.go @@ -63,6 +63,12 @@ func (j *RegoJudge) Eval(input []map[string]interface{}) ([]Result, error) { m["Namespace"] = "" } + var labels map[string]interface{} + if v, ok := m["Labels"].(map[string]interface{}); ok { + labels = v + } else { + labels = make(map[string]interface{}) + } results = append(results, Result{ Name: m["Name"].(string), Namespace: m["Namespace"].(string), @@ -71,6 +77,7 @@ func (j *RegoJudge) Eval(input []map[string]interface{}) ([]Result, error) { ReplaceWith: m["ReplaceWith"].(string), RuleSet: m["RuleSet"].(string), Since: since, + Labels: labels, }) } } diff --git a/pkg/judge/rego_test.go b/pkg/judge/rego_test.go index bc29b70d..0dd58982 100644 --- a/pkg/judge/rego_test.go +++ b/pkg/judge/rego_test.go @@ -90,7 +90,7 @@ func TestEvalRules(t *testing.T) { t.Errorf("expected %d findings, instead got: %d", len(tc.expected), len(results)) } - for i, _ := range results { + for i := range results { if results[i].Kind != tc.expected[i] { t.Errorf("expected to get %s finding, instead got: %s", tc.expected[i], results[i].Kind) } diff --git a/pkg/printer/csv.go b/pkg/printer/csv.go index 3ea7d334..932024a7 100644 --- a/pkg/printer/csv.go +++ b/pkg/printer/csv.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -29,7 +30,7 @@ func (c *csvPrinter) Close() error { } // Print will print results in CSV format -func (c *csvPrinter) Print(results []judge.Result) error { +func (c *csvPrinter) Print(results []judge.Result, optionalFlags config.OptionalFlags) error { sort.Slice(results, func(i, j int) bool { return results[i].Name < results[j].Name @@ -46,7 +47,7 @@ func (c *csvPrinter) Print(results []judge.Result) error { w := csv.NewWriter(c.commonPrinter.outputFile) - w.Write([]string{ + fields := []string{ "api_version", "kind", "namespace", @@ -54,10 +55,16 @@ func (c *csvPrinter) Print(results []judge.Result) error { "replace_with", "since", "rule_set", - }) + } + + if optionalFlags.Labels { + fields = append(fields, "labels") + } + + w.Write(fields) for _, r := range results { - w.Write([]string{ + row := []string{ r.ApiVersion, r.Kind, r.Namespace, @@ -65,7 +72,13 @@ func (c *csvPrinter) Print(results []judge.Result) error { r.ReplaceWith, r.Since.String(), r.RuleSet, - }) + } + + if optionalFlags.Labels { + row = append(row, mapToCommaSeparatedString(r.Labels)) + } + + w.Write(row) } w.Flush() diff --git a/pkg/printer/csv_test.go b/pkg/printer/csv_test.go index c0552bc4..1634427a 100644 --- a/pkg/printer/csv_test.go +++ b/pkg/printer/csv_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -51,9 +52,11 @@ func TestCSVPrinterPrint(t *testing.T) { } version, _ := judge.NewVersion("1.2.3") - results := []judge.Result{{"Name", "Namespace", "Kind", "1.2.3", "Test", "4.5.6", version}} + labels := map[string]interface{}{"key1": "value1"} - if err := tp.Print(results); err != nil { + results := []judge.Result{{Name: "Name", Namespace: "Namespace", Kind: "Kind", ApiVersion: "1.2.3", RuleSet: "Test", ReplaceWith: "4.5.6", Since: version, Labels: labels}} + + if err := tp.Print(results, config.OptionalFlags{Labels: true}); err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/printer/json.go b/pkg/printer/json.go index e51756d7..bb178dbe 100644 --- a/pkg/printer/json.go +++ b/pkg/printer/json.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -29,7 +30,7 @@ func (c *jsonPrinter) Close() error { } // Print will print results in text format -func (c *jsonPrinter) Print(results []judge.Result) error { +func (c *jsonPrinter) Print(results []judge.Result, _ config.OptionalFlags) error { writer := bufio.NewWriter(c.commonPrinter.outputFile) defer writer.Flush() diff --git a/pkg/printer/json_test.go b/pkg/printer/json_test.go index 9a2f1a58..47e282ee 100644 --- a/pkg/printer/json_test.go +++ b/pkg/printer/json_test.go @@ -7,6 +7,7 @@ import ( "reflect" "testing" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -40,7 +41,6 @@ func Test_newJSONPrinter(t *testing.T) { }) } } - func Test_jsonPrinter_Print(t *testing.T) { tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) if err != nil { @@ -53,9 +53,19 @@ func Test_jsonPrinter_Print(t *testing.T) { } version, _ := judge.NewVersion("1.2.3") - results := []judge.Result{{"Name", "Namespace", "Kind", "1.2.3", "Test", "4.5.6", version}} + labels := map[string]interface{}{"key1": "value1"} - if err := c.Print(results); err != nil { + results := []judge.Result{{ + Name: "Name", + Namespace: "Namespace", + Kind: "Kind", + ApiVersion: "1.2.3", + RuleSet: "Test", + ReplaceWith: "4.5.6", + Since: version, + Labels: labels, + }} + if err := c.Print(results, config.OptionalFlags{Labels: true}); err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/printer/printer.go b/pkg/printer/printer.go index 31ab2e58..edd870e1 100644 --- a/pkg/printer/printer.go +++ b/pkg/printer/printer.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -14,7 +15,7 @@ var printers = map[string]func(string) (Printer, error){ } type Printer interface { - Print([]judge.Result) error + Print([]judge.Result, config.OptionalFlags) error Close() error } diff --git a/pkg/printer/printer_helper.go b/pkg/printer/printer_helper.go new file mode 100644 index 00000000..051e8d87 --- /dev/null +++ b/pkg/printer/printer_helper.go @@ -0,0 +1,17 @@ +package printer + +import ( + "fmt" + "strings" +) + +func mapToCommaSeparatedString(m map[string]interface{}) string { + var sb strings.Builder + for k, v := range m { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString(fmt.Sprintf("%s:%v", k, v)) + } + return sb.String() +} diff --git a/pkg/printer/printer_helper_test.go b/pkg/printer/printer_helper_test.go new file mode 100644 index 00000000..7fc19679 --- /dev/null +++ b/pkg/printer/printer_helper_test.go @@ -0,0 +1,127 @@ +package printer + +import ( + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/doitintl/kube-no-trouble/pkg/config" + "github.com/doitintl/kube-no-trouble/pkg/judge" +) + +func TestTypePrinterPrint(t *testing.T) { + tests := []struct { + name string + labels map[string]interface{} + withLabels config.OptionalFlags + }{ + { + name: "WithLabels", + labels: map[string]interface{}{"app": "version1"}, + withLabels: config.OptionalFlags{Labels: true}, + }, + { + name: "NoLabels", + labels: map[string]interface{}{}, + withLabels: config.OptionalFlags{Labels: false}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + if err != nil { + t.Fatalf(tempFileCreateFailureMessage, err) + } + defer os.Remove(tmpFile.Name()) + + tp := &csvPrinter{ + commonPrinter: &commonPrinter{tmpFile}, + } + + version, _ := judge.NewVersion("1.2.3") + results := []judge.Result{{ + Name: "Name", + Namespace: "Namespace", + Kind: "Kind", + ApiVersion: "1.2.3", + RuleSet: "Test", + ReplaceWith: "4.5.6", + Since: version, + Labels: tt.labels, + }} + + if err := tp.Print(results, tt.withLabels); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + fi, _ := tmpFile.Stat() + if fi.Size() == 0 { + t.Fatalf("expected non-zero size output file: %v", err) + } + }) + } +} + +func TestMapToCommaSeparatedString(t *testing.T) { + tests := []struct { + input map[string]interface{} + expected string + }{ + { + input: map[string]interface{}{}, + expected: "", + }, + { + input: map[string]interface{}{"key1": "value1"}, + expected: "key1:value1", + }, + { + input: map[string]interface{}{"key1": "value1", "key2": "value2"}, + expected: "key1:value1, key2:value2", + }, + { + input: map[string]interface{}{"key1": 123, "key2": true, "key3": 45.67}, + expected: "key1:123, key2:true, key3:45.67", + }, + } + + for _, test := range tests { + result := mapToCommaSeparatedString(test.input) + parsedResult := parseCSVString(result) + parsedExpected := parseCSVString(test.expected) + + // Compare the parsed maps + if !compareMaps(parsedResult, parsedExpected) { + t.Errorf("For input %v, expected %s but got %s", test.input, test.expected, result) + } + } +} + +func parseCSVString(s string) map[string]string { + result := make(map[string]string) + if s == "" { + return result + } + + pairs := strings.Split(s, ", ") + for _, pair := range pairs { + kv := strings.SplitN(pair, ":", 2) + if len(kv) == 2 { + result[kv[0]] = kv[1] + } + } + return result +} + +func compareMaps(a, b map[string]string) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + if bv, ok := b[k]; !ok || v != bv { + return false + } + } + return true +} diff --git a/pkg/printer/text.go b/pkg/printer/text.go index f613cc7b..8e9b0eb0 100644 --- a/pkg/printer/text.go +++ b/pkg/printer/text.go @@ -6,6 +6,7 @@ import ( "strings" "text/tabwriter" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -30,7 +31,7 @@ func (c *textPrinter) Close() error { return c.commonPrinter.Close() } -func (c *textPrinter) Print(results []judge.Result) error { +func (c *textPrinter) Print(results []judge.Result, optionalFlags config.OptionalFlags) error { sort.Slice(results, func(i, j int) bool { return results[i].Name < results[j].Name @@ -54,9 +55,18 @@ func (c *textPrinter) Print(results []judge.Result) error { fmt.Fprintf(w, "%s\n", strings.Repeat("_", 90)) fmt.Fprintf(w, ">>> %s <<<\n", ruleSet) fmt.Fprintf(w, "%s\n", strings.Repeat("-", 90)) - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s (%s)\n", "KIND", "NAMESPACE", "NAME", "API_VERSION", "REPLACE_WITH", "SINCE") + if optionalFlags.Labels { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s \t(%s) \t%s\n", "KIND", "NAMESPACE", "NAME", "API_VERSION", "REPLACE_WITH", "SINCE", "LABELS") + } else { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s \t(%s)\n", "KIND", "NAMESPACE", "NAME", "API_VERSION", "REPLACE_WITH", "SINCE") + } + + } + if optionalFlags.Labels { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s \t(%s) \t%s\n", r.Kind, r.Namespace, r.Name, r.ApiVersion, r.ReplaceWith, r.Since, mapToCommaSeparatedString(r.Labels)) + } else { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s \t(%s) \n", r.Kind, r.Namespace, r.Name, r.ApiVersion, r.ReplaceWith, r.Since) } - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s (%s)\n", r.Kind, r.Namespace, r.Name, r.ApiVersion, r.ReplaceWith, r.Since) } w.Flush() return nil diff --git a/pkg/printer/text_test.go b/pkg/printer/text_test.go index 6888e88d..ca6662ef 100644 --- a/pkg/printer/text_test.go +++ b/pkg/printer/text_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/doitintl/kube-no-trouble/pkg/config" "github.com/doitintl/kube-no-trouble/pkg/judge" ) @@ -51,9 +52,11 @@ func Test_textPrinter_Print(t *testing.T) { } version, _ := judge.NewVersion("1.2.3") - results := []judge.Result{{"Name", "Namespace", "Kind", "1.2.3", "Test", "4.5.6", version}} + labels := map[string]interface{}{"key1": "value1"} - if err := tp.Print(results); err != nil { + results := []judge.Result{{Name: "Name", Namespace: "Namespace", Kind: "Kind", ApiVersion: "1.2.3", RuleSet: "Test", ReplaceWith: "4.5.6", Since: version, Labels: labels}} + + if err := tp.Print(results, config.OptionalFlags{Labels: true}); err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/rules/rego/deprecated-1-26.rego b/pkg/rules/rego/deprecated-1-26.rego index e2d0e09f..7ae9e2bf 100644 --- a/pkg/rules/rego/deprecated-1-26.rego +++ b/pkg/rules/rego/deprecated-1-26.rego @@ -12,6 +12,7 @@ main[return] { "ReplaceWith": api.new, "RuleSet": "Deprecated APIs removed in 1.26", "Since": api.since, + "Labels": get_default(resource.metadata, "labels", ""), } } diff --git a/pkg/rules/rules_test.go b/pkg/rules/rules_test.go index cc617708..e8156347 100644 --- a/pkg/rules/rules_test.go +++ b/pkg/rules/rules_test.go @@ -75,7 +75,7 @@ func TestRenderRuleRego(t *testing.T) { func TestRenderRuleTmpl(t *testing.T) { additionalResources := []schema.GroupVersionKind{ - schema.GroupVersionKind{ + { Group: "example.com", Version: "v2", Kind: "Test", diff --git a/test/rules_custom_test.go b/test/rules_custom_test.go index 0ac387fc..e6c273ae 100644 --- a/test/rules_custom_test.go +++ b/test/rules_custom_test.go @@ -14,7 +14,7 @@ func TestRegoCustom(t *testing.T) { manifestName := "../fixtures/issuer-v1alpha2.yaml" expectedKind := "Issuer" additionalKinds := []schema.GroupVersionKind{ - schema.GroupVersionKind{ + { Group: "cert-manager.io", Version: "v1alpha2", Kind: "Issuer",