Skip to content

Commit b5bc635

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request containers#59 from Cali0707/sync-downstream
NO-JIRA: Sync downstream to include provider refactors
2 parents 80d6c70 + 107265f commit b5bc635

16 files changed

+836
-316
lines changed

‎internal/test/env.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package test
2+
3+
import (
4+
"os"
5+
"strings"
6+
)
7+
8+
func RestoreEnv(originalEnv []string) {
9+
os.Clearenv()
10+
for _, env := range originalEnv {
11+
if key, value, found := strings.Cut(env, "="); found {
12+
_ = os.Setenv(key, value)
13+
}
14+
}
15+
}

‎pkg/config/config.go‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package config
22

33
import (
4+
"bytes"
5+
"fmt"
46
"os"
57

68
"github.com/BurntSushi/toml"
@@ -59,8 +61,13 @@ type StaticConfig struct {
5961
// If set to "kubeconfig", the clusters will be loaded from those in the kubeconfig.
6062
// If set to "in-cluster", the server will use the in cluster config
6163
ClusterProviderStrategy string `toml:"cluster_provider_strategy,omitempty"`
62-
// ClusterContexts is which context should be used for each cluster
63-
ClusterContexts map[string]string `toml:"cluster_contexts"`
64+
65+
// ClusterProvider-specific configurations
66+
// This map holds raw TOML primitives that will be parsed by registered provider parsers
67+
ClusterProviderConfigs map[string]toml.Primitive `toml:"cluster_provider_configs,omitempty"`
68+
69+
// Internal: parsed provider configs (not exposed to TOML package)
70+
parsedClusterProviderConfigs map[string]ProviderConfig
6471
}
6572

6673
func Default() *StaticConfig {
@@ -88,8 +95,46 @@ func Read(configPath string) (*StaticConfig, error) {
8895
// ReadToml reads the toml data and returns the StaticConfig.
8996
func ReadToml(configData []byte) (*StaticConfig, error) {
9097
config := Default()
91-
if err := toml.Unmarshal(configData, config); err != nil {
98+
md, err := toml.NewDecoder(bytes.NewReader(configData)).Decode(config)
99+
if err != nil {
100+
return nil, err
101+
}
102+
103+
if err := config.parseClusterProviderConfigs(md); err != nil {
92104
return nil, err
93105
}
106+
94107
return config, nil
95108
}
109+
110+
func (c *StaticConfig) GetProviderConfig(strategy string) (ProviderConfig, bool) {
111+
config, ok := c.parsedClusterProviderConfigs[strategy]
112+
113+
return config, ok
114+
}
115+
116+
func (c *StaticConfig) parseClusterProviderConfigs(md toml.MetaData) error {
117+
if c.parsedClusterProviderConfigs == nil {
118+
c.parsedClusterProviderConfigs = make(map[string]ProviderConfig, len(c.ClusterProviderConfigs))
119+
}
120+
121+
for strategy, primitive := range c.ClusterProviderConfigs {
122+
parser, ok := getProviderConfigParser(strategy)
123+
if !ok {
124+
continue
125+
}
126+
127+
providerConfig, err := parser(primitive, md)
128+
if err != nil {
129+
return fmt.Errorf("failed to parse config for ClusterProvider '%s': %w", strategy, err)
130+
}
131+
132+
if err := providerConfig.Validate(); err != nil {
133+
return fmt.Errorf("invalid config file for ClusterProvider '%s': %w", strategy, err)
134+
}
135+
136+
c.parsedClusterProviderConfigs[strategy] = providerConfig
137+
}
138+
139+
return nil
140+
}

‎pkg/config/config_test.go‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,25 @@ import (
1111
"github.com/stretchr/testify/suite"
1212
)
1313

14-
type ConfigSuite struct {
14+
type BaseConfigSuite struct {
1515
suite.Suite
1616
}
1717

18+
func (s *BaseConfigSuite) writeConfig(content string) string {
19+
s.T().Helper()
20+
tempDir := s.T().TempDir()
21+
path := filepath.Join(tempDir, "config.toml")
22+
err := os.WriteFile(path, []byte(content), 0644)
23+
if err != nil {
24+
s.T().Fatalf("Failed to write config file %s: %v", path, err)
25+
}
26+
return path
27+
}
28+
29+
type ConfigSuite struct {
30+
BaseConfigSuite
31+
}
32+
1833
func (s *ConfigSuite) TestReadConfigMissingFile() {
1934
config, err := Read("non-existent-config.toml")
2035
s.Run("returns error for missing file", func() {
@@ -159,17 +174,6 @@ func (s *ConfigSuite) TestReadConfigValidPreservesDefaultsForMissingFields() {
159174
})
160175
}
161176

162-
func (s *ConfigSuite) writeConfig(content string) string {
163-
s.T().Helper()
164-
tempDir := s.T().TempDir()
165-
path := filepath.Join(tempDir, "config.toml")
166-
err := os.WriteFile(path, []byte(content), 0644)
167-
if err != nil {
168-
s.T().Fatalf("Failed to write config file %s: %v", path, err)
169-
}
170-
return path
171-
}
172-
173177
func TestConfig(t *testing.T) {
174178
suite.Run(t, new(ConfigSuite))
175179
}

‎pkg/config/provider_config.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package config
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/BurntSushi/toml"
7+
)
8+
9+
// ProviderConfig is the interface that all provider-specific configurations must implement.
10+
// Each provider registers a factory function to parse its config from TOML primitives
11+
type ProviderConfig interface {
12+
Validate() error
13+
}
14+
15+
type ProviderConfigParser func(primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error)
16+
17+
var (
18+
providerConfigParsers = make(map[string]ProviderConfigParser)
19+
)
20+
21+
func RegisterProviderConfig(strategy string, parser ProviderConfigParser) {
22+
if _, exists := providerConfigParsers[strategy]; exists {
23+
panic(fmt.Sprintf("provider config parser already registered for strategy '%s'", strategy))
24+
}
25+
26+
providerConfigParsers[strategy] = parser
27+
}
28+
29+
func getProviderConfigParser(strategy string) (ProviderConfigParser, bool) {
30+
provider, ok := providerConfigParsers[strategy]
31+
32+
return provider, ok
33+
}

‎pkg/config/provider_config_test.go‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package config
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/BurntSushi/toml"
8+
"github.com/stretchr/testify/suite"
9+
)
10+
11+
type ProviderConfigSuite struct {
12+
BaseConfigSuite
13+
originalProviderConfigParsers map[string]ProviderConfigParser
14+
}
15+
16+
func (s *ProviderConfigSuite) SetupTest() {
17+
s.originalProviderConfigParsers = make(map[string]ProviderConfigParser)
18+
for k, v := range providerConfigParsers {
19+
s.originalProviderConfigParsers[k] = v
20+
}
21+
}
22+
23+
func (s *ProviderConfigSuite) TearDownTest() {
24+
providerConfigParsers = make(map[string]ProviderConfigParser)
25+
for k, v := range s.originalProviderConfigParsers {
26+
providerConfigParsers[k] = v
27+
}
28+
}
29+
30+
type ProviderConfigForTest struct {
31+
BoolProp bool `toml:"bool_prop"`
32+
StrProp string `toml:"str_prop"`
33+
IntProp int `toml:"int_prop"`
34+
}
35+
36+
var _ ProviderConfig = (*ProviderConfigForTest)(nil)
37+
38+
func (p *ProviderConfigForTest) Validate() error {
39+
if p.StrProp == "force-error" {
40+
return errors.New("validation error forced by test")
41+
}
42+
return nil
43+
}
44+
45+
func providerConfigForTestParser(primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
46+
var providerConfigForTest ProviderConfigForTest
47+
if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {
48+
return nil, err
49+
}
50+
return &providerConfigForTest, nil
51+
}
52+
53+
func (s *ProviderConfigSuite) TestRegisterProviderConfig() {
54+
s.Run("panics when registering duplicate provider config parser", func() {
55+
s.Panics(func() {
56+
RegisterProviderConfig("test", providerConfigForTestParser)
57+
RegisterProviderConfig("test", providerConfigForTestParser)
58+
}, "Expected panic when registering duplicate provider config parser")
59+
})
60+
}
61+
62+
func (s *ProviderConfigSuite) TestReadConfigValid() {
63+
RegisterProviderConfig("test", providerConfigForTestParser)
64+
validConfigPath := s.writeConfig(`
65+
cluster_provider_strategy = "test"
66+
[cluster_provider_configs.test]
67+
bool_prop = true
68+
str_prop = "a string"
69+
int_prop = 42
70+
`)
71+
72+
config, err := Read(validConfigPath)
73+
s.Run("returns no error for valid file with registered provider config", func() {
74+
s.Require().NoError(err, "Expected no error for valid file, got %v", err)
75+
})
76+
s.Run("returns config for valid file with registered provider config", func() {
77+
s.Require().NotNil(config, "Expected non-nil config for valid file")
78+
})
79+
s.Run("parses provider config correctly", func() {
80+
providerConfig, ok := config.GetProviderConfig("test")
81+
s.Require().True(ok, "Expected to find provider config for strategy 'test'")
82+
s.Require().NotNil(providerConfig, "Expected non-nil provider config for strategy 'test'")
83+
testProviderConfig, ok := providerConfig.(*ProviderConfigForTest)
84+
s.Require().True(ok, "Expected provider config to be of type *ProviderConfigForTest")
85+
s.Equal(true, testProviderConfig.BoolProp, "Expected BoolProp to be true")
86+
s.Equal("a string", testProviderConfig.StrProp, "Expected StrProp to be 'a string'")
87+
s.Equal(42, testProviderConfig.IntProp, "Expected IntProp to be 42")
88+
})
89+
}
90+
91+
func (s *ProviderConfigSuite) TestReadConfigInvalidProviderConfig() {
92+
RegisterProviderConfig("test", providerConfigForTestParser)
93+
invalidConfigPath := s.writeConfig(`
94+
cluster_provider_strategy = "test"
95+
[cluster_provider_configs.test]
96+
bool_prop = true
97+
str_prop = "force-error"
98+
int_prop = 42
99+
`)
100+
101+
config, err := Read(invalidConfigPath)
102+
s.Run("returns error for invalid provider config", func() {
103+
s.Require().NotNil(err, "Expected error for invalid provider config, got nil")
104+
s.ErrorContains(err, "validation error forced by test", "Expected validation error from provider config")
105+
})
106+
s.Run("returns nil config for invalid provider config", func() {
107+
s.Nil(config, "Expected nil config for invalid provider config")
108+
})
109+
}
110+
111+
func (s *ProviderConfigSuite) TestReadConfigUnregisteredProviderConfig() {
112+
invalidConfigPath := s.writeConfig(`
113+
cluster_provider_strategy = "unregistered"
114+
[cluster_provider_configs.unregistered]
115+
bool_prop = true
116+
str_prop = "a string"
117+
int_prop = 42
118+
`)
119+
120+
config, err := Read(invalidConfigPath)
121+
s.Run("returns no error for unregistered provider config", func() {
122+
s.Require().NoError(err, "Expected no error for unregistered provider config, got %v", err)
123+
})
124+
s.Run("returns config for unregistered provider config", func() {
125+
s.Require().NotNil(config, "Expected non-nil config for unregistered provider config")
126+
})
127+
s.Run("does not parse unregistered provider config", func() {
128+
_, ok := config.GetProviderConfig("unregistered")
129+
s.Require().False(ok, "Expected no provider config for unregistered strategy")
130+
})
131+
}
132+
133+
func (s *ProviderConfigSuite) TestReadConfigParserError() {
134+
RegisterProviderConfig("test", func(primitive toml.Primitive, md toml.MetaData) (ProviderConfig, error) {
135+
return nil, errors.New("parser error forced by test")
136+
})
137+
invalidConfigPath := s.writeConfig(`
138+
cluster_provider_strategy = "test"
139+
[cluster_provider_configs.test]
140+
bool_prop = true
141+
str_prop = "a string"
142+
int_prop = 42
143+
`)
144+
145+
config, err := Read(invalidConfigPath)
146+
s.Run("returns error for provider config parser error", func() {
147+
s.Require().NotNil(err, "Expected error for provider config parser error, got nil")
148+
s.ErrorContains(err, "parser error forced by test", "Expected parser error from provider config")
149+
})
150+
s.Run("returns nil config for provider config parser error", func() {
151+
s.Nil(config, "Expected nil config for provider config parser error")
152+
})
153+
}
154+
155+
func TestProviderConfig(t *testing.T) {
156+
suite.Run(t, new(ProviderConfigSuite))
157+
}

‎pkg/kubernetes/configuration.go‎

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package kubernetes
22

33
import (
4+
"github.com/containers/kubernetes-mcp-server/pkg/config"
45
"k8s.io/apimachinery/pkg/runtime"
56
"k8s.io/client-go/rest"
6-
"k8s.io/client-go/tools/clientcmd"
77
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
88
"k8s.io/client-go/tools/clientcmd/api/latest"
99
)
@@ -22,29 +22,13 @@ var InClusterConfig = func() (*rest.Config, error) {
2222
return inClusterConfig, err
2323
}
2424

25-
// resolveKubernetesConfigurations resolves the required kubernetes configurations and sets them in the Kubernetes struct
26-
func resolveKubernetesConfigurations(kubernetes *Manager) error {
27-
// Always set clientCmdConfig
28-
pathOptions := clientcmd.NewDefaultPathOptions()
29-
if kubernetes.staticConfig.KubeConfig != "" {
30-
pathOptions.LoadingRules.ExplicitPath = kubernetes.staticConfig.KubeConfig
25+
func IsInCluster(cfg *config.StaticConfig) bool {
26+
// Even if running in-cluster, if a kubeconfig is provided, we consider it as out-of-cluster
27+
if cfg != nil && cfg.KubeConfig != "" {
28+
return false
3129
}
32-
kubernetes.clientCmdConfig = clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
33-
pathOptions.LoadingRules,
34-
&clientcmd.ConfigOverrides{ClusterInfo: clientcmdapi.Cluster{Server: ""}})
35-
var err error
36-
if kubernetes.IsInCluster() {
37-
kubernetes.cfg, err = InClusterConfig()
38-
if err == nil && kubernetes.cfg != nil {
39-
return nil
40-
}
41-
}
42-
// Out of cluster
43-
kubernetes.cfg, err = kubernetes.clientCmdConfig.ClientConfig()
44-
if kubernetes.cfg != nil && kubernetes.cfg.UserAgent == "" {
45-
kubernetes.cfg.UserAgent = rest.DefaultKubernetesUserAgent()
46-
}
47-
return err
30+
restConfig, err := InClusterConfig()
31+
return err == nil && restConfig != nil
4832
}
4933

5034
func (k *Kubernetes) NamespaceOrDefault(namespace string) string {
@@ -54,7 +38,7 @@ func (k *Kubernetes) NamespaceOrDefault(namespace string) string {
5438
// ConfigurationContextsDefault returns the current context name
5539
// TODO: Should be moved to the Provider level ?
5640
func (k *Kubernetes) ConfigurationContextsDefault() (string, error) {
57-
if k.manager.IsInCluster() {
41+
if k.manager.inCluster {
5842
return inClusterKubeConfigDefaultContext, nil
5943
}
6044
cfg, err := k.manager.clientCmdConfig.RawConfig()
@@ -67,7 +51,7 @@ func (k *Kubernetes) ConfigurationContextsDefault() (string, error) {
6751
// ConfigurationContextsList returns the list of available context names
6852
// TODO: Should be moved to the Provider level ?
6953
func (k *Kubernetes) ConfigurationContextsList() (map[string]string, error) {
70-
if k.manager.IsInCluster() {
54+
if k.manager.inCluster {
7155
return map[string]string{inClusterKubeConfigDefaultContext: ""}, nil
7256
}
7357
cfg, err := k.manager.clientCmdConfig.RawConfig()
@@ -93,7 +77,7 @@ func (k *Kubernetes) ConfigurationContextsList() (map[string]string, error) {
9377
func (k *Kubernetes) ConfigurationView(minify bool) (runtime.Object, error) {
9478
var cfg clientcmdapi.Config
9579
var err error
96-
if k.manager.IsInCluster() {
80+
if k.manager.inCluster {
9781
cfg = *clientcmdapi.NewConfig()
9882
cfg.Clusters["cluster"] = &clientcmdapi.Cluster{
9983
Server: k.manager.cfg.Host,

0 commit comments

Comments
 (0)