Skip to content

Commit aeb75d7

Browse files
committed
refactor(config)!: extract config interfaces to pkg/api/config
Decouples the pkg/kubernetes module from pkg/config by introducing interfaces for configuration access. This allows pkg/kubernetes to depend on abstractions rather than the concrete StaticConfig type. Changes: - Add AuthProvider, ClusterProvider, DeniedResourcesProvider, and ExtendedProvider interfaces in pkg/api/config - Move cluster provider strategy constants to pkg/api/config - Move GroupVersionKind struct to pkg/api/config - Update pkg/kubernetes to use the new interfaces - StaticConfig now implements the Config interface Signed-off-by: Marc Nuri <[email protected]>
1 parent c9804bf commit aeb75d7

27 files changed

+208
-142
lines changed

pkg/api/config/auth.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package config
2+
3+
type AuthProvider interface {
4+
// IsRequireOAuth indicates whether OAuth authentication is required.
5+
IsRequireOAuth() bool
6+
}

pkg/api/config/cluster_provider.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package config
2+
3+
const (
4+
ClusterProviderKubeConfig = "kubeconfig"
5+
ClusterProviderInCluster = "in-cluster"
6+
ClusterProviderDisabled = "disabled"
7+
)
8+
9+
type ClusterProvider interface {
10+
// GetClusterProviderStrategy returns the cluster provider strategy (if configured).
11+
GetClusterProviderStrategy() string
12+
// GetKubeConfigPath returns the path to the kubeconfig file (if configured).
13+
GetKubeConfigPath() string
14+
}

pkg/api/config/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package config
2+
3+
type Config interface {
4+
AuthProvider
5+
ClusterProvider
6+
DeniedResourcesProvider
7+
ExtendedProvider
8+
}

pkg/api/config/extended.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package config
2+
3+
// Extended is the interface that all configuration extensions must implement.
4+
// Each extended config manager registers a factory function to parse its config from TOML primitives
5+
type Extended interface {
6+
// Validate validates the extended configuration. Returns an error if the configuration is invalid.
7+
Validate() error
8+
}
9+
10+
type ExtendedProvider interface {
11+
// GetProviderConfig returns the extended configuration for the given provider strategy.
12+
// The boolean return value indicates whether the configuration was found.
13+
GetProviderConfig(strategy string) (Extended, bool)
14+
// GetToolsetConfig returns the extended configuration for the given toolset name.
15+
// The boolean return value indicates whether the configuration was found.
16+
GetToolsetConfig(name string) (Extended, bool)
17+
}

pkg/api/config/resources.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package config
2+
3+
type GroupVersionKind struct {
4+
Group string `json:"group" toml:"group"`
5+
Version string `json:"version" toml:"version"`
6+
Kind string `json:"kind,omitempty" toml:"kind,omitempty"`
7+
}
8+
9+
type DeniedResourcesProvider interface {
10+
// GetDeniedResources returns a list of GroupVersionKinds that are denied.
11+
GetDeniedResources() []GroupVersionKind
12+
}

pkg/config/config.go

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,18 @@ import (
1010
"strings"
1111

1212
"github.com/BurntSushi/toml"
13+
configapi "github.com/containers/kubernetes-mcp-server/pkg/api/config"
1314
"k8s.io/klog/v2"
1415
)
1516

1617
const (
1718
DefaultDropInConfigDir = "conf.d"
1819
)
1920

20-
const (
21-
ClusterProviderKubeConfig = "kubeconfig"
22-
ClusterProviderInCluster = "in-cluster"
23-
ClusterProviderDisabled = "disabled"
24-
)
25-
2621
// StaticConfig is the configuration for the server.
2722
// It allows to configure server specific settings and tools to be enabled or disabled.
2823
type StaticConfig struct {
29-
DeniedResources []GroupVersionKind `toml:"denied_resources"`
24+
DeniedResources []configapi.GroupVersionKind `toml:"denied_resources"`
3025

3126
LogLevel int `toml:"log_level,omitzero"`
3227
Port string `toml:"port,omitempty"`
@@ -80,19 +75,15 @@ type StaticConfig struct {
8075
ToolsetConfigs map[string]toml.Primitive `toml:"toolset_configs,omitempty"`
8176

8277
// Internal: parsed provider configs (not exposed to TOML package)
83-
parsedClusterProviderConfigs map[string]Extended
78+
parsedClusterProviderConfigs map[string]configapi.Extended
8479
// Internal: parsed toolset configs (not exposed to TOML package)
85-
parsedToolsetConfigs map[string]Extended
80+
parsedToolsetConfigs map[string]configapi.Extended
8681

8782
// Internal: the config.toml directory, to help resolve relative file paths
8883
configDirPath string
8984
}
9085

91-
type GroupVersionKind struct {
92-
Group string `toml:"group"`
93-
Version string `toml:"version"`
94-
Kind string `toml:"kind,omitempty"`
95-
}
86+
var _ configapi.Config = (*StaticConfig)(nil)
9687

9788
type ReadConfigOpt func(cfg *StaticConfig)
9889

@@ -292,13 +283,29 @@ func ReadToml(configData []byte, opts ...ReadConfigOpt) (*StaticConfig, error) {
292283
return config, nil
293284
}
294285

295-
func (c *StaticConfig) GetProviderConfig(strategy string) (Extended, bool) {
296-
config, ok := c.parsedClusterProviderConfigs[strategy]
286+
func (c *StaticConfig) GetClusterProviderStrategy() string {
287+
return c.ClusterProviderStrategy
288+
}
289+
290+
func (c *StaticConfig) GetDeniedResources() []configapi.GroupVersionKind {
291+
return c.DeniedResources
292+
}
297293

298-
return config, ok
294+
func (c *StaticConfig) GetKubeConfigPath() string {
295+
return c.KubeConfig
299296
}
300297

301-
func (c *StaticConfig) GetToolsetConfig(name string) (Extended, bool) {
298+
func (c *StaticConfig) GetProviderConfig(strategy string) (configapi.Extended, bool) {
299+
cfg, ok := c.parsedClusterProviderConfigs[strategy]
300+
301+
return cfg, ok
302+
}
303+
304+
func (c *StaticConfig) GetToolsetConfig(name string) (configapi.Extended, bool) {
302305
cfg, ok := c.parsedToolsetConfigs[name]
303306
return cfg, ok
304307
}
308+
309+
func (c *StaticConfig) IsRequireOAuth() bool {
310+
return c.RequireOAuth
311+
}

pkg/config/config_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99
"testing"
1010

11+
configapi "github.com/containers/kubernetes-mcp-server/pkg/api/config"
1112
"github.com/stretchr/testify/suite"
1213
)
1314

@@ -136,11 +137,11 @@ func (s *ConfigSuite) TestReadConfigValid() {
136137
s.Run("denied_resources", func() {
137138
s.Require().Lenf(config.DeniedResources, 2, "Expected 2 denied resources, got %d", len(config.DeniedResources))
138139
s.Run("contains apps/v1/Deployment", func() {
139-
s.Contains(config.DeniedResources, GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
140+
s.Contains(config.DeniedResources, configapi.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
140141
"Expected denied resources to contain apps/v1/Deployment")
141142
})
142143
s.Run("contains rbac.authorization.k8s.io/v1/Role", func() {
143-
s.Contains(config.DeniedResources, GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "Role"},
144+
s.Contains(config.DeniedResources, configapi.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "Role"},
144145
"Expected denied resources to contain rbac.authorization.k8s.io/v1/Role")
145146
})
146147
})
@@ -777,16 +778,16 @@ func (s *ConfigSuite) TestDropInWithDeniedResources() {
777778

778779
s.Run("drop-in replaces denied_resources array", func() {
779780
s.Len(config.DeniedResources, 2, "denied_resources should have 2 entries from drop-in")
780-
s.Contains(config.DeniedResources, GroupVersionKind{
781+
s.Contains(config.DeniedResources, configapi.GroupVersionKind{
781782
Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole",
782783
})
783-
s.Contains(config.DeniedResources, GroupVersionKind{
784+
s.Contains(config.DeniedResources, configapi.GroupVersionKind{
784785
Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRoleBinding",
785786
})
786787
})
787788

788789
s.Run("original denied_resources from main config are replaced", func() {
789-
s.NotContains(config.DeniedResources, GroupVersionKind{
790+
s.NotContains(config.DeniedResources, configapi.GroupVersionKind{
790791
Group: "apps", Version: "v1", Kind: "Deployment",
791792
}, "original entry should be replaced by drop-in")
792793
})

pkg/config/extended.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,10 @@ import (
55
"fmt"
66

77
"github.com/BurntSushi/toml"
8+
configapi "github.com/containers/kubernetes-mcp-server/pkg/api/config"
89
)
910

10-
// Extended is the interface that all configuration extensions must implement.
11-
// Each extended config manager registers a factory function to parse its config from TOML primitives
12-
type Extended interface {
13-
Validate() error
14-
}
15-
16-
type ExtendedConfigParser func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error)
11+
type ExtendedConfigParser func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error)
1712

1813
type extendedConfigRegistry struct {
1914
parsers map[string]ExtendedConfigParser
@@ -33,11 +28,11 @@ func (r *extendedConfigRegistry) register(name string, parser ExtendedConfigPars
3328
r.parsers[name] = parser
3429
}
3530

36-
func (r *extendedConfigRegistry) parse(ctx context.Context, metaData toml.MetaData, configs map[string]toml.Primitive) (map[string]Extended, error) {
31+
func (r *extendedConfigRegistry) parse(ctx context.Context, metaData toml.MetaData, configs map[string]toml.Primitive) (map[string]configapi.Extended, error) {
3732
if len(configs) == 0 {
38-
return make(map[string]Extended), nil
33+
return make(map[string]configapi.Extended), nil
3934
}
40-
parsedConfigs := make(map[string]Extended, len(configs))
35+
parsedConfigs := make(map[string]configapi.Extended, len(configs))
4136

4237
for name, primitive := range configs {
4338
parser, ok := r.parsers[name]

pkg/config/provider_config_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/BurntSushi/toml"
11+
configapi "github.com/containers/kubernetes-mcp-server/pkg/api/config"
1112
"github.com/stretchr/testify/suite"
1213
)
1314

@@ -31,7 +32,7 @@ type ProviderConfigForTest struct {
3132
IntProp int `toml:"int_prop"`
3233
}
3334

34-
var _ Extended = (*ProviderConfigForTest)(nil)
35+
var _ configapi.Extended = (*ProviderConfigForTest)(nil)
3536

3637
func (p *ProviderConfigForTest) Validate() error {
3738
if p.StrProp == "force-error" {
@@ -40,7 +41,7 @@ func (p *ProviderConfigForTest) Validate() error {
4041
return nil
4142
}
4243

43-
func providerConfigForTestParser(_ context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
44+
func providerConfigForTestParser(_ context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
4445
var providerConfigForTest ProviderConfigForTest
4546
if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {
4647
return nil, err
@@ -129,7 +130,7 @@ func (s *ProviderConfigSuite) TestReadConfigUnregisteredProviderConfig() {
129130
}
130131

131132
func (s *ProviderConfigSuite) TestReadConfigParserError() {
132-
RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
133+
RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
133134
return nil, errors.New("parser error forced by test")
134135
})
135136
invalidConfigPath := s.writeConfig(`
@@ -152,7 +153,7 @@ func (s *ProviderConfigSuite) TestReadConfigParserError() {
152153

153154
func (s *ProviderConfigSuite) TestConfigDirPathInContext() {
154155
var capturedDirPath string
155-
RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
156+
RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
156157
capturedDirPath = ConfigDirPathFromContext(ctx)
157158
var providerConfigForTest ProviderConfigForTest
158159
if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {
@@ -328,7 +329,7 @@ func (s *ProviderConfigSuite) TestStandaloneConfigDirWithExtendedConfig() {
328329
func (s *ProviderConfigSuite) TestConfigDirPathInContextStandalone() {
329330
// Test that configDirPath is correctly set in context for standalone --config-dir
330331
var capturedDirPath string
331-
RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
332+
RegisterProviderConfig("test", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
332333
capturedDirPath = ConfigDirPathFromContext(ctx)
333334
var providerConfigForTest ProviderConfigForTest
334335
if err := md.PrimitiveDecode(primitive, &providerConfigForTest); err != nil {

pkg/config/toolset_config_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/BurntSushi/toml"
11+
configapi "github.com/containers/kubernetes-mcp-server/pkg/api/config"
1112
"github.com/stretchr/testify/suite"
1213
)
1314

@@ -31,7 +32,7 @@ type ToolsetConfigForTest struct {
3132
Timeout int `toml:"timeout"`
3233
}
3334

34-
var _ Extended = (*ToolsetConfigForTest)(nil)
35+
var _ configapi.Extended = (*ToolsetConfigForTest)(nil)
3536

3637
func (t *ToolsetConfigForTest) Validate() error {
3738
if t.Endpoint == "force-error" {
@@ -40,7 +41,7 @@ func (t *ToolsetConfigForTest) Validate() error {
4041
return nil
4142
}
4243

43-
func toolsetConfigForTestParser(_ context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
44+
func toolsetConfigForTestParser(_ context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
4445
var toolsetConfigForTest ToolsetConfigForTest
4546
if err := md.PrimitiveDecode(primitive, &toolsetConfigForTest); err != nil {
4647
return nil, err
@@ -127,7 +128,7 @@ func (s *ToolsetConfigSuite) TestReadConfigUnregisteredToolsetConfig() {
127128

128129
func (s *ToolsetConfigSuite) TestConfigDirPathInContext() {
129130
var capturedDirPath string
130-
RegisterToolsetConfig("test-toolset", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
131+
RegisterToolsetConfig("test-toolset", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
131132
capturedDirPath = ConfigDirPathFromContext(ctx)
132133
var toolsetConfigForTest ToolsetConfigForTest
133134
if err := md.PrimitiveDecode(primitive, &toolsetConfigForTest); err != nil {
@@ -299,7 +300,7 @@ func (s *ToolsetConfigSuite) TestStandaloneConfigDirWithExtendedConfig() {
299300
func (s *ToolsetConfigSuite) TestConfigDirPathInContextStandalone() {
300301
// Test that configDirPath is correctly set in context for standalone --config-dir
301302
var capturedDirPath string
302-
RegisterToolsetConfig("test-toolset", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (Extended, error) {
303+
RegisterToolsetConfig("test-toolset", func(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (configapi.Extended, error) {
303304
capturedDirPath = ConfigDirPathFromContext(ctx)
304305
var toolsetConfigForTest ToolsetConfigForTest
305306
if err := md.PrimitiveDecode(primitive, &toolsetConfigForTest); err != nil {

0 commit comments

Comments
 (0)