feat(config)!: consolidate custom config parsing into config.Extended (#475)

* feat(config)!: consolidate custom config parsing into config.Extended

Signed-off-by: Marc Nuri <marc@marcnuri.com>

* review(config)!: consolidate custom config parsing into config.Extended

Signed-off-by: Marc Nuri <marc@marcnuri.com>

---------

Signed-off-by: Marc Nuri <marc@marcnuri.com>

Author: manusa

pkg/config/config.go

@@ -73,9 +73,9 @@ type StaticConfig struct {
 	ToolsetConfigs map[string]toml.Primitive `toml:"toolset_configs,omitempty"`
 
 	// Internal: parsed provider configs (not exposed to TOML package)
-	parsedClusterProviderConfigs map[string]ProviderConfig
+	parsedClusterProviderConfigs map[string]Extended
 	// Internal: parsed toolset configs (not exposed to TOML package)
-	parsedToolsetConfigs map[string]ToolsetConfig
+	parsedToolsetConfigs map[string]Extended
 
 	// Internal: the config.toml directory, to help resolve relative file paths
 	configDirPath string
@@ -129,87 +129,28 @@ func ReadToml(configData []byte, opts ...ReadConfigOpt) (*StaticConfig, error) {
 		opt(config)
 	}
 
-	if err := config.parseClusterProviderConfigs(md); err != nil {
+	ctx := withConfigDirPath(context.Background(), config.configDirPath)
+
+	config.parsedClusterProviderConfigs, err = providerConfigRegistry.parse(ctx, md, config.ClusterProviderConfigs)
+	if err != nil {
 		return nil, err
 	}
 
-	if err := config.parseToolsetConfigs(md); err != nil {
+	config.parsedToolsetConfigs, err = toolsetConfigRegistry.parse(ctx, md, config.ToolsetConfigs)
+	if err != nil {
 		return nil, err
 	}
 
 	return config, nil
 }
 
-func (c *StaticConfig) GetProviderConfig(strategy string) (ProviderConfig, bool) {
+func (c *StaticConfig) GetProviderConfig(strategy string) (Extended, bool) {
 	config, ok := c.parsedClusterProviderConfigs[strategy]
 
 	return config, ok
 }
 
-func (c *StaticConfig) parseClusterProviderConfigs(md toml.MetaData) error {
-	if c.parsedClusterProviderConfigs == nil {
-		c.parsedClusterProviderConfigs = make(map[string]ProviderConfig, len(c.ClusterProviderConfigs))
-	}
-
-	ctx := withConfigDirPath(context.Background(), c.configDirPath)
-
-	for strategy, primitive := range c.ClusterProviderConfigs {
-		parser, ok := getProviderConfigParser(strategy)
-		if !ok {
-			continue
-		}
-
-		providerConfig, err := parser(ctx, primitive, md)
-		if err != nil {
-			return fmt.Errorf("failed to parse config for ClusterProvider '%s': %w", strategy, err)
-		}
-
-		if err := providerConfig.Validate(); err != nil {
-			return fmt.Errorf("invalid config file for ClusterProvider '%s': %w", strategy, err)
-		}
-
-		c.parsedClusterProviderConfigs[strategy] = providerConfig
-	}
-
-	return nil
-}
-
-func (c *StaticConfig) parseToolsetConfigs(md toml.MetaData) error {
-	if c.parsedToolsetConfigs == nil {
-		c.parsedToolsetConfigs = make(map[string]ToolsetConfig, len(c.ToolsetConfigs))
-	}
-
-	ctx := withConfigDirPath(context.Background(), c.configDirPath)
-
-	for name, primitive := range c.ToolsetConfigs {
-		parser, ok := getToolsetConfigParser(name)
-		if !ok {
-			continue
-		}
-
-		toolsetConfig, err := parser(ctx, primitive, md)
-		if err != nil {
-			return fmt.Errorf("failed to parse config for Toolset '%s': %w", name, err)
-		}
-
-		if err := toolsetConfig.Validate(); err != nil {
-			return fmt.Errorf("invalid config file for Toolset '%s': %w", name, err)
-		}
-
-		c.parsedToolsetConfigs[name] = toolsetConfig
-	}
-
-	return nil
-}
-
-func (c *StaticConfig) GetToolsetConfig(name string) (ToolsetConfig, bool) {
+func (c *StaticConfig) GetToolsetConfig(name string) (Extended, bool) {
 	cfg, ok := c.parsedToolsetConfigs[name]
 	return cfg, ok
 }
-
-func (c *StaticConfig) SetToolsetConfig(name string, cfg ToolsetConfig) {
-	if c.parsedToolsetConfigs == nil {
-		c.parsedToolsetConfigs = make(map[string]ToolsetConfig)
-	}
-	c.parsedToolsetConfigs[name] = cfg
-}

pkg/config/context.go

@@ -0,0 +1,23 @@
+package config
+
+import "context"
+
+type configDirPathKey struct{}
+
+func withConfigDirPath(ctx context.Context, dirPath string) context.Context {
+	return context.WithValue(ctx, configDirPathKey{}, dirPath)
+}
+
+func ConfigDirPathFromContext(ctx context.Context) string {
+	val := ctx.Value(configDirPathKey{})
+
+	if val == nil {
+		return ""
+	}
+
+	if strVal, ok := val.(string); ok {
+		return strVal
+	}
+
+	return ""
+}

pkg/config/extended.go

@@ -0,0 +1,61 @@
+package config
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/BurntSushi/toml"
+)
+
+// Extended is the interface that all configuration extensions must implement.
+// Each extended config manager registers a factory function to parse its config from TOML primitives
+type Extended interface {
+	Validate() error
+}
+
+type ExtendedConfigParser func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error)
+
+type extendedConfigRegistry struct {
+	parsers map[string]ExtendedConfigParser
+}
+
+func newExtendedConfigRegistry() *extendedConfigRegistry {
+	return &extendedConfigRegistry{
+		parsers: make(map[string]ExtendedConfigParser),
+	}
+}
+
+func (r *extendedConfigRegistry) register(name string, parser ExtendedConfigParser) {
+	if _, exists := r.parsers[name]; exists {
+		panic("extended config parser already registered for name: " + name)
+	}
+
+	r.parsers[name] = parser
+}
+
+func (r *extendedConfigRegistry) parse(ctx context.Context, metaData toml.MetaData, configs map[string]toml.Primitive) (map[string]Extended, error) {
+	if len(configs) == 0 {
+		return make(map[string]Extended), nil
+	}
+	parsedConfigs := make(map[string]Extended, len(configs))
+
+	for name, primitive := range configs {
+		parser, ok := r.parsers[name]
+		if !ok {
+			continue
+		}
+
+		extendedConfig, err := parser(ctx, primitive, metaData)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse extended config for '%s': %w", name, err)
+		}
+
+		if err = extendedConfig.Validate(); err != nil {
+			return nil, fmt.Errorf("failed to validate extended config for '%s': %w", name, err)
+		}
+
+		parsedConfigs[name] = extendedConfig
+	}
+
+	return parsedConfigs, nil
+}

pkg/config/provider_config.go

@@ -1,54 +1,7 @@
 package config
 
-import (
-	"context"
-	"fmt"
+var providerConfigRegistry = newExtendedConfigRegistry()
 
-	"github.com/BurntSushi/toml"
-)
-
-// ProviderConfig is the interface that all provider-specific configurations must implement.
-// Each provider registers a factory function to parse its config from TOML primitives
-type ProviderConfig interface {
-	Validate() error
-}
-
-type ProviderConfigParser func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error)
-
-type configDirPathKey struct{}
-
-func withConfigDirPath(ctx context.Context, dirPath string) context.Context {
-	return context.WithValue(ctx, configDirPathKey{}, dirPath)
-}
-
-func ConfigDirPathFromContext(ctx context.Context) string {
-	val := ctx.Value(configDirPathKey{})
-
-	if val == nil {
-		return ""
-	}
-
-	if strVal, ok := val.(string); ok {
-		return strVal
-	}
-
-	return ""
-}
-
-var (
-	providerConfigParsers = make(map[string]ProviderConfigParser)
-)
-
-func RegisterProviderConfig(strategy string, parser ProviderConfigParser) {
-	if _, exists := providerConfigParsers[strategy]; exists {
-		panic(fmt.Sprintf("provider config parser already registered for strategy '%s'", strategy))
-	}
-
-	providerConfigParsers[strategy] = parser
-}
-
-func getProviderConfigParser(strategy string) (ProviderConfigParser, bool) {
-	provider, ok := providerConfigParsers[strategy]
-
-	return provider, ok
+func RegisterProviderConfig(name string, parser ExtendedConfigParser) {
+	providerConfigRegistry.register(name, parser)
 }

pkg/config/provider_config_test.go

@@ -12,21 +12,16 @@ import (
 
 type ProviderConfigSuite struct {
 	BaseConfigSuite
-	originalProviderConfigParsers map[string]ProviderConfigParser
+	originalProviderConfigRegistry *extendedConfigRegistry
 }
 
 func (s *ProviderConfigSuite) SetupTest() {
-	s.originalProviderConfigParsers = make(map[string]ProviderConfigParser)
-	for k, v := range providerConfigParsers {
-		s.originalProviderConfigParsers[k] = v
-	}
+	s.originalProviderConfigRegistry = providerConfigRegistry
+	providerConfigRegistry = newExtendedConfigRegistry()
 }
 
 func (s *ProviderConfigSuite) TearDownTest() {
-	providerConfigParsers = make(map[string]ProviderConfigParser)
-	for k, v := range s.originalProviderConfigParsers {
-		providerConfigParsers[k] = v
-	}
+	providerConfigRegistry = s.originalProviderConfigRegistry
 }
 
 type ProviderConfigForTest struct {
@@ -35,7 +30,7 @@ type ProviderConfigForTest struct {
 	IntProp  int    `toml:"int_prop"`
 }
 
-var _ ProviderConfig = (*ProviderConfigForTest)(nil)
+var _ Extended = (*ProviderConfigForTest)(nil)
 
 func (p *ProviderConfigForTest) Validate() error {
 	if p.StrProp == "force-error" {
@@ -44,7 +39,7 @@ func (p *ProviderConfigForTest) Validate() error {
 	return nil
 }
 
-func providerConfigForTestParser(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
+func providerConfigForTestParser(_ context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
 	var providerConfigForTest ProviderConfigForTest
 	if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {
 		return nil, err
@@ -133,7 +128,7 @@ func (s *ProviderConfigSuite) TestReadConfigUnregisteredProviderConfig() {
 }
 
 func (s *ProviderConfigSuite) TestReadConfigParserError() {
-	RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
+	RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
 		return nil, errors.New("parser error forced by test")
 	})
 	invalidConfigPath := s.writeConfig(`
@@ -156,7 +151,7 @@ func (s *ProviderConfigSuite) TestReadConfigParserError() {
 
 func (s *ProviderConfigSuite) TestConfigDirPathInContext() {
 	var capturedDirPath string
-	RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
+	RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
 		capturedDirPath = ConfigDirPathFromContext(ctx)
 		var providerConfigForTest ProviderConfigForTest
 		if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {

pkg/config/toolset_config.go

@@ -1,34 +1,7 @@
 package config
 
-import (
-	"context"
-	"fmt"
+var toolsetConfigRegistry = newExtendedConfigRegistry()
 
-	"github.com/BurntSushi/toml"
-)
-
-// ToolsetConfig is the interface that all toolset-specific configurations must implement.
-// Each toolset registers a factory function to parse its config from TOML primitives
-type ToolsetConfig interface {
-	Validate() error
-}
-
-type ToolsetConfigParser func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (ToolsetConfig, error)
-
-var (
-	toolsetConfigParsers = make(map[string]ToolsetConfigParser)
-)
-
-func RegisterToolsetConfig(name string, parser ToolsetConfigParser) {
-	if _, exists := toolsetConfigParsers[name]; exists {
-		panic(fmt.Sprintf("toolset config parser already registered for toolset '%s'", name))
-	}
-
-	toolsetConfigParsers[name] = parser
-}
-
-func getToolsetConfigParser(name string) (ToolsetConfigParser, bool) {
-	parser, ok := toolsetConfigParsers[name]
-
-	return parser, ok
+func RegisterToolsetConfig(name string, parser ExtendedConfigParser) {
+	toolsetConfigRegistry.register(name, parser)
 }