From d1d55349527b828ddf6f72e55babf94163e52679 Mon Sep 17 00:00:00 2001 From: Harry Gardiner Date: Thu, 14 Nov 2024 01:29:47 +0000 Subject: [PATCH] ref: Add OutputFormat type to enforce flag validation at parse time --- cmd/kubent/main.go | 2 +- pkg/config/config.go | 26 +++----------------------- pkg/config/config_test.go | 37 +++++++++++++++++++++++++++++++++++++ pkg/config/output_format.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 pkg/config/output_format.go diff --git a/cmd/kubent/main.go b/cmd/kubent/main.go index 9568b8a1..35e1ed80 100644 --- a/cmd/kubent/main.go +++ b/cmd/kubent/main.go @@ -162,7 +162,7 @@ func main() { log.Fatal().Err(err).Msgf("Failed to create output file") } - err = outputResults(results, config.Output, options) + err = outputResults(results, config.Output.String(), options) if err != nil { log.Fatal().Err(err).Msgf("Failed to output results") } diff --git a/pkg/config/config.go b/pkg/config/config.go index e9da7a90..20ac4c0e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -16,12 +16,6 @@ import ( flag "github.com/spf13/pflag" ) -const ( - JSON = "json" - TEXT = "text" - CSV = "csv" -) - type Config struct { AdditionalKinds []string AdditionalAnnotations []string @@ -32,7 +26,7 @@ type Config struct { Helm3 bool Kubeconfig string LogLevel ZeroLogLevel - Output string + Output OutputFormat OutputFile string TargetVersion *judge.Version KubentVersion bool @@ -43,6 +37,7 @@ func NewFromFlags() (*Config, error) { config := Config{ LogLevel: ZeroLogLevel(zerolog.InfoLevel), TargetVersion: &judge.Version{}, + Output: TEXT, } flag.StringSliceVarP(&config.AdditionalKinds, "additional-kind", "a", []string{}, "additional kinds of resources to report in Kind.version.group.com format") @@ -54,17 +49,13 @@ func NewFromFlags() (*Config, 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.VarP(&config.Output, "output", "o", "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(&config.ShowLabels, "labels", false, "print resource labels") flag.Parse() - if !isValidOutputFormat(config.Output) { - return nil, fmt.Errorf("failed to validate argument output: %s", config.Output) - } - if err := validateOutputFile(config.OutputFile); err != nil { return nil, fmt.Errorf("failed to validate argument output-file: %w", err) } @@ -83,17 +74,6 @@ func NewFromFlags() (*Config, error) { return &config, nil } -// 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 { - 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 a8cd40fb..4018d81e 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -56,6 +56,14 @@ func TestNewFromFlags(t *testing.T) { if !config.Cluster && config.Output != "text" { t.Errorf("Config not parsed correctly") } + + err = pflag.CommandLine.Parse([]string{"--output", "json"}) + if err != nil { + t.Errorf("Flag parsing failed: %v", err) + } + if config.Output != "json" { + t.Errorf("Expected output format to be 'json', got '%s'", config.Output) + } } func TestValidateAdditionalResources(t *testing.T) { @@ -183,3 +191,32 @@ func Test_validateOutputFile(t *testing.T) { }) } } + +func TestValidOutputFormatValues(t *testing.T) { + validFormats := []string{"json", "text", "csv"} + for _, format := range validFormats { + t.Run("Valid "+format, func(t *testing.T) { + var output OutputFormat + err := output.Set(format) + if err != nil { + t.Errorf("Expected no error for valid format '%s', got %v", format, err) + } + if output.String() != format { + t.Errorf("Expected output format to be '%s', got '%s'", format, output.String()) + } + }) + } +} + +func TestInvalidOutputFormatValues(t *testing.T) { + invalidFormats := []string{"xml", "yaml", "pdf"} + for _, format := range invalidFormats { + t.Run("Invalid "+format, func(t *testing.T) { + var output OutputFormat + err := output.Set(format) + if err == nil { + t.Errorf("Expected an error for invalid format '%s', but got none", format) + } + }) + } +} diff --git a/pkg/config/output_format.go b/pkg/config/output_format.go new file mode 100644 index 00000000..728941b9 --- /dev/null +++ b/pkg/config/output_format.go @@ -0,0 +1,30 @@ +package config + +import "fmt" + +const ( + JSON = "json" + TEXT = "text" + CSV = "csv" +) + +// implements pflag.Value to enforce strict string matching for the output format during flag parsing +type OutputFormat string + +func (o *OutputFormat) String() string { + return string(*o) +} + +func (o *OutputFormat) Set(value string) error { + switch value { + case JSON, TEXT, CSV: + *o = OutputFormat(value) + return nil + default: + return fmt.Errorf("invalid output format: %s (must be one of: json, text, csv)", value) + } +} + +func (o *OutputFormat) Type() string { + return "OutputFormat" +}