From 19730a20711ed41a0a3b2268b000a16f54698cbf Mon Sep 17 00:00:00 2001 From: dark0dave Date: Fri, 25 Oct 2024 13:39:12 +0100 Subject: [PATCH] chore(context): Remove unneeded global context Signed-off-by: dark0dave --- .mise.toml | 2 + cmd/kubent/main.go | 17 +++--- cmd/kubent/main_test.go | 15 +++-- pkg/config/config.go | 24 +++----- pkg/config/config_test.go | 17 ++---- pkg/context/context-keys.go | 5 -- pkg/printer/csv.go | 18 ++---- pkg/printer/csv_test.go | 43 ++++++-------- pkg/printer/json.go | 16 ++---- pkg/printer/json_test.go | 46 +++++++-------- pkg/printer/printer.go | 57 +++++++++---------- pkg/printer/printer_helper.go | 10 ---- pkg/printer/printer_helper_test.go | 12 ++-- pkg/printer/printer_test.go | 90 +++++++++++------------------- pkg/printer/text.go | 19 ++----- pkg/printer/text_test.go | 43 ++++++-------- 16 files changed, 174 insertions(+), 260 deletions(-) create mode 100644 .mise.toml delete mode 100644 pkg/context/context-keys.go diff --git a/.mise.toml b/.mise.toml new file mode 100644 index 00000000..b5d311e6 --- /dev/null +++ b/.mise.toml @@ -0,0 +1,2 @@ +[tools] +go = "1.23.0" diff --git a/cmd/kubent/main.go b/cmd/kubent/main.go index 0ea68824..9568b8a1 100644 --- a/cmd/kubent/main.go +++ b/cmd/kubent/main.go @@ -1,7 +1,6 @@ package main import ( - "context" "flag" "fmt" "io" @@ -97,12 +96,11 @@ func getServerVersion(cv *judge.Version, collectors []collector.Collector) (*jud } func main() { - ctx := context.Background() exitCode := EXIT_CODE_FAIL_GENERIC configureGlobalLogging() - config, ctx, err := config.NewFromFlags(ctx) + config, err := config.NewFromFlags() if err != nil { log.Fatal().Err(err).Msg("failed to parse config flags") @@ -159,7 +157,12 @@ func main() { log.Fatal().Err(err).Str("name", "Rego").Msg("Failed to filter results") } - err = outputResults(results, config.Output, config.OutputFile, ctx) + options, err := printer.NewPrinterOptions(config.OutputFile, config.ShowLabels) + if err != nil { + log.Fatal().Err(err).Msgf("Failed to create output file") + } + + err = outputResults(results, config.Output, options) if err != nil { log.Fatal().Err(err).Msgf("Failed to output results") } @@ -183,14 +186,14 @@ func configureGlobalLogging() { log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr}) } -func outputResults(results []judge.Result, outputType string, outputFile string, ctx context.Context) error { - printer, err := printer.NewPrinter(outputType, outputFile) +func outputResults(results []judge.Result, outputType string, options *printer.PrinterOptions) error { + printer, err := printer.NewPrinter(outputType, options) if err != nil { return fmt.Errorf("failed to create printer: %v", err) } defer printer.Close() - err = printer.Print(results, ctx) + err = printer.Print(results) 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 b7db2b99..12f88041 100644 --- a/cmd/kubent/main_test.go +++ b/cmd/kubent/main_test.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "context" "encoding/base64" "encoding/json" "errors" @@ -14,8 +13,8 @@ import ( "github.com/doitintl/kube-no-trouble/pkg/collector" "github.com/doitintl/kube-no-trouble/pkg/config" - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" "github.com/doitintl/kube-no-trouble/pkg/judge" + "github.com/doitintl/kube-no-trouble/pkg/printer" "github.com/rs/zerolog" ) @@ -288,15 +287,15 @@ func Test_outputResults(t *testing.T) { }{ {"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}, } - labelsFlag := false - ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &labelsFlag) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile, ctx); (err != nil) != tt.wantErr { + options, err := printer.NewPrinterOptions(tt.args.outputFile, false) + if err != nil { + t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr) + } + if err = outputResults(tt.args.results, tt.args.outputType, options); (err != nil) != tt.wantErr { t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr) } }) @@ -305,6 +304,6 @@ func Test_outputResults(t *testing.T) { func Test_configureGlobalLogging(t *testing.T) { // just make sure the method runs, this is mostly covered - //by the Test_MainExitCodes + // by the Test_MainExitCodes configureGlobalLogging() } diff --git a/pkg/config/config.go b/pkg/config/config.go index b1cff056..e9da7a90 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,7 +1,6 @@ package config import ( - "context" "errors" "fmt" "io/fs" @@ -10,7 +9,6 @@ import ( "strings" "unicode" - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" "github.com/doitintl/kube-no-trouble/pkg/judge" "k8s.io/client-go/tools/clientcmd" @@ -38,16 +36,15 @@ type Config struct { OutputFile string TargetVersion *judge.Version KubentVersion bool + ShowLabels bool } -func NewFromFlags(ctx context.Context) (*Config, context.Context, error) { +func NewFromFlags() (*Config, error) { config := Config{ LogLevel: ZeroLogLevel(zerolog.InfoLevel), TargetVersion: &judge.Version{}, } - var labels bool - flag.StringSliceVarP(&config.AdditionalKinds, "additional-kind", "a", []string{}, "additional kinds of resources to report in Kind.version.group.com format") flag.StringSliceVarP(&config.AdditionalAnnotations, "additional-annotation", "A", []string{}, "additional annotations that should be checked to determine the last applied config") flag.BoolVarP(&config.Cluster, "cluster", "c", true, "enable Cluster collector") @@ -57,26 +54,23 @@ func NewFromFlags(ctx context.Context) (*Config, context.Context, error) { flag.BoolVar(&config.Helm3, "helm3", true, "enable Helm v3 collector") flag.StringSliceVarP(&config.Filenames, "filename", "f", []string{}, "manifests to check, use - for stdin") flag.StringVarP(&config.Kubeconfig, "kubeconfig", "k", os.Getenv(clientcmd.RecommendedConfigPathEnvVar), "path to the kubeconfig file") - flag.StringVarP(&config.Output, "output", "o", "text", "output format - [text|json|csv]") + flag.StringVarP(&config.Output, "output", "o", TEXT, "output format - [text|json|csv]") 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(&labels, "labels", false, "print resource labels") - + flag.BoolVar(&config.ShowLabels, "labels", false, "print resource labels") flag.Parse() - newContext := context.WithValue(ctx, ctxKey.LABELS_CTX_KEY, &labels) - if !isValidOutputFormat(config.Output) { - return nil, nil, fmt.Errorf("failed to validate argument output: %s", config.Output) + return nil, fmt.Errorf("failed to validate argument output: %s", config.Output) } if err := validateOutputFile(config.OutputFile); err != nil { - return nil, nil, fmt.Errorf("failed to validate argument output-file: %w", err) + return nil, fmt.Errorf("failed to validate argument output-file: %w", err) } if err := validateAdditionalResources(config.AdditionalKinds); err != nil { - return nil, nil, fmt.Errorf("failed to validate arguments: %w", err) + return nil, fmt.Errorf("failed to validate arguments: %w", err) } // This is a little ugly, but I think preferred to implementing @@ -86,10 +80,10 @@ func NewFromFlags(ctx context.Context) (*Config, context.Context, error) { config.TargetVersion = nil } - return &config, newContext, nil + return &config, nil } -// Previuosly this was handled by a printer.go ParsePrinter function +// Previously 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 { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 2a4b1cba..a8cd40fb 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1,7 +1,6 @@ package config import ( - "context" "os" "testing" @@ -13,7 +12,6 @@ import ( func TestValidLogLevelFromFlags(t *testing.T) { oldArgs := os.Args[1] defer func() { os.Args[1] = oldArgs }() - ctx := context.Background() var validLevels = []string{"trace", "debug", "info", "warn", "error", "fatal", "panic", "", "disabled"} for i, level := range validLevels { @@ -22,7 +20,7 @@ func TestValidLogLevelFromFlags(t *testing.T) { os.Args[1] = "--log-level=" + level - config, _, err := NewFromFlags(ctx) + config, err := NewFromFlags() if err != nil { t.Errorf("Flags parsing failed %s", err) @@ -48,9 +46,8 @@ func TestInvalidLogLevelFromFlags(t *testing.T) { func TestNewFromFlags(t *testing.T) { // reset for testing pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError) - ctx := context.Background() - config, _, err := NewFromFlags(ctx) + config, err := NewFromFlags() if err != nil { t.Errorf("Flags parsing failed %s", err) @@ -95,7 +92,6 @@ func TestTargetVersion(t *testing.T) { validVersions := []string{ "1.16", "1.16.3", "1.2.3", } - ctx := context.Background() oldArgs := os.Args[1] defer func() { os.Args[1] = oldArgs }() @@ -105,7 +101,7 @@ func TestTargetVersion(t *testing.T) { pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError) os.Args[1] = "--target-version=" + v - config, _, err := NewFromFlags(ctx) + config, err := NewFromFlags() if err != nil { t.Errorf("Flags parsing failed %s", err) @@ -126,7 +122,7 @@ func TestTargetVersionInvalid(t *testing.T) { invalidVersions := []string{ "1.blah", "nope", } - ctx := context.Background() + oldArgs := os.Args[1] defer func() { os.Args[1] = oldArgs }() @@ -135,7 +131,7 @@ func TestTargetVersionInvalid(t *testing.T) { pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ContinueOnError) os.Args[1] = "--target-version=" + v - config, _, _ := NewFromFlags(ctx) + config, _ := NewFromFlags() if config.TargetVersion != nil { t.Errorf("expected --target-version flag parsing to fail for: %s", v) @@ -147,7 +143,6 @@ func TestContext(t *testing.T) { validContexts := []string{ "my-context", } - ctx := context.Background() oldArgs := os.Args[1] defer func() { os.Args[1] = oldArgs }() @@ -156,7 +151,7 @@ func TestContext(t *testing.T) { pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError) os.Args[1] = "--context=" + context - config, _, err := NewFromFlags(ctx) + config, err := NewFromFlags() if err != nil { t.Errorf("Flags parsing failed %s", err) diff --git a/pkg/context/context-keys.go b/pkg/context/context-keys.go deleted file mode 100644 index b6eef230..00000000 --- a/pkg/context/context-keys.go +++ /dev/null @@ -1,5 +0,0 @@ -package context - -type ctxKey string - -const LABELS_CTX_KEY ctxKey = "labels" diff --git a/pkg/printer/csv.go b/pkg/printer/csv.go index 1030e47a..7fcc3745 100644 --- a/pkg/printer/csv.go +++ b/pkg/printer/csv.go @@ -1,7 +1,6 @@ package printer import ( - "context" "encoding/csv" "fmt" "sort" @@ -14,8 +13,8 @@ type csvPrinter struct { } // newCSVPrinter creates new CSV printer that prints to given output file -func newCSVPrinter(outputFileName string) (Printer, error) { - cp, err := newCommonPrinter(outputFileName) +func newCSVPrinter(options *PrinterOptions) (Printer, error) { + cp, err := newCommonPrinter(options) if err != nil { return nil, fmt.Errorf("failed to create new common printer: %w", err) } @@ -30,7 +29,7 @@ func (c *csvPrinter) Close() error { } // Print will print results in CSV format -func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error { +func (c *csvPrinter) Print(results []judge.Result) error { sort.Slice(results, func(i, j int) bool { return results[i].Name < results[j].Name @@ -45,7 +44,7 @@ func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error { return results[i].RuleSet < results[j].RuleSet }) - w := csv.NewWriter(c.commonPrinter.outputFile) + w := csv.NewWriter(c.commonPrinter.options.outputFile) fields := []string{ "api_version", @@ -57,12 +56,7 @@ func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error { "rule_set", } - labels, err := shouldShowLabels(ctx) - if err != nil { - return fmt.Errorf("failed to get labels: %w", err) - } - - if labels != nil && *labels { + if c.options.showLabels { fields = append(fields, "labels") } @@ -79,7 +73,7 @@ func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error { r.RuleSet, } - if labels != nil && *labels { + if c.options.showLabels { row = append(row, mapToCommaSeparatedString(r.Labels)) } diff --git a/pkg/printer/csv_test.go b/pkg/printer/csv_test.go index 38f3921f..d9cef929 100644 --- a/pkg/printer/csv_test.go +++ b/pkg/printer/csv_test.go @@ -1,33 +1,28 @@ package printer import ( - "context" - "io/ioutil" "os" "testing" - - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" ) func TestNewCSVPrinter(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFileName string - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-stdout", "-", false}, - {"good-file", tmpFile.Name(), false}, - {"bad-empty", "", true}, + {"good-stdout", PrinterOptions{outputFile: os.Stdout}, false}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := newCSVPrinter(tt.outputFileName) + got, err := newCSVPrinter(&tt.options) if (err != nil) != tt.wantErr { t.Errorf("Unexpected error %v, wantErr %v", err, tt.wantErr) return @@ -41,22 +36,20 @@ func TestNewCSVPrinter(t *testing.T) { } func TestCSVPrinterPrint(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) + options := &PrinterOptions{outputFile: tmpFile} tp := &csvPrinter{ - commonPrinter: &commonPrinter{tmpFile}, + commonPrinter: &commonPrinter{options}, } - labelsFlag := false - ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &labelsFlag) - results := getTestResult(map[string]interface{}{"key2": "value2"}) - if err := tp.Print(results, ctx); err != nil { + if err := tp.Print(results); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -67,24 +60,24 @@ func TestCSVPrinterPrint(t *testing.T) { } func TestCSVPrinterClose(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFile *os.File - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-file", tmpFile, false}, - {"bad-closed-file", tmpFile, true}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, + {"bad-closed-file", PrinterOptions{outputFile: tmpFile}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &csvPrinter{ - commonPrinter: &commonPrinter{tt.outputFile}, + commonPrinter: &commonPrinter{&tt.options}, } if err := c.Close(); (err != nil) != tt.wantErr { t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr) diff --git a/pkg/printer/json.go b/pkg/printer/json.go index 2c19e71d..468363d2 100644 --- a/pkg/printer/json.go +++ b/pkg/printer/json.go @@ -2,7 +2,6 @@ package printer import ( "bufio" - "context" "encoding/json" "fmt" @@ -14,8 +13,8 @@ type jsonPrinter struct { } // newJSONPrinter creates new JSON printer that prints to given output file -func newJSONPrinter(outputFileName string) (Printer, error) { - cp, err := newCommonPrinter(outputFileName) +func newJSONPrinter(options *PrinterOptions) (Printer, error) { + cp, err := newCommonPrinter(options) if err != nil { return nil, fmt.Errorf("failed to create new common printer: %w", err) } @@ -30,21 +29,18 @@ func (c *jsonPrinter) Close() error { } // Print will print results in text format -func (c *jsonPrinter) Print(results []judge.Result, ctx context.Context) error { - writer := bufio.NewWriter(c.commonPrinter.outputFile) +func (c *jsonPrinter) Print(results []judge.Result) error { + writer := bufio.NewWriter(c.commonPrinter.options.outputFile) defer writer.Flush() encoder := json.NewEncoder(writer) encoder.SetIndent("", "\t") - labels, err := shouldShowLabels(ctx) - if err != nil { - return fmt.Errorf("failed to get labels flag from context: %w", err) - } else if labels != nil && !*labels { + if !c.commonPrinter.options.showLabels { removeLabels(results) } - err = encoder.Encode(results) + err := encoder.Encode(results) if err != nil { return err } diff --git a/pkg/printer/json_test.go b/pkg/printer/json_test.go index 6097f0f3..7eaab228 100644 --- a/pkg/printer/json_test.go +++ b/pkg/printer/json_test.go @@ -1,71 +1,65 @@ package printer import ( - "context" "encoding/json" - "io/ioutil" "os" "reflect" "testing" - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" "github.com/doitintl/kube-no-trouble/pkg/judge" ) func Test_newJSONPrinter(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFileName string - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-stdout", "-", false}, - {"good-file", tmpFile.Name(), false}, - {"bad-empty", "", true}, + {"good-stdout", PrinterOptions{outputFile: os.Stdout}, false}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := newJSONPrinter(tt.outputFileName) + got, err := newJSONPrinter(&tt.options) if (err != nil) != tt.wantErr { t.Fatalf("unexpected error: %v, expected error: %v", err, tt.wantErr) } - if err != nil && got != nil { + if err != nil || got == nil { t.Errorf("expected nil in case of an error, got %v", got) } }) } } func Test_jsonPrinter_Print(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) + options := &PrinterOptions{outputFile: tmpFile} c := &jsonPrinter{ - commonPrinter: &commonPrinter{tmpFile}, + commonPrinter: &commonPrinter{options}, } results := getTestResult(map[string]interface{}{"key2": "value2"}) - labelsFlag := false - ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &labelsFlag) - - if err := c.Print(results, ctx); err != nil { + if err := c.Print(results); err != nil { t.Fatalf("unexpected error: %v", err) } tmpFile.Seek(0, 0) var readResults []judge.Result - readBytes, err := ioutil.ReadAll(tmpFile) + readBytes, err := os.ReadFile(tmpFile.Name()) if err != nil { t.Fatalf("unexpected error reading back the file: %v", err) } @@ -78,24 +72,24 @@ func Test_jsonPrinter_Print(t *testing.T) { } func Test_jsonPrinter_Close(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFile *os.File - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-file", tmpFile, false}, - {"bad-closed-file", tmpFile, true}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, + {"bad-closed-file", PrinterOptions{outputFile: tmpFile}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &jsonPrinter{ - commonPrinter: &commonPrinter{tt.outputFile}, + commonPrinter: &commonPrinter{&tt.options}, } if err := c.Close(); (err != nil) != tt.wantErr { t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr) diff --git a/pkg/printer/printer.go b/pkg/printer/printer.go index 880f464a..67057efb 100644 --- a/pkg/printer/printer.go +++ b/pkg/printer/printer.go @@ -1,53 +1,62 @@ package printer import ( - "context" "fmt" "os" "github.com/doitintl/kube-no-trouble/pkg/judge" ) -var printers = map[string]func(string) (Printer, error){ - "json": newJSONPrinter, - "text": newTextPrinter, - "csv": newCSVPrinter, -} - type Printer interface { - Print([]judge.Result, context.Context) error + Print([]judge.Result) error Close() error } -type commonPrinter struct { +type PrinterOptions struct { + showLabels bool outputFile *os.File } -// newCommonPrinter creates new printer that prints to given output file -func newCommonPrinter(outputFileName string) (*commonPrinter, error) { - outputFile, err := ensureOutputFileExists(outputFileName) +type commonPrinter struct { + options *PrinterOptions +} + +func NewPrinterOptions(fileName string, showLabels bool) (*PrinterOptions, error) { + outputFile, err := ensureOutputFileExists(fileName) if err != nil { return nil, fmt.Errorf("failed to ensure output device: %v", err) } + return &PrinterOptions{ + showLabels, + outputFile, + }, nil +} +// newCommonPrinter creates new printer that prints to given output file +func newCommonPrinter(options *PrinterOptions) (*commonPrinter, error) { return &commonPrinter{ - outputFile: outputFile, + options, }, nil } // NewPrinter creates new printer of given type that prints to given output file -func NewPrinter(choice string, outputFileName string) (Printer, error) { - printer, err := ParsePrinter(choice) - if err != nil { - return nil, err +func NewPrinter(choice string, options *PrinterOptions) (Printer, error) { + switch choice { + case "json": + return newJSONPrinter(options) + case "text": + return newTextPrinter(options) + case "csv": + return newCSVPrinter(options) + default: + return nil, fmt.Errorf("unknown printer type %s", choice) } - return printer(outputFileName) } // Close will free resources used by the printer func (c *commonPrinter) Close() error { - if c.outputFile.Name() != os.Stdout.Name() { - if err := c.outputFile.Close(); err != nil { + if c.options.outputFile.Name() != os.Stdout.Name() { + if err := c.options.outputFile.Close(); err != nil { return fmt.Errorf("failed to close output file: %w", err) } } @@ -55,14 +64,6 @@ func (c *commonPrinter) Close() error { return nil } -func ParsePrinter(choice string) (func(string) (Printer, error), error) { - printer, exists := printers[choice] - if !exists { - return nil, fmt.Errorf("unknown printer type %s", choice) - } - return printer, nil -} - // ensureOutputFileExists will open file for writing, or create one if non-existent func ensureOutputFileExists(fileName string) (*os.File, error) { if fileName == "-" { diff --git a/pkg/printer/printer_helper.go b/pkg/printer/printer_helper.go index da4b3eda..051e8d87 100644 --- a/pkg/printer/printer_helper.go +++ b/pkg/printer/printer_helper.go @@ -1,11 +1,8 @@ package printer import ( - "context" "fmt" "strings" - - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" ) func mapToCommaSeparatedString(m map[string]interface{}) string { @@ -18,10 +15,3 @@ func mapToCommaSeparatedString(m map[string]interface{}) string { } return sb.String() } - -func shouldShowLabels(ctx context.Context) (*bool, error) { - if v := ctx.Value(ctxKey.LABELS_CTX_KEY); v != nil { - return ctx.Value(ctxKey.LABELS_CTX_KEY).(*bool), nil - } - return nil, fmt.Errorf("labels flag not present in the context") -} diff --git a/pkg/printer/printer_helper_test.go b/pkg/printer/printer_helper_test.go index 2ba211a1..a403b3c0 100644 --- a/pkg/printer/printer_helper_test.go +++ b/pkg/printer/printer_helper_test.go @@ -1,13 +1,9 @@ package printer import ( - "context" - "io/ioutil" "os" "strings" "testing" - - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" ) func TestTypePrinterPrint(t *testing.T) { @@ -29,20 +25,20 @@ func TestTypePrinterPrint(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) + options := &PrinterOptions{outputFile: tmpFile} tp := &csvPrinter{ - commonPrinter: &commonPrinter{tmpFile}, + commonPrinter: &commonPrinter{options}, } results := getTestResult(tt.labels) - ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &tt.withLabels) - if err := tp.Print(results, ctx); err != nil { + if err := tp.Print(results); err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/printer/printer_test.go b/pkg/printer/printer_test.go index e85be5b0..d916a01f 100644 --- a/pkg/printer/printer_test.go +++ b/pkg/printer/printer_test.go @@ -1,7 +1,6 @@ package printer import ( - "io/ioutil" "os" "reflect" "testing" @@ -12,51 +11,29 @@ const ( tempFileCreateFailureMessage = "failed to create temp dir for testing: %v" ) -func TestParsePrinter(t *testing.T) { - for k, v := range printers { - p, err := ParsePrinter(k) - if err != nil { - t.Fatalf("failed to parse printer %s: %v", k, err) - } - - if reflect.ValueOf(p).Pointer() != reflect.ValueOf(v).Pointer() { - t.Fatalf("expected to get function %p, got %p instead", p, p) - } - } -} - -func TestParsePrinterInvalid(t *testing.T) { - _, err := ParsePrinter("BAD") - if err == nil { - t.Fatalf("expected ParsePrinter to fail with unimplemented type") - } -} - func TestNewPrinter(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - argChoice string - argOutputFileName string - wantErr bool - want Printer + name string + argChoice string + options PrinterOptions + wantErr bool + want Printer }{ - {"good", "json", "-", false, &jsonPrinter{&commonPrinter{}}}, - {"good-file", "json", tmpFile.Name(), false, &jsonPrinter{&commonPrinter{}}}, - {"invalid", "xxx", "", true, nil}, - {"invalid-out", "json", "/not/likely/to/exist", true, nil}, - {"empty", "", "-", true, nil}, - {"empty-out", "json", "", true, nil}, + {"good", "json", PrinterOptions{outputFile: os.Stdout}, false, &jsonPrinter{&commonPrinter{}}}, + {"good-file", "json", PrinterOptions{outputFile: tmpFile}, false, &jsonPrinter{&commonPrinter{}}}, + {"invalid", "xxx", PrinterOptions{outputFile: tmpFile}, true, nil}, + {"empty", "", PrinterOptions{outputFile: os.Stdout}, true, nil}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewPrinter(tt.argChoice, tt.argOutputFileName) - if (err != nil) != tt.wantErr { + got, err := NewPrinter(tt.argChoice, &tt.options) + if (err != nil) && !tt.wantErr { t.Errorf("unexpected error - got: %v, wantErr %v", err, tt.wantErr) return } @@ -68,27 +45,26 @@ func TestNewPrinter(t *testing.T) { } func Test_newCommonPrinter(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - argOutputFileName string - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-file", tmpFile.Name(), false}, - {"good-stdout", "-", false}, - {"bad-empty", "", true}, - {"bad-path", "/this/is/unlikely/to/exist", true}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, + {"good-stdout", PrinterOptions{outputFile: os.Stdout}, false}, + {"bad-empty", PrinterOptions{outputFile: nil, showLabels: false}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := newCommonPrinter(tt.argOutputFileName) - if (err != nil) != tt.wantErr { + got, err := newCommonPrinter(&tt.options) + if (err != nil) && !tt.wantErr { t.Fatalf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr) } @@ -98,9 +74,9 @@ func Test_newCommonPrinter(t *testing.T) { if !tt.wantErr { defer got.Close() - if (tt.argOutputFileName != "-" && got.outputFile.Name() != tt.argOutputFileName) || - tt.argOutputFileName == "-" && got.outputFile.Name() != os.Stdout.Name() { - t.Errorf("unexpected file name- got: %s, want: %s", got.outputFile.Name(), tt.argOutputFileName) + if (tt.options.outputFile.Name() != "-" && got.options.outputFile.Name() != tt.options.outputFile.Name()) || + tt.options.outputFile.Name() == "-" && got.options.outputFile.Name() != os.Stdout.Name() { + t.Errorf("unexpected file name- got: %s, want: %s", got.options.outputFile.Name(), tt.options.outputFile.Name()) } } }) @@ -108,7 +84,7 @@ func Test_newCommonPrinter(t *testing.T) { } func Test_ensureOutputFileExists(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } @@ -150,25 +126,25 @@ func Test_ensureOutputFileExists(t *testing.T) { } func Test_commonPrinter_Close(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFile *os.File - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-file", tmpFile, false}, - {"good-stdout", os.Stdout, false}, - {"bad-closed-file", tmpFile, true}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, + {"good-stdout", PrinterOptions{outputFile: os.Stdout}, false}, + {"bad-closed-file", PrinterOptions{outputFile: tmpFile}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &commonPrinter{ - outputFile: tt.outputFile, + options: &tt.options, } if err := c.Close(); (err != nil) != tt.wantErr { t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr) diff --git a/pkg/printer/text.go b/pkg/printer/text.go index b521ce3b..18647ec0 100644 --- a/pkg/printer/text.go +++ b/pkg/printer/text.go @@ -1,7 +1,6 @@ package printer import ( - "context" "fmt" "sort" "strings" @@ -15,8 +14,8 @@ type textPrinter struct { } // newTextPrinter creates new text printer that prints to given output file -func newTextPrinter(outputFileName string) (Printer, error) { - cp, err := newCommonPrinter(outputFileName) +func newTextPrinter(options *PrinterOptions) (Printer, error) { + cp, err := newCommonPrinter(options) if err != nil { return nil, fmt.Errorf("failed to create new common printer: %w", err) } @@ -31,8 +30,7 @@ func (c *textPrinter) Close() error { return c.commonPrinter.Close() } -func (c *textPrinter) Print(results []judge.Result, ctx context.Context) error { - +func (c *textPrinter) Print(results []judge.Result) error { sort.Slice(results, func(i, j int) bool { return results[i].Name < results[j].Name }) @@ -47,12 +45,7 @@ func (c *textPrinter) Print(results []judge.Result, ctx context.Context) error { }) ruleSet := "" - w := tabwriter.NewWriter(c.commonPrinter.outputFile, 10, 0, 3, ' ', 0) - - labels, err := shouldShowLabels(ctx) - if err != nil { - return fmt.Errorf("failed to get labels flag from context: %w", err) - } + w := tabwriter.NewWriter(c.commonPrinter.options.outputFile, 10, 0, 3, ' ', 0) for _, r := range results { if ruleSet != r.RuleSet { @@ -60,14 +53,14 @@ func (c *textPrinter) Print(results []judge.Result, ctx context.Context) error { fmt.Fprintf(w, "%s\n", strings.Repeat("_", 90)) fmt.Fprintf(w, ">>> %s <<<\n", ruleSet) fmt.Fprintf(w, "%s\n", strings.Repeat("-", 90)) - if labels != nil && *labels { + if c.commonPrinter.options.showLabels { 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 labels != nil && *labels { + if c.commonPrinter.options.showLabels { 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) diff --git a/pkg/printer/text_test.go b/pkg/printer/text_test.go index 7e6123ad..91de6280 100644 --- a/pkg/printer/text_test.go +++ b/pkg/printer/text_test.go @@ -1,33 +1,28 @@ package printer import ( - "context" - "io/ioutil" "os" "testing" - - ctxKey "github.com/doitintl/kube-no-trouble/pkg/context" ) func Test_newTextPrinter(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFileName string - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-stdout", "-", false}, - {"good-file", tmpFile.Name(), false}, - {"bad-empty", "", true}, + {"good-stdout", PrinterOptions{outputFile: os.Stdout}, false}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := newTextPrinter(tt.outputFileName) + got, err := newTextPrinter(&tt.options) if (err != nil) != tt.wantErr { t.Errorf("Unexpected error %v, wantErr %v", err, tt.wantErr) return @@ -41,21 +36,19 @@ func Test_newTextPrinter(t *testing.T) { } func Test_textPrinter_Print(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) - + options := &PrinterOptions{outputFile: tmpFile} tp := &textPrinter{ - commonPrinter: &commonPrinter{tmpFile}, + commonPrinter: &commonPrinter{options}, } results := getTestResult(map[string]interface{}{"key2": "value2"}) - labelsFlag := false - ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &labelsFlag) - if err := tp.Print(results, ctx); err != nil { + if err := tp.Print(results); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -66,24 +59,24 @@ func Test_textPrinter_Print(t *testing.T) { } func Test_textPrinter_Close(t *testing.T) { - tmpFile, err := ioutil.TempFile(os.TempDir(), tempFilePrefix) + tmpFile, err := os.CreateTemp(os.TempDir(), tempFilePrefix) if err != nil { t.Fatalf(tempFileCreateFailureMessage, err) } defer os.Remove(tmpFile.Name()) tests := []struct { - name string - outputFile *os.File - wantErr bool + name string + options PrinterOptions + wantErr bool }{ - {"good-file", tmpFile, false}, - {"bad-closed-file", tmpFile, true}, + {"good-file", PrinterOptions{outputFile: tmpFile}, false}, + {"bad-closed-file", PrinterOptions{outputFile: tmpFile}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &textPrinter{ - commonPrinter: &commonPrinter{tt.outputFile}, + commonPrinter: &commonPrinter{&tt.options}, } if err := c.Close(); (err != nil) != tt.wantErr { t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr)