NO-JIRA: Sync downstream to include provider refactors

internal/test/env.go

@@ -0,0 +1,15 @@
+package test
+
+import (
+	"os"
+	"strings"
+)
+
+func RestoreEnv(originalEnv []string) {
+	os.Clearenv()
+	for _, env := range originalEnv {
+		if key, value, found := strings.Cut(env, "="); found {
+			_ = os.Setenv(key, value)
+		}
+	}
+}

pkg/config/config.go

@@ -1,6 +1,8 @@
 package config
 
 import (
+	"bytes"
+	"fmt"
 	"os"
 
 	"github.com/BurntSushi/toml"
@@ -59,8 +61,13 @@ type StaticConfig struct {
 	// If set to "kubeconfig", the clusters will be loaded from those in the kubeconfig.
 	// If set to "in-cluster", the server will use the in cluster config
 	ClusterProviderStrategy string `toml:"cluster_provider_strategy,omitempty"`
-	// ClusterContexts is which context should be used for each cluster
-	ClusterContexts map[string]string `toml:"cluster_contexts"`
+
+	// ClusterProvider-specific configurations
+	// This map holds raw TOML primitives that will be parsed by registered provider parsers
+	ClusterProviderConfigs map[string]toml.Primitive `toml:"cluster_provider_configs,omitempty"`
+
+	// Internal: parsed provider configs (not exposed to TOML package)
+	parsedClusterProviderConfigs map[string]ProviderConfig
 }
 
 func Default() *StaticConfig {
@@ -88,8 +95,46 @@ func Read(configPath string) (*StaticConfig, error) {
 // ReadToml reads the toml data and returns the StaticConfig.
 func ReadToml(configData []byte) (*StaticConfig, error) {
 	config := Default()
-	if err := toml.Unmarshal(configData, config); err != nil {
+	md, err := toml.NewDecoder(bytes.NewReader(configData)).Decode(config)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := config.parseClusterProviderConfigs(md); err != nil {
 		return nil, err
 	}
+
 	return config, nil
 }
+
+func (c *StaticConfig) GetProviderConfig(strategy string) (ProviderConfig, 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))
+	}
+
+	for strategy, primitive := range c.ClusterProviderConfigs {
+		parser, ok := getProviderConfigParser(strategy)
+		if !ok {
+			continue
+		}
+
+		providerConfig, err := parser(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
+}

pkg/config/config_test.go

@@ -11,10 +11,25 @@ import (
 	"github.com/stretchr/testify/suite"
 )
 
-type ConfigSuite struct {
+type BaseConfigSuite struct {
 	suite.Suite
 }
 
+func (s *BaseConfigSuite) writeConfig(content string) string {
+	s.T().Helper()
+	tempDir := s.T().TempDir()
+	path := filepath.Join(tempDir, "config.toml")
+	err := os.WriteFile(path, []byte(content), 0644)
+	if err != nil {
+		s.T().Fatalf("Failed to write config file %s: %v", path, err)
+	}
+	return path
+}
+
+type ConfigSuite struct {
+	BaseConfigSuite
+}
+
 func (s *ConfigSuite) TestReadConfigMissingFile() {
 	config, err := Read("non-existent-config.toml")
 	s.Run("returns error for missing file", func() {
@@ -159,17 +174,6 @@ func (s *ConfigSuite) TestReadConfigValidPreservesDefaultsForMissingFields() {
 	})
 }
 
-func (s *ConfigSuite) writeConfig(content string) string {
-	s.T().Helper()
-	tempDir := s.T().TempDir()
-	path := filepath.Join(tempDir, "config.toml")
-	err := os.WriteFile(path, []byte(content), 0644)
-	if err != nil {
-		s.T().Fatalf("Failed to write config file %s: %v", path, err)
-	}
-	return path
-}
-
 func TestConfig(t *testing.T) {
 	suite.Run(t, new(ConfigSuite))
 }

pkg/config/provider_config.go

@@ -0,0 +1,33 @@
+package config
+
+import (
+	"fmt"
+
+	"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(primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error)
+
+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
+}

pkg/config/provider_config_test.go

@@ -0,0 +1,157 @@
+package config
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/BurntSushi/toml"
+	"github.com/stretchr/testify/suite"
+)
+
+type ProviderConfigSuite struct {
+	BaseConfigSuite
+	originalProviderConfigParsers map[string]ProviderConfigParser
+}
+
+func (s *ProviderConfigSuite) SetupTest() {
+	s.originalProviderConfigParsers = make(map[string]ProviderConfigParser)
+	for k, v := range providerConfigParsers {
+		s.originalProviderConfigParsers[k] = v
+	}
+}
+
+func (s *ProviderConfigSuite) TearDownTest() {
+	providerConfigParsers = make(map[string]ProviderConfigParser)
+	for k, v := range s.originalProviderConfigParsers {
+		providerConfigParsers[k] = v
+	}
+}
+
+type ProviderConfigForTest struct {
+	BoolProp bool   `toml:"bool_prop"`
+	StrProp  string `toml:"str_prop"`
+	IntProp  int    `toml:"int_prop"`
+}
+
+var _ ProviderConfig = (*ProviderConfigForTest)(nil)
+
+func (p *ProviderConfigForTest) Validate() error {
+	if p.StrProp == "force-error" {
+		return errors.New("validation error forced by test")
+	}
+	return nil
+}
+
+func providerConfigForTestParser(primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
+	var providerConfigForTest ProviderConfigForTest
+	if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {
+		return nil, err
+	}
+	return &providerConfigForTest, nil
+}
+
+func (s *ProviderConfigSuite) TestRegisterProviderConfig() {
+	s.Run("panics when registering duplicate provider config parser", func() {
+		s.Panics(func() {
+			RegisterProviderConfig("test", providerConfigForTestParser)
+			RegisterProviderConfig("test", providerConfigForTestParser)
+		}, "Expected panic when registering duplicate provider config parser")
+	})
+}
+
+func (s *ProviderConfigSuite) TestReadConfigValid() {
+	RegisterProviderConfig("test", providerConfigForTestParser)
+	validConfigPath := s.writeConfig(`
+		cluster_provider_strategy = "test"
+		[cluster_provider_configs.test]
+		bool_prop = true
+		str_prop = "a string"
+		int_prop = 42
+	`)
+
+	config, err := Read(validConfigPath)
+	s.Run("returns no error for valid file with registered provider config", func() {
+		s.Require().NoError(err, "Expected no error for valid file, got %v", err)
+	})
+	s.Run("returns config for valid file with registered provider config", func() {
+		s.Require().NotNil(config, "Expected non-nil config for valid file")
+	})
+	s.Run("parses provider config correctly", func() {
+		providerConfig, ok := config.GetProviderConfig("test")
+		s.Require().True(ok, "Expected to find provider config for strategy 'test'")
+		s.Require().NotNil(providerConfig, "Expected non-nil provider config for strategy 'test'")
+		testProviderConfig, ok := providerConfig.(*ProviderConfigForTest)
+		s.Require().True(ok, "Expected provider config to be of type *ProviderConfigForTest")
+		s.Equal(true, testProviderConfig.BoolProp, "Expected BoolProp to be true")
+		s.Equal("a string", testProviderConfig.StrProp, "Expected StrProp to be 'a string'")
+		s.Equal(42, testProviderConfig.IntProp, "Expected IntProp to be 42")
+	})
+}
+
+func (s *ProviderConfigSuite) TestReadConfigInvalidProviderConfig() {
+	RegisterProviderConfig("test", providerConfigForTestParser)
+	invalidConfigPath := s.writeConfig(`
+		cluster_provider_strategy = "test"
+		[cluster_provider_configs.test]
+		bool_prop = true
+		str_prop = "force-error"
+		int_prop = 42
+	`)
+
+	config, err := Read(invalidConfigPath)
+	s.Run("returns error for invalid provider config", func() {
+		s.Require().NotNil(err, "Expected error for invalid provider config, got nil")
+		s.ErrorContains(err, "validation error forced by test", "Expected validation error from provider config")
+	})
+	s.Run("returns nil config for invalid provider config", func() {
+		s.Nil(config, "Expected nil config for invalid provider config")
+	})
+}
+
+func (s *ProviderConfigSuite) TestReadConfigUnregisteredProviderConfig() {
+	invalidConfigPath := s.writeConfig(`
+		cluster_provider_strategy = "unregistered"
+		[cluster_provider_configs.unregistered]
+		bool_prop = true
+		str_prop = "a string"
+		int_prop = 42
+	`)
+
+	config, err := Read(invalidConfigPath)
+	s.Run("returns no error for unregistered provider config", func() {
+		s.Require().NoError(err, "Expected no error for unregistered provider config, got %v", err)
+	})
+	s.Run("returns config for unregistered provider config", func() {
+		s.Require().NotNil(config, "Expected non-nil config for unregistered provider config")
+	})
+	s.Run("does not parse unregistered provider config", func() {
+		_, ok := config.GetProviderConfig("unregistered")
+		s.Require().False(ok, "Expected no provider config for unregistered strategy")
+	})
+}
+
+func (s *ProviderConfigSuite) TestReadConfigParserError() {
+	RegisterProviderConfig("test", func(primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
+		return nil, errors.New("parser error forced by test")
+	})
+	invalidConfigPath := s.writeConfig(`
+		cluster_provider_strategy = "test"
+		[cluster_provider_configs.test]
+		bool_prop = true
+		str_prop = "a string"
+		int_prop = 42
+	`)
+
+	config, err := Read(invalidConfigPath)
+	s.Run("returns error for provider config parser error", func() {
+		s.Require().NotNil(err, "Expected error for provider config parser error, got nil")
+		s.ErrorContains(err, "parser error forced by test", "Expected parser error from provider config")
+	})
+	s.Run("returns nil config for provider config parser error", func() {
+		s.Nil(config, "Expected nil config for provider config parser error")
+	})
+}
+
+func TestProviderConfig(t *testing.T) {
+	suite.Run(t, new(ProviderConfigSuite))
+}

pkg/kubernetes/configuration.go

@@ -1,9 +1,9 @@
 package kubernetes
 
 import (
+	"github.com/containers/kubernetes-mcp-server/pkg/config"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	"k8s.io/client-go/tools/clientcmd/api/latest"
 )
@@ -22,29 +22,13 @@ var InClusterConfig = func() (*rest.Config, error) {
 	return inClusterConfig, err
 }
 
-// resolveKubernetesConfigurations resolves the required kubernetes configurations and sets them in the Kubernetes struct
-func resolveKubernetesConfigurations(kubernetes *Manager) error {
-	// Always set clientCmdConfig
-	pathOptions := clientcmd.NewDefaultPathOptions()
-	if kubernetes.staticConfig.KubeConfig != "" {
-		pathOptions.LoadingRules.ExplicitPath = kubernetes.staticConfig.KubeConfig
+func IsInCluster(cfg *config.StaticConfig) bool {
+	// Even if running in-cluster, if a kubeconfig is provided, we consider it as out-of-cluster
+	if cfg != nil && cfg.KubeConfig != "" {
+		return false
 	}
-	kubernetes.clientCmdConfig = clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
-		pathOptions.LoadingRules,
-		&clientcmd.ConfigOverrides{ClusterInfo: clientcmdapi.Cluster{Server: ""}})
-	var err error
-	if kubernetes.IsInCluster() {
-		kubernetes.cfg, err = InClusterConfig()
-		if err == nil && kubernetes.cfg != nil {
-			return nil
-		}
-	}
-	// Out of cluster
-	kubernetes.cfg, err = kubernetes.clientCmdConfig.ClientConfig()
-	if kubernetes.cfg != nil && kubernetes.cfg.UserAgent == "" {
-		kubernetes.cfg.UserAgent = rest.DefaultKubernetesUserAgent()
-	}
-	return err
+	restConfig, err := InClusterConfig()
+	return err == nil && restConfig != nil
 }
 
 func (k *Kubernetes) NamespaceOrDefault(namespace string) string {
@@ -54,7 +38,7 @@ func (k *Kubernetes) NamespaceOrDefault(namespace string) string {
 // ConfigurationContextsDefault returns the current context name
 // TODO: Should be moved to the Provider level ?
 func (k *Kubernetes) ConfigurationContextsDefault() (string, error) {
-	if k.manager.IsInCluster() {
+	if k.manager.inCluster {
 		return inClusterKubeConfigDefaultContext, nil
 	}
 	cfg, err := k.manager.clientCmdConfig.RawConfig()
@@ -67,7 +51,7 @@ func (k *Kubernetes) ConfigurationContextsDefault() (string, error) {
 // ConfigurationContextsList returns the list of available context names
 // TODO: Should be moved to the Provider level ?
 func (k *Kubernetes) ConfigurationContextsList() (map[string]string, error) {
-	if k.manager.IsInCluster() {
+	if k.manager.inCluster {
 		return map[string]string{inClusterKubeConfigDefaultContext: ""}, nil
 	}
 	cfg, err := k.manager.clientCmdConfig.RawConfig()
@@ -93,7 +77,7 @@ func (k *Kubernetes) ConfigurationContextsList() (map[string]string, error) {
 func (k *Kubernetes) ConfigurationView(minify bool) (runtime.Object, error) {
 	var cfg clientcmdapi.Config
 	var err error
-	if k.manager.IsInCluster() {
+	if k.manager.inCluster {
 		cfg = *clientcmdapi.NewConfig()
 		cfg.Clusters["cluster"] = &clientcmdapi.Cluster{
 			Server:                k.manager.cfg.Host,