diff --git a/pkg/kubernetes/accesscontrol_clientset.go b/pkg/kubernetes/accesscontrol_client_set.go similarity index 86% rename from pkg/kubernetes/accesscontrol_clientset.go rename to pkg/kubernetes/accesscontrol_client_set.go index e871bd96..abafb75c 100644 --- a/pkg/kubernetes/accesscontrol_clientset.go +++ b/pkg/kubernetes/accesscontrol_client_set.go @@ -15,6 +15,7 @@ import ( corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" + "k8s.io/client-go/tools/clientcmd" metricsv1beta1 "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1" ) @@ -22,18 +23,21 @@ import ( // Only a limited set of functions are implemented with a single point of access to the kubernetes API where // apiVersion and kinds are checked for allowed access type AccessControlClientset struct { - cfg *rest.Config kubernetes.Interface + staticConfig *config.StaticConfig + clientCmdConfig clientcmd.ClientConfig + cfg *rest.Config restMapper meta.ResettableRESTMapper discoveryClient discovery.CachedDiscoveryInterface dynamicClient dynamic.Interface metricsV1beta1 *metricsv1beta1.MetricsV1beta1Client } -func NewAccessControlClientset(staticConfig *config.StaticConfig, restConfig *rest.Config) (*AccessControlClientset, error) { - rest.CopyConfig(restConfig) +func NewAccessControlClientset(staticConfig *config.StaticConfig, clientCmdConfig clientcmd.ClientConfig, restConfig *rest.Config) (*AccessControlClientset, error) { acc := &AccessControlClientset{ - cfg: rest.CopyConfig(restConfig), + staticConfig: staticConfig, + clientCmdConfig: clientCmdConfig, + cfg: rest.CopyConfig(restConfig), } if acc.cfg.UserAgent == "" { acc.cfg.UserAgent = rest.DefaultKubernetesUserAgent() @@ -111,3 +115,8 @@ func (a *AccessControlClientset) SelfSubjectAccessReviews() (authorizationv1.Sel func (a *AccessControlClientset) TokenReview() (authenticationv1.TokenReviewInterface, error) { return a.AuthenticationV1().TokenReviews(), nil } + +// ToRawKubeConfigLoader returns the clientcmd.ClientConfig object (genericclioptions.RESTClientGetter) +func (a *AccessControlClientset) ToRawKubeConfigLoader() clientcmd.ClientConfig { + return a.clientCmdConfig +} diff --git a/pkg/kubernetes/configuration.go b/pkg/kubernetes/configuration.go index 71fd2dd2..8983eb34 100644 --- a/pkg/kubernetes/configuration.go +++ b/pkg/kubernetes/configuration.go @@ -32,13 +32,16 @@ func IsInCluster(cfg *config.StaticConfig) bool { } func (k *Kubernetes) NamespaceOrDefault(namespace string) string { - return k.manager.NamespaceOrDefault(namespace) + if namespace == "" { + return k.configuredNamespace() + } + return namespace } // ConfigurationContextsDefault returns the current context name // TODO: Should be moved to the Provider level ? func (k *Kubernetes) ConfigurationContextsDefault() (string, error) { - cfg, err := k.manager.clientCmdConfig.RawConfig() + cfg, err := k.ToRawKubeConfigLoader().RawConfig() if err != nil { return "", err } @@ -48,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) { - cfg, err := k.manager.clientCmdConfig.RawConfig() + cfg, err := k.ToRawKubeConfigLoader().RawConfig() if err != nil { return nil, err } @@ -71,7 +74,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 cfg, err = k.manager.clientCmdConfig.RawConfig(); err != nil { + if cfg, err = k.ToRawKubeConfigLoader().RawConfig(); err != nil { return nil, err } if minify { diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index 03bb9758..78296c54 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -1,9 +1,13 @@ package kubernetes import ( + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" "github.com/containers/kubernetes-mcp-server/pkg/helm" "github.com/containers/kubernetes-mcp-server/pkg/kiali" @@ -20,26 +24,52 @@ const ( type CloseWatchKubeConfig func() error +var Scheme = scheme.Scheme +var ParameterCodec = runtime.NewParameterCodec(Scheme) + type Kubernetes struct { - manager *Manager + accessControlClientSet *AccessControlClientset } +var _ helm.Kubernetes = (*Kubernetes)(nil) + // AccessControlClientset returns the access-controlled clientset // This ensures that any denied resources configured in the system are properly enforced func (k *Kubernetes) AccessControlClientset() *AccessControlClientset { - return k.manager.accessControlClientSet + return k.accessControlClientSet } -var Scheme = scheme.Scheme -var ParameterCodec = runtime.NewParameterCodec(Scheme) - func (k *Kubernetes) NewHelm() *helm.Helm { // This is a derived Kubernetes, so it already has the Helm initialized - return helm.NewHelm(k.manager) + return helm.NewHelm(k) } // NewKiali returns a Kiali client initialized with the same StaticConfig and bearer token // as the underlying derived Kubernetes manager. func (k *Kubernetes) NewKiali() *kiali.Kiali { - return kiali.NewKiali(k.manager.staticConfig, k.AccessControlClientset().cfg) + return kiali.NewKiali(k.AccessControlClientset().staticConfig, k.AccessControlClientset().cfg) +} + +func (k *Kubernetes) configuredNamespace() string { + if ns, _, nsErr := k.AccessControlClientset().ToRawKubeConfigLoader().Namespace(); nsErr == nil { + return ns + } + return "" +} + +func (k *Kubernetes) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + return k.AccessControlClientset().DiscoveryClient(), nil +} + +func (k *Kubernetes) ToRESTMapper() (meta.RESTMapper, error) { + return k.AccessControlClientset().RESTMapper(), nil +} + +// ToRESTConfig returns the rest.Config object (genericclioptions.RESTClientGetter) +func (k *Kubernetes) ToRESTConfig() (*rest.Config, error) { + return k.AccessControlClientset().cfg, nil +} + +func (k *Kubernetes) ToRawKubeConfigLoader() clientcmd.ClientConfig { + return k.AccessControlClientset().ToRawKubeConfigLoader() } diff --git a/pkg/kubernetes/kubernetes_derived_test.go b/pkg/kubernetes/kubernetes_derived_test.go index 88a39da3..485fef02 100644 --- a/pkg/kubernetes/kubernetes_derived_test.go +++ b/pkg/kubernetes/kubernetes_derived_test.go @@ -10,6 +10,7 @@ import ( "github.com/containers/kubernetes-mcp-server/internal/test" "github.com/containers/kubernetes-mcp-server/pkg/config" "github.com/stretchr/testify/suite" + "k8s.io/client-go/tools/clientcmd" ) type DerivedTestSuite struct { @@ -46,46 +47,46 @@ users: testStaticConfig := test.Must(config.ReadToml([]byte(` kubeconfig = "` + strings.ReplaceAll(kubeconfigPath, `\`, `\\`) + `" `))) - s.Run("without authorization header returns original manager", func() { + s.Run("without authorization header returns original clientset", func() { testManager, err := NewKubeconfigManager(testStaticConfig, "") s.Require().NoErrorf(err, "failed to create test manager: %v", err) s.T().Cleanup(testManager.Close) derived, err := testManager.Derived(s.T().Context()) - s.Require().NoErrorf(err, "failed to create derived manager: %v", err) + s.Require().NoErrorf(err, "failed to create derived kubernetes: %v", err) - s.Equal(derived.manager, testManager, "expected original manager, got different manager") + s.Equal(derived.AccessControlClientset(), testManager.accessControlClientset, "expected original clientset, got different clientset") }) - s.Run("with invalid authorization header returns original manager", func() { + s.Run("with invalid authorization header returns original clientset", func() { testManager, err := NewKubeconfigManager(testStaticConfig, "") s.Require().NoErrorf(err, "failed to create test manager: %v", err) s.T().Cleanup(testManager.Close) ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "invalid-token") derived, err := testManager.Derived(ctx) - s.Require().NoErrorf(err, "failed to create derived manager: %v", err) + s.Require().NoErrorf(err, "failed to create derived kubernetes: %v", err) - s.Equal(derived.manager, testManager, "expected original manager, got different manager") + s.Equal(derived.AccessControlClientset(), testManager.accessControlClientset, "expected original clientset, got different clientset") }) - s.Run("with valid bearer token creates derived manager with correct configuration", func() { + s.Run("with valid bearer token creates derived kubernetes with correct configuration", func() { testManager, err := NewKubeconfigManager(testStaticConfig, "") s.Require().NoErrorf(err, "failed to create test manager: %v", err) s.T().Cleanup(testManager.Close) ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "Bearer aiTana-julIA") derived, err := testManager.Derived(ctx) - s.Require().NoErrorf(err, "failed to create derived manager: %v", err) + s.Require().NoErrorf(err, "failed to create derived kubernetes: %v", err) - s.NotEqual(derived.manager, testManager, "expected new derived manager, got original manager") - s.Equal(derived.manager.staticConfig, testStaticConfig, "staticConfig not properly wired to derived manager") + s.NotEqual(derived.AccessControlClientset(), testManager.accessControlClientset, "expected new derived clientset, got original clientset") + s.Equal(derived.AccessControlClientset().staticConfig, testStaticConfig, "staticConfig not properly wired to derived clientset") s.Run("RestConfig is correctly copied and sensitive fields are omitted", func() { - derivedCfg := derived.manager.accessControlClientSet.cfg + derivedCfg := derived.AccessControlClientset().cfg s.Require().NotNil(derivedCfg, "derived config is nil") - originalCfg := testManager.accessControlClientSet.cfg + originalCfg := testManager.accessControlClientset.cfg s.Equalf(originalCfg.Host, derivedCfg.Host, "expected Host %s, got %s", originalCfg.Host, derivedCfg.Host) s.Equalf(originalCfg.APIPath, derivedCfg.APIPath, "expected APIPath %s, got %s", originalCfg.APIPath, derivedCfg.APIPath) s.Equalf(originalCfg.QPS, derivedCfg.QPS, "expected QPS %f, got %f", originalCfg.QPS, derivedCfg.QPS) @@ -119,17 +120,140 @@ users: s.Falsef(originalCfg.Username == "" && originalCfg.Password == "", "original kubeconfig shouldn't be modified") }) - s.Run("derived manager has initialized clients", func() { - // Verify that the derived manager has proper clients initialized + s.Run("derived kubernetes has ClientConfig properly wired", func() { + // Verify that the derived kubernetes has proper ClientConfig initialized + s.NotNilf(derived.AccessControlClientset().ToRawKubeConfigLoader(), "expected ToRawKubeConfigLoader to be initialized") + derivedClientCmdApiConfig, err := derived.AccessControlClientset().ToRawKubeConfigLoader().RawConfig() + s.Require().NoErrorf(err, "failed to get derived clientCmdApiConfig: %v", err) + s.Equalf("test-context", derivedClientCmdApiConfig.CurrentContext, "expected CurrentContext %s, got %s", "test-context", derivedClientCmdApiConfig.CurrentContext) + s.Equalf(1, len(derivedClientCmdApiConfig.Clusters), "expected 1 cluster, got %d", len(derivedClientCmdApiConfig.Clusters)) + s.Equalf(1, len(derivedClientCmdApiConfig.Contexts), "expected 1 context, got %d", len(derivedClientCmdApiConfig.Contexts)) + s.Emptyf(derivedClientCmdApiConfig.AuthInfos, "expected 0 authInfos, got %d", len(derivedClientCmdApiConfig.AuthInfos)) + }) + s.Run("derived kubernetes has initialized clients", func() { + // Verify that the derived kubernetes has proper clients initialized s.NotNilf(derived.AccessControlClientset(), "expected accessControlClientSet to be initialized") - s.Equalf(testStaticConfig, derived.manager.staticConfig, "staticConfig not properly wired to derived manager") - s.NotNilf(derived.AccessControlClientset().DiscoveryClient(), "expected discoveryClient to be initialized") + s.Equalf(testStaticConfig, derived.AccessControlClientset().staticConfig, "staticConfig not properly wired to derived clientset") s.NotNilf(derived.AccessControlClientset().RESTMapper(), "expected accessControlRESTMapper to be initialized") + s.NotNilf(derived.AccessControlClientset().DiscoveryClient(), "expected discoveryClient to be initialized") s.NotNilf(derived.AccessControlClientset().DynamicClient(), "expected dynamicClient to be initialized") + s.NotNilf(derived.AccessControlClientset().MetricsV1beta1Client(), "expected metricsV1beta1Client to be initialized") }) }) }) + s.Run("with no RequireOAuth (default) and RawConfig error", func() { + testStaticConfig := test.Must(config.ReadToml([]byte(` + kubeconfig = "` + strings.ReplaceAll(kubeconfigPath, `\`, `\\`) + `" + `))) + + s.Run("with bearer token but RawConfig fails returns original clientset", func() { + testManager, err := NewKubeconfigManager(testStaticConfig, "") + s.Require().NoErrorf(err, "failed to create test manager: %v", err) + s.T().Cleanup(testManager.Close) + + // Corrupt the clientCmdConfig by setting it to a config that will fail on RawConfig() + // We'll do this by creating a config with an invalid file path + badKubeconfigPath := filepath.Join(s.T().TempDir(), "nonexistent", "config") + badConfig := test.Must(config.ReadToml([]byte(` + kubeconfig = "` + strings.ReplaceAll(badKubeconfigPath, `\`, `\\`) + `" + `))) + badManager, _ := newManager(badConfig, testManager.accessControlClientset.cfg, testManager.accessControlClientset.clientCmdConfig) + // Replace the clientCmdConfig with one that will fail + pathOptions := clientcmd.NewDefaultPathOptions() + pathOptions.LoadingRules.ExplicitPath = badKubeconfigPath + badClientCmdConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( + pathOptions.LoadingRules, + &clientcmd.ConfigOverrides{}) + badManager.accessControlClientset.clientCmdConfig = badClientCmdConfig + + ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "Bearer aiTana-julIA") + derived, err := badManager.Derived(ctx) + s.Require().NoErrorf(err, "expected no error when RequireOAuth=false, got: %v", err) + s.Equal(derived.AccessControlClientset(), badManager.accessControlClientset, "expected original clientset when RawConfig fails and RequireOAuth=false") + }) + }) + + s.Run("with RequireOAuth=true and RawConfig error", func() { + badKubeconfigPath := filepath.Join(s.T().TempDir(), "nonexistent", "config") + testStaticConfig := test.Must(config.ReadToml([]byte(` + kubeconfig = "` + strings.ReplaceAll(badKubeconfigPath, `\`, `\\`) + `" + require_oauth = true + `))) + + s.Run("with bearer token but RawConfig fails returns error", func() { + // First create a working manager + workingKubeconfigPath := filepath.Join(s.T().TempDir(), "working-config") + err := os.WriteFile(workingKubeconfigPath, []byte(kubeconfigContent), 0644) + s.Require().NoError(err) + workingConfig := test.Must(config.ReadToml([]byte(` + kubeconfig = "` + strings.ReplaceAll(workingKubeconfigPath, `\`, `\\`) + `" + `))) + testManager, err := NewKubeconfigManager(workingConfig, "") + s.Require().NoErrorf(err, "failed to create test manager: %v", err) + s.T().Cleanup(testManager.Close) + + // Now create a bad manager with RequireOAuth=true + badManager, _ := newManager(testStaticConfig, testManager.accessControlClientset.cfg, testManager.accessControlClientset.clientCmdConfig) + // Replace the clientCmdConfig with one that will fail + pathOptions := clientcmd.NewDefaultPathOptions() + pathOptions.LoadingRules.ExplicitPath = badKubeconfigPath + badClientCmdConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( + pathOptions.LoadingRules, + &clientcmd.ConfigOverrides{}) + badManager.accessControlClientset.clientCmdConfig = badClientCmdConfig + + ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "Bearer aiTana-julIA") + derived, err := badManager.Derived(ctx) + s.Require().Error(err, "expected error when RawConfig fails and RequireOAuth=true") + s.ErrorContains(err, "failed to get kubeconfig", "expected error containing 'failed to get kubeconfig'") + s.Nil(derived, "expected nil derived kubernetes when RawConfig fails and RequireOAuth=true") + }) + }) + + s.Run("with no RequireOAuth (default) and NewAccessControlClientset error", func() { + testStaticConfig := test.Must(config.ReadToml([]byte(` + kubeconfig = "` + strings.ReplaceAll(kubeconfigPath, `\`, `\\`) + `" + `))) + + s.Run("with bearer token but invalid rest config returns original clientset", func() { + testManager, err := NewKubeconfigManager(testStaticConfig, "") + s.Require().NoErrorf(err, "failed to create test manager: %v", err) + s.T().Cleanup(testManager.Close) + + // Corrupt the rest config to make NewAccessControlClientset fail + // Setting an invalid Host URL should cause client creation to fail + testManager.accessControlClientset.cfg.Host = "://invalid-url" + + ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "Bearer aiTana-julIA") + derived, err := testManager.Derived(ctx) + s.Require().NoErrorf(err, "expected no error when RequireOAuth=false, got: %v", err) + s.Equal(derived.accessControlClientSet, testManager.accessControlClientset, "expected original clientset when NewAccessControlClientset fails and RequireOAuth=false") + }) + }) + + s.Run("with RequireOAuth=true and NewAccessControlClientset error", func() { + testStaticConfig := test.Must(config.ReadToml([]byte(` + kubeconfig = "` + strings.ReplaceAll(kubeconfigPath, `\`, `\\`) + `" + require_oauth = true + `))) + + s.Run("with bearer token but invalid rest config returns error", func() { + testManager, err := NewKubeconfigManager(testStaticConfig, "") + s.Require().NoErrorf(err, "failed to create test manager: %v", err) + s.T().Cleanup(testManager.Close) + + // Corrupt the rest config to make NewAccessControlClientset fail + testManager.accessControlClientset.cfg.Host = "://invalid-url" + + ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "Bearer aiTana-julIA") + derived, err := testManager.Derived(ctx) + s.Require().Error(err, "expected error when NewAccessControlClientset fails and RequireOAuth=true") + s.ErrorContains(err, "failed to create derived clientset", "expected error containing 'failed to create derived clientset'") + s.Nil(derived, "expected nil derived kubernetes when NewAccessControlClientset fails and RequireOAuth=true") + }) + }) + s.Run("with RequireOAuth=true", func() { testStaticConfig := test.Must(config.ReadToml([]byte(` kubeconfig = "` + strings.ReplaceAll(kubeconfigPath, `\`, `\\`) + `" @@ -144,7 +268,7 @@ users: derived, err := testManager.Derived(s.T().Context()) s.Require().Error(err, "expected error for missing oauth token, got nil") s.EqualError(err, "oauth token required", "expected error 'oauth token required', got %s", err.Error()) - s.Nil(derived, "expected nil derived manager when oauth token required") + s.Nil(derived, "expected nil derived kubernetes when oauth token required") }) s.Run("with invalid authorization header returns oauth token required error", func() { @@ -156,22 +280,22 @@ users: derived, err := testManager.Derived(ctx) s.Require().Error(err, "expected error for invalid oauth token, got nil") s.EqualError(err, "oauth token required", "expected error 'oauth token required', got %s", err.Error()) - s.Nil(derived, "expected nil derived manager when oauth token required") + s.Nil(derived, "expected nil derived kubernetes when oauth token required") }) - s.Run("with valid bearer token creates derived manager", func() { + s.Run("with valid bearer token creates derived kubernetes", func() { testManager, err := NewKubeconfigManager(testStaticConfig, "") s.Require().NoErrorf(err, "failed to create test manager: %v", err) s.T().Cleanup(testManager.Close) ctx := context.WithValue(s.T().Context(), HeaderKey("Authorization"), "Bearer aiTana-julIA") derived, err := testManager.Derived(ctx) - s.Require().NoErrorf(err, "failed to create derived manager: %v", err) + s.Require().NoErrorf(err, "failed to create derived kubernetes: %v", err) - s.NotEqual(derived.manager, testManager, "expected new derived manager, got original manager") - s.Equal(derived.manager.staticConfig, testStaticConfig, "staticConfig not properly wired to derived manager") + s.NotEqual(derived.AccessControlClientset(), testManager.accessControlClientset, "expected new derived clientset, got original clientset") + s.Equal(derived.AccessControlClientset().staticConfig, testStaticConfig, "staticConfig not properly wired to derived clientset") - derivedCfg := derived.manager.accessControlClientSet.cfg + derivedCfg := derived.AccessControlClientset().cfg s.Require().NotNil(derivedCfg, "derived config is nil") s.Equalf("aiTana-julIA", derivedCfg.BearerToken, "expected BearerToken %s, got %s", "aiTana-julIA", derivedCfg.BearerToken) diff --git a/pkg/kubernetes/manager.go b/pkg/kubernetes/manager.go index 32bd278e..dd3444ea 100644 --- a/pkg/kubernetes/manager.go +++ b/pkg/kubernetes/manager.go @@ -7,12 +7,9 @@ import ( "strings" "github.com/containers/kubernetes-mcp-server/pkg/config" - "github.com/containers/kubernetes-mcp-server/pkg/helm" "github.com/fsnotify/fsnotify" authenticationv1api "k8s.io/api/authentication/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/discovery" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -20,14 +17,12 @@ import ( ) type Manager struct { - clientCmdConfig clientcmd.ClientConfig - accessControlClientSet *AccessControlClientset + accessControlClientset *AccessControlClientset staticConfig *config.StaticConfig CloseWatchKubeConfig CloseWatchKubeConfig } -var _ helm.Kubernetes = (*Manager)(nil) var _ Openshift = (*Manager)(nil) var ( @@ -92,16 +87,24 @@ func NewInClusterManager(config *config.StaticConfig) (*Manager, error) { } func newManager(config *config.StaticConfig, restConfig *rest.Config, clientCmdConfig clientcmd.ClientConfig) (*Manager, error) { + if config == nil { + return nil, errors.New("config cannot be nil") + } + if restConfig == nil { + return nil, errors.New("restConfig cannot be nil") + } + if clientCmdConfig == nil { + return nil, errors.New("clientCmdConfig cannot be nil") + } k8s := &Manager{ - staticConfig: config, - clientCmdConfig: clientCmdConfig, + staticConfig: config, } var err error // TODO: Won't work because not all client-go clients use the shared context (e.g. discovery client uses context.TODO()) //k8s.cfg.Wrap(func(original http.RoundTripper) http.RoundTripper { // return &impersonateRoundTripper{original} //}) - k8s.accessControlClientSet, err = NewAccessControlClientset(k8s.staticConfig, restConfig) + k8s.accessControlClientset, err = NewAccessControlClientset(k8s.staticConfig, clientCmdConfig, restConfig) if err != nil { return nil, err } @@ -109,10 +112,7 @@ func newManager(config *config.StaticConfig, restConfig *rest.Config, clientCmdC } func (m *Manager) WatchKubeConfig(onKubeConfigChange func() error) { - if m.clientCmdConfig == nil { - return - } - kubeConfigFiles := m.clientCmdConfig.ConfigAccess().GetLoadingPrecedence() + kubeConfigFiles := m.accessControlClientset.ToRawKubeConfigLoader().ConfigAccess().GetLoadingPrecedence() if len(kubeConfigFiles) == 0 { return } @@ -150,40 +150,8 @@ func (m *Manager) Close() { } } -func (m *Manager) configuredNamespace() string { - if ns, _, nsErr := m.clientCmdConfig.Namespace(); nsErr == nil { - return ns - } - return "" -} - -func (m *Manager) NamespaceOrDefault(namespace string) string { - if namespace == "" { - return m.configuredNamespace() - } - return namespace -} - -func (m *Manager) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { - return m.accessControlClientSet.DiscoveryClient(), nil -} - -func (m *Manager) ToRESTMapper() (meta.RESTMapper, error) { - return m.accessControlClientSet.RESTMapper(), nil -} - -// ToRESTConfig returns the rest.Config object (genericclioptions.RESTClientGetter) -func (m *Manager) ToRESTConfig() (*rest.Config, error) { - return m.accessControlClientSet.cfg, nil -} - -// ToRawKubeConfigLoader returns the clientcmd.ClientConfig object (genericclioptions.RESTClientGetter) -func (m *Manager) ToRawKubeConfigLoader() clientcmd.ClientConfig { - return m.clientCmdConfig -} - func (m *Manager) VerifyToken(ctx context.Context, token, audience string) (*authenticationv1api.UserInfo, []string, error) { - tokenReviewClient := m.accessControlClientSet.AuthenticationV1().TokenReviews() + tokenReviewClient := m.accessControlClientset.AuthenticationV1().TokenReviews() tokenReview := &authenticationv1api.TokenReview{ TypeMeta: metav1.TypeMeta{ APIVersion: "authentication.k8s.io/v1", @@ -216,50 +184,44 @@ func (m *Manager) Derived(ctx context.Context) (*Kubernetes, error) { if m.staticConfig.RequireOAuth { return nil, errors.New("oauth token required") } - return &Kubernetes{manager: m}, nil + return &Kubernetes{m.accessControlClientset}, nil } klog.V(5).Infof("%s header found (Bearer), using provided bearer token", OAuthAuthorizationHeader) derivedCfg := &rest.Config{ - Host: m.accessControlClientSet.cfg.Host, - APIPath: m.accessControlClientSet.cfg.APIPath, - WrapTransport: m.accessControlClientSet.cfg.WrapTransport, + Host: m.accessControlClientset.cfg.Host, + APIPath: m.accessControlClientset.cfg.APIPath, + WrapTransport: m.accessControlClientset.cfg.WrapTransport, // Copy only server verification TLS settings (CA bundle and server name) TLSClientConfig: rest.TLSClientConfig{ - Insecure: m.accessControlClientSet.cfg.Insecure, - ServerName: m.accessControlClientSet.cfg.ServerName, - CAFile: m.accessControlClientSet.cfg.CAFile, - CAData: m.accessControlClientSet.cfg.CAData, + Insecure: m.accessControlClientset.cfg.Insecure, + ServerName: m.accessControlClientset.cfg.ServerName, + CAFile: m.accessControlClientset.cfg.CAFile, + CAData: m.accessControlClientset.cfg.CAData, }, BearerToken: strings.TrimPrefix(authorization, "Bearer "), // pass custom UserAgent to identify the client UserAgent: CustomUserAgent, - QPS: m.accessControlClientSet.cfg.QPS, - Burst: m.accessControlClientSet.cfg.Burst, - Timeout: m.accessControlClientSet.cfg.Timeout, + QPS: m.accessControlClientset.cfg.QPS, + Burst: m.accessControlClientset.cfg.Burst, + Timeout: m.accessControlClientset.cfg.Timeout, Impersonate: rest.ImpersonationConfig{}, } - clientCmdApiConfig, err := m.clientCmdConfig.RawConfig() + clientCmdApiConfig, err := m.accessControlClientset.clientCmdConfig.RawConfig() if err != nil { if m.staticConfig.RequireOAuth { klog.Errorf("failed to get kubeconfig: %v", err) - return nil, errors.New("failed to get kubeconfig") + return nil, fmt.Errorf("failed to get kubeconfig: %w", err) } - return &Kubernetes{manager: m}, nil + return &Kubernetes{m.accessControlClientset}, nil } clientCmdApiConfig.AuthInfos = make(map[string]*clientcmdapi.AuthInfo) - derived := &Kubernetes{ - manager: &Manager{ - clientCmdConfig: clientcmd.NewDefaultClientConfig(clientCmdApiConfig, nil), - staticConfig: m.staticConfig, - }, - } - derived.manager.accessControlClientSet, err = NewAccessControlClientset(derived.manager.staticConfig, derivedCfg) + derived, err := NewAccessControlClientset(m.staticConfig, clientcmd.NewDefaultClientConfig(clientCmdApiConfig, nil), derivedCfg) if err != nil { if m.staticConfig.RequireOAuth { - klog.Errorf("failed to get kubeconfig: %v", err) - return nil, errors.New("failed to get kubeconfig") + klog.Errorf("failed to create derived clientset: %v", err) + return nil, fmt.Errorf("failed to create derived clientset: %w", err) } - return &Kubernetes{manager: m}, nil + return &Kubernetes{m.accessControlClientset}, nil } - return derived, nil + return &Kubernetes{derived}, nil } diff --git a/pkg/kubernetes/manager_test.go b/pkg/kubernetes/manager_test.go index c6f9da6a..6d8b6ee7 100644 --- a/pkg/kubernetes/manager_test.go +++ b/pkg/kubernetes/manager_test.go @@ -10,6 +10,7 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/config" "github.com/stretchr/testify/suite" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -44,12 +45,12 @@ func (s *ManagerTestSuite) TestNewInClusterManager() { s.Require().NoError(err) s.Require().NotNil(manager) s.Run("behaves as in cluster", func() { - rawConfig, err := manager.clientCmdConfig.RawConfig() + rawConfig, err := manager.accessControlClientset.ToRawKubeConfigLoader().RawConfig() s.Require().NoError(err) s.Equal("in-cluster", rawConfig.CurrentContext, "expected current context to be 'in-cluster'") }) s.Run("sets default user-agent", func() { - s.Contains(manager.accessControlClientSet.cfg.UserAgent, "("+runtime.GOOS+"/"+runtime.GOARCH+")") + s.Contains(manager.accessControlClientset.cfg.UserAgent, "("+runtime.GOOS+"/"+runtime.GOARCH+")") }) }) s.Run("with explicit kubeconfig", func() { @@ -89,19 +90,19 @@ func (s *ManagerTestSuite) TestNewKubeconfigManager() { s.Require().NoError(err) s.Require().NotNil(manager) s.Run("behaves as NOT in cluster", func() { - rawConfig, err := manager.clientCmdConfig.RawConfig() + rawConfig, err := manager.accessControlClientset.ToRawKubeConfigLoader().RawConfig() s.Require().NoError(err) s.NotEqual("in-cluster", rawConfig.CurrentContext, "expected current context to NOT be 'in-cluster'") s.Equal("fake-context", rawConfig.CurrentContext, "expected current context to be 'fake-context' as in kubeconfig") }) s.Run("loads correct config", func() { - s.Contains(manager.clientCmdConfig.ConfigAccess().GetLoadingPrecedence(), kubeconfig, "expected kubeconfig path to match") + s.Contains(manager.accessControlClientset.ToRawKubeConfigLoader().ConfigAccess().GetLoadingPrecedence(), kubeconfig, "expected kubeconfig path to match") }) s.Run("sets default user-agent", func() { - s.Contains(manager.accessControlClientSet.cfg.UserAgent, "("+runtime.GOOS+"/"+runtime.GOARCH+")") + s.Contains(manager.accessControlClientset.cfg.UserAgent, "("+runtime.GOOS+"/"+runtime.GOARCH+")") }) s.Run("rest config host points to mock server", func() { - s.Equal(s.mockServer.Config().Host, manager.accessControlClientSet.cfg.Host, "expected rest config host to match mock server") + s.Equal(s.mockServer.Config().Host, manager.accessControlClientset.cfg.Host, "expected rest config host to match mock server") }) }) s.Run("with valid kubeconfig in env and explicit kubeconfig in config", func() { @@ -114,17 +115,17 @@ func (s *ManagerTestSuite) TestNewKubeconfigManager() { s.Require().NoError(err) s.Require().NotNil(manager) s.Run("behaves as NOT in cluster", func() { - rawConfig, err := manager.clientCmdConfig.RawConfig() + rawConfig, err := manager.accessControlClientset.ToRawKubeConfigLoader().RawConfig() s.Require().NoError(err) s.NotEqual("in-cluster", rawConfig.CurrentContext, "expected current context to NOT be 'in-cluster'") s.Equal("fake-context", rawConfig.CurrentContext, "expected current context to be 'fake-context' as in kubeconfig") }) s.Run("loads correct config (explicit)", func() { - s.NotContains(manager.clientCmdConfig.ConfigAccess().GetLoadingPrecedence(), kubeconfigInEnv, "expected kubeconfig path to NOT match env") - s.Contains(manager.clientCmdConfig.ConfigAccess().GetLoadingPrecedence(), kubeconfigExplicit, "expected kubeconfig path to match explicit") + s.NotContains(manager.accessControlClientset.ToRawKubeConfigLoader().ConfigAccess().GetLoadingPrecedence(), kubeconfigInEnv, "expected kubeconfig path to NOT match env") + s.Contains(manager.accessControlClientset.ToRawKubeConfigLoader().ConfigAccess().GetLoadingPrecedence(), kubeconfigExplicit, "expected kubeconfig path to match explicit") }) s.Run("rest config host points to mock server", func() { - s.Equal(s.mockServer.Config().Host, manager.accessControlClientSet.cfg.Host, "expected rest config host to match mock server") + s.Equal(s.mockServer.Config().Host, manager.accessControlClientset.cfg.Host, "expected rest config host to match mock server") }) }) s.Run("with valid kubeconfig in env and explicit kubeconfig context (valid)", func() { @@ -140,16 +141,16 @@ func (s *ManagerTestSuite) TestNewKubeconfigManager() { s.Require().NoError(err) s.Require().NotNil(manager) s.Run("behaves as NOT in cluster", func() { - rawConfig, err := manager.clientCmdConfig.RawConfig() + rawConfig, err := manager.accessControlClientset.ToRawKubeConfigLoader().RawConfig() s.Require().NoError(err) s.NotEqual("in-cluster", rawConfig.CurrentContext, "expected current context to NOT be 'in-cluster'") s.Equal("not-the-mock-server", rawConfig.CurrentContext, "expected current context to be 'not-the-mock-server' as in explicit context") }) s.Run("loads correct config", func() { - s.Contains(manager.clientCmdConfig.ConfigAccess().GetLoadingPrecedence(), kubeconfigFile, "expected kubeconfig path to match") + s.Contains(manager.accessControlClientset.ToRawKubeConfigLoader().ConfigAccess().GetLoadingPrecedence(), kubeconfigFile, "expected kubeconfig path to match") }) s.Run("rest config host points to mock server", func() { - s.Equal(s.mockServer.Config().Host, manager.accessControlClientSet.cfg.Host, "expected rest config host to match mock server") + s.Equal(s.mockServer.Config().Host, manager.accessControlClientset.cfg.Host, "expected rest config host to match mock server") }) }) s.Run("with valid kubeconfig in env and explicit kubeconfig context (invalid)", func() { @@ -197,6 +198,36 @@ func (s *ManagerTestSuite) TestNewKubeconfigManager() { }) } +func (s *ManagerTestSuite) TestNewManager() { + s.Run("with nil config returns error", func() { + manager, err := newManager(nil, &rest.Config{}, clientcmd.NewDefaultClientConfig(clientcmdapi.Config{}, nil)) + s.Require().Error(err) + s.EqualError(err, "config cannot be nil", "expected 'config cannot be nil' error") + s.Nil(manager, "expected nil manager when config is nil") + }) + + s.Run("with nil restConfig returns error", func() { + manager, err := newManager(&config.StaticConfig{}, nil, clientcmd.NewDefaultClientConfig(clientcmdapi.Config{}, nil)) + s.Require().Error(err) + s.EqualError(err, "restConfig cannot be nil", "expected 'restConfig cannot be nil' error") + s.Nil(manager, "expected nil manager when restConfig is nil") + }) + + s.Run("with nil clientCmdConfig returns error", func() { + manager, err := newManager(&config.StaticConfig{}, &rest.Config{}, nil) + s.Require().Error(err) + s.EqualError(err, "clientCmdConfig cannot be nil", "expected 'clientCmdConfig cannot be nil' error") + s.Nil(manager, "expected nil manager when clientCmdConfig is nil") + }) + + s.Run("with all nil parameters returns config error first", func() { + manager, err := newManager(nil, nil, nil) + s.Require().Error(err) + s.EqualError(err, "config cannot be nil", "expected 'config cannot be nil' error as first check") + s.Nil(manager, "expected nil manager when all parameters are nil") + }) +} + func TestManager(t *testing.T) { suite.Run(t, new(ManagerTestSuite)) } diff --git a/pkg/kubernetes/provider_kubeconfig.go b/pkg/kubernetes/provider_kubeconfig.go index 9ab055c8..e70f0a6c 100644 --- a/pkg/kubernetes/provider_kubeconfig.go +++ b/pkg/kubernetes/provider_kubeconfig.go @@ -40,7 +40,7 @@ func newKubeConfigClusterProvider(cfg *config.StaticConfig) (Provider, error) { return nil, err } - rawConfig, err := m.clientCmdConfig.RawConfig() + rawConfig, err := m.accessControlClientset.clientCmdConfig.RawConfig() if err != nil { return nil, err } diff --git a/pkg/kubernetes/resources.go b/pkg/kubernetes/resources.go index c73cc0f4..4f25052e 100644 --- a/pkg/kubernetes/resources.go +++ b/pkg/kubernetes/resources.go @@ -38,7 +38,7 @@ func (k *Kubernetes) ResourcesList(ctx context.Context, gvk *schema.GroupVersion // Check if operation is allowed for all namespaces (applicable for namespaced resources) isNamespaced, _ := k.isNamespaced(gvk) if isNamespaced && !k.canIUse(ctx, gvr, namespace, "list") && namespace == "" { - namespace = k.manager.configuredNamespace() + namespace = k.configuredNamespace() } if options.AsTable { return k.resourcesListAsTable(ctx, gvk, gvr, namespace, options)