From 29b85d5b839b2d72d3a0058eae58575078f1cde6 Mon Sep 17 00:00:00 2001 From: Aykhan Shahsuvarov Date: Thu, 28 Aug 2025 23:57:00 +0400 Subject: [PATCH] Add config file support to CLI parser Add -f/--config-file flag for loading YAML configs from local or remote sources. Fix error handling to return unmatched errors. --- cmd/cli/main.go | 4 +- go.mod | 13 +++---- pkg/config/cli.go | 37 +++++++++++++++++- pkg/config/cli_test.go | 73 +++++++++++++++++++++++++++++++++++ pkg/types/config_file.go | 9 ++++- pkg/types/config_file_test.go | 8 ++-- pkg/types/errors.go | 8 ++-- pkg/utils/error.go | 2 +- pkg/utils/error_test.go | 10 ++--- 9 files changed, 138 insertions(+), 26 deletions(-) diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 963d5a3..cb618be 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -12,7 +12,7 @@ import ( func main() { cliParser := config.NewConfigCLIParser(os.Args) - _, err := cliParser.Parse() + cfg, err := cliParser.Parse() _ = utils.HandleErrorOrDie(err, utils.OnSentinelError(types.ErrCLINoArgs, func(err error) error { @@ -34,6 +34,8 @@ func main() { return nil }), ) + + fmt.Println(cfg) } func printValidationErrors(parserName string, errors ...types.FieldParseError) { diff --git a/go.mod b/go.mod index 71398e3..c74e483 100644 --- a/go.mod +++ b/go.mod @@ -2,18 +2,17 @@ module github.com/aykhans/dodo go 1.25 -require github.com/stretchr/testify v1.10.0 - require ( - github.com/mattn/go-runewidth v0.0.16 // indirect - github.com/rivo/uniseg v0.4.7 // indirect - golang.org/x/sys v0.30.0 // indirect - golang.org/x/text v0.22.0 // indirect + github.com/jedib0t/go-pretty/v6 v6.6.8 + github.com/stretchr/testify v1.10.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/jedib0t/go-pretty/v6 v6.6.8 + github.com/mattn/go-runewidth v0.0.16 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/rivo/uniseg v0.4.7 // indirect + golang.org/x/sys v0.30.0 // indirect + golang.org/x/text v0.22.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/pkg/config/cli.go b/pkg/config/cli.go index d741a4f..15e7c88 100644 --- a/pkg/config/cli.go +++ b/pkg/config/cli.go @@ -1,6 +1,7 @@ package config import ( + "errors" "flag" "fmt" "net/url" @@ -52,14 +53,19 @@ Flags: -skip-verify bool Skip SSL/TLS certificate verification (default %v)` type ConfigCLIParser struct { - args []string + args []string + configFile *types.ConfigFile } func NewConfigCLIParser(args []string) *ConfigCLIParser { if args == nil { args = []string{} } - return &ConfigCLIParser{args} + return &ConfigCLIParser{args: args} +} + +func (parser ConfigCLIParser) GetConfigFile() *types.ConfigFile { + return parser.configFile } type stringSliceArg []string @@ -85,6 +91,7 @@ func (parser *ConfigCLIParser) Parse() (*Config, error) { var ( config = &Config{} + configFile string yes bool skipVerify bool method string @@ -101,6 +108,9 @@ func (parser *ConfigCLIParser) Parse() (*Config, error) { ) { + flagSet.StringVar(&configFile, "config-file", "", "Config file") + flagSet.StringVar(&configFile, "f", "", "Config file") + flagSet.BoolVar(&yes, "yes", false, "Answer yes to all questions") flagSet.BoolVar(&yes, "y", false, "Answer yes to all questions") @@ -160,6 +170,29 @@ func (parser *ConfigCLIParser) Parse() (*Config, error) { // Iterate over flags that were explicitly set on the command line. flagSet.Visit(func(flagVar *flag.Flag) { switch flagVar.Name { + case "config-file", "f": + var err error + parser.configFile, err = types.ParseConfigFile(configFile) + _ = utils.HandleErrorOrDie(err, + utils.OnSentinelError(types.ErrConfigFileExtensionNotFound, func(err error) error { + fieldParseErrors = append(fieldParseErrors, *types.NewFieldParseError("config-file", errors.New("file extension not found"))) + return nil + }), + utils.OnCustomError(func(err types.RemoteConfigFileParseError) error { + fieldParseErrors = append(fieldParseErrors, *types.NewFieldParseError("config-file", fmt.Errorf("parse error: %w", err))) + return nil + }), + utils.OnCustomError(func(err types.UnknownConfigFileTypeError) error { + fieldParseErrors = append( + fieldParseErrors, + *types.NewFieldParseError( + "config-file", + fmt.Errorf("file type '%s' not supported (supported types: %s)", err.Type, types.ConfigFileTypeYAML.String()), + ), + ) + return nil + }), + ) case "yes", "y": config.Yes = utils.ToPtr(yes) case "skip-verify": diff --git a/pkg/config/cli_test.go b/pkg/config/cli_test.go index e165c4e..5c708cb 100644 --- a/pkg/config/cli_test.go +++ b/pkg/config/cli_test.go @@ -20,6 +20,7 @@ func TestNewConfigCLIParser(t *testing.T) { require.NotNil(t, parser) assert.Equal(t, args, parser.args) + assert.Nil(t, parser.configFile) }) t.Run("NewConfigCLIParser with nil args", func(t *testing.T) { @@ -27,6 +28,7 @@ func TestNewConfigCLIParser(t *testing.T) { require.NotNil(t, parser) assert.Equal(t, []string{}, parser.args) + assert.Nil(t, parser.configFile) }) t.Run("NewConfigCLIParser with empty args", func(t *testing.T) { @@ -35,6 +37,21 @@ func TestNewConfigCLIParser(t *testing.T) { require.NotNil(t, parser) assert.Equal(t, args, parser.args) + assert.Nil(t, parser.configFile) + }) +} + +func TestConfigCLIParser_GetConfigFile(t *testing.T) { + t.Run("GetConfigFile returns nil when no config file is set", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo"}) + assert.Nil(t, parser.GetConfigFile()) + }) + + t.Run("GetConfigFile returns config file when set", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo"}) + expectedConfigFile := &types.ConfigFile{} + parser.configFile = expectedConfigFile + assert.Equal(t, expectedConfigFile, parser.GetConfigFile()) }) } @@ -295,6 +312,58 @@ func TestConfigCLIParser_Parse(t *testing.T) { assert.Equal(t, 10*time.Second, *config.Timeout) }) + t.Run("Parse with config-file flag valid YAML", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo", "-f", "/path/to/config.yaml"}) + config, err := parser.Parse() + + require.NoError(t, err) + require.NotNil(t, config) + assert.NotNil(t, parser.GetConfigFile()) + }) + + t.Run("Parse with config-file flag using long form", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo", "--config-file", "/path/to/config.yml"}) + config, err := parser.Parse() + + require.NoError(t, err) + require.NotNil(t, config) + assert.NotNil(t, parser.GetConfigFile()) + }) + + t.Run("Parse with config-file flag invalid extension", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo", "-f", "/path/to/config"}) + config, err := parser.Parse() + + assert.Nil(t, config) + var fieldErr types.FieldParseErrors + require.ErrorAs(t, err, &fieldErr) + assert.Len(t, fieldErr.Errors, 1) + assert.Equal(t, "config-file", fieldErr.Errors[0].Field) + assert.Contains(t, fieldErr.Errors[0].Err.Error(), "file extension not found") + }) + + t.Run("Parse with config-file flag unsupported file type", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo", "-f", "/path/to/config.json"}) + config, err := parser.Parse() + + assert.Nil(t, config) + var fieldErr types.FieldParseErrors + require.ErrorAs(t, err, &fieldErr) + assert.Len(t, fieldErr.Errors, 1) + assert.Equal(t, "config-file", fieldErr.Errors[0].Field) + assert.Contains(t, fieldErr.Errors[0].Err.Error(), "file type") + assert.Contains(t, fieldErr.Errors[0].Err.Error(), "not supported") + }) + + t.Run("Parse with config-file flag remote URL", func(t *testing.T) { + parser := NewConfigCLIParser([]string{"dodo", "-f", "https://example.com/config.yaml"}) + config, err := parser.Parse() + + require.NoError(t, err) + require.NotNil(t, config) + assert.NotNil(t, parser.GetConfigFile()) + }) + t.Run("Parse with all flags combined", func(t *testing.T) { parser := NewConfigCLIParser([]string{ "dodo", @@ -311,6 +380,7 @@ func TestConfigCLIParser_Parse(t *testing.T) { "-c", "session=token123", "-b", `{"data": "test"}`, "-x", "http://proxy.example.com:3128", + "-f", "/path/to/config.yaml", }) config, err := parser.Parse() @@ -341,6 +411,8 @@ func TestConfigCLIParser_Parse(t *testing.T) { assert.Len(t, config.Proxies, 1) assert.Equal(t, "http://proxy.example.com:3128", config.Proxies[0].String()) + + assert.NotNil(t, parser.GetConfigFile()) }) t.Run("Parse with multiple field parse errors", func(t *testing.T) { @@ -404,6 +476,7 @@ func TestConfigCLIParser_PrintHelp(t *testing.T) { assert.Contains(t, output, "-x, -proxy") assert.Contains(t, output, "-skip-verify") assert.Contains(t, output, "-y, -yes") + assert.Contains(t, output, "-f, -config-file") // Verify default values are included assert.Contains(t, output, Defaults.Method) diff --git a/pkg/types/config_file.go b/pkg/types/config_file.go index d4026a4..505db42 100644 --- a/pkg/types/config_file.go +++ b/pkg/types/config_file.go @@ -15,7 +15,7 @@ const ( func (t ConfigFileType) String() string { switch t { case ConfigFileTypeYAML: - return "yaml" + return "yaml/yml" default: return "unknown" } @@ -57,6 +57,13 @@ func (configFile ConfigFile) LocationType() ConfigFileLocationType { return configFile.locationType } +// ParseConfigFile parses a raw string representing a configuration file +// path or URL and returns a ConfigFile struct. +// It determines the file's type and location (local or remote) based on the string. +// It can return the following errors: +// - ErrConfigFileExtensionNotFound +// - RemoteConfigFileParseError +// - UnknownConfigFileTypeError func ParseConfigFile(configFileRaw string) (*ConfigFile, error) { configFileParsed := &ConfigFile{ path: configFileRaw, diff --git a/pkg/types/config_file_test.go b/pkg/types/config_file_test.go index fe5f1c7..07263de 100644 --- a/pkg/types/config_file_test.go +++ b/pkg/types/config_file_test.go @@ -10,7 +10,7 @@ import ( func TestConfigFileType_String(t *testing.T) { t.Run("ConfigFileTypeYAML returns yaml", func(t *testing.T) { configType := ConfigFileTypeYAML - assert.Equal(t, "yaml", configType.String()) + assert.Equal(t, "yaml/yml", configType.String()) }) t.Run("Unknown config file type returns unknown", func(t *testing.T) { @@ -146,7 +146,7 @@ func TestParseConfigFile(t *testing.T) { configFile, err := ParseConfigFile("config.json") require.Error(t, err) - assert.IsType(t, &UnknownConfigFileTypeError{}, err) + assert.IsType(t, UnknownConfigFileTypeError{}, err) assert.Contains(t, err.Error(), "json") assert.Nil(t, configFile) }) @@ -155,7 +155,7 @@ func TestParseConfigFile(t *testing.T) { configFile, err := ParseConfigFile("http://192.168.1.%30/config.yaml") require.Error(t, err) - assert.IsType(t, &RemoteConfigFileParseError{}, err) + assert.IsType(t, RemoteConfigFileParseError{}, err) assert.Nil(t, configFile) }) @@ -171,7 +171,7 @@ func TestParseConfigFile(t *testing.T) { configFile, err := ParseConfigFile("https://example.com/config.txt") require.Error(t, err) - assert.IsType(t, &UnknownConfigFileTypeError{}, err) + assert.IsType(t, UnknownConfigFileTypeError{}, err) assert.Contains(t, err.Error(), "txt") assert.Nil(t, configFile) }) diff --git a/pkg/types/errors.go b/pkg/types/errors.go index 963180b..90c5d3a 100644 --- a/pkg/types/errors.go +++ b/pkg/types/errors.go @@ -85,11 +85,11 @@ type RemoteConfigFileParseError struct { error error } -func NewRemoteConfigFileParseError(err error) *RemoteConfigFileParseError { +func NewRemoteConfigFileParseError(err error) RemoteConfigFileParseError { if err == nil { err = ErrNoError } - return &RemoteConfigFileParseError{err} + return RemoteConfigFileParseError{err} } func (e RemoteConfigFileParseError) Error() string { @@ -104,8 +104,8 @@ type UnknownConfigFileTypeError struct { Type string } -func NewUnknownConfigFileTypeError(_type string) *UnknownConfigFileTypeError { - return &UnknownConfigFileTypeError{_type} +func NewUnknownConfigFileTypeError(_type string) UnknownConfigFileTypeError { + return UnknownConfigFileTypeError{_type} } func (e UnknownConfigFileTypeError) Error() string { diff --git a/pkg/utils/error.go b/pkg/utils/error.go index ebbf33c..55c8f06 100644 --- a/pkg/utils/error.go +++ b/pkg/utils/error.go @@ -55,7 +55,7 @@ func HandleError(err error, matchers ...ErrorMatcher) (bool, error) { } } - return false, nil // No matcher found + return false, err // No matcher found } // HandleErrorOrDie processes an error against a list of matchers and executes the appropriate handler. diff --git a/pkg/utils/error_test.go b/pkg/utils/error_test.go index 832774e..7bf1757 100644 --- a/pkg/utils/error_test.go +++ b/pkg/utils/error_test.go @@ -17,7 +17,7 @@ type CustomError struct { Message string } -func (e *CustomError) Error() string { +func (e CustomError) Error() string { return fmt.Sprintf("custom error %d: %s", e.Code, e.Message) } @@ -91,16 +91,15 @@ func TestHandleError(t *testing.T) { t.Run("HandleError with no matching handler", func(t *testing.T) { err := errors.New("unhandled error") - handled, result := HandleError(err, + handled, _ := HandleError(err, OnSentinelError(io.EOF, func(e error) error { return nil }), - OnCustomError(func(e *CustomError) error { + OnCustomError(func(e CustomError) error { return nil }), ) assert.False(t, handled) - assert.NoError(t, result) }) t.Run("HandleError with multiple matchers first match wins", func(t *testing.T) { @@ -352,9 +351,8 @@ func TestErrorMatcherEdgeCases(t *testing.T) { } err := errors.New("test error") - handled, result := HandleError(err, matcher) + handled, _ := HandleError(err, matcher) assert.False(t, handled) - assert.NoError(t, result) }) t.Run("Handler that panics", func(t *testing.T) {