Skip to content

Commit f138b06

Browse files
manusamanusa
authored
refactor(kubernetes): force usage of Derived kubernetes (125)
refactor(kubernetes): force usage of Derived kubernetes Prevents consumers of the kubernetes package the usage of public methods on a non-derived config instance. --- review(kubernetes): force usage of Derived kubernetes Addresses comment by ardaguclu
1 parent 4a3ff2f commit f138b06

File tree

9 files changed

+120
-101
lines changed

9 files changed

+120
-101
lines changed

‎pkg/kubernetes/configuration.go‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ var InClusterConfig = func() (*rest.Config, error) {
2121
}
2222

2323
// resolveKubernetesConfigurations resolves the required kubernetes configurations and sets them in the Kubernetes struct
24-
func resolveKubernetesConfigurations(kubernetes *Kubernetes) error {
24+
func resolveKubernetesConfigurations(kubernetes *Manager) error {
2525
// Always set clientCmdConfig
2626
pathOptions := clientcmd.NewDefaultPathOptions()
2727
if kubernetes.Kubeconfig != "" {
@@ -45,56 +45,60 @@ func resolveKubernetesConfigurations(kubernetes *Kubernetes) error {
4545
return err
4646
}
4747

48-
func (k *Kubernetes) IsInCluster() bool {
49-
if k.Kubeconfig != "" {
48+
func (m *Manager) IsInCluster() bool {
49+
if m.Kubeconfig != "" {
5050
return false
5151
}
5252
cfg, err := InClusterConfig()
5353
return err == nil && cfg != nil
5454
}
5555

56-
func (k *Kubernetes) configuredNamespace() string {
57-
if ns, _, nsErr := k.clientCmdConfig.Namespace(); nsErr == nil {
56+
func (m *Manager) configuredNamespace() string {
57+
if ns, _, nsErr := m.clientCmdConfig.Namespace(); nsErr == nil {
5858
return ns
5959
}
6060
return ""
6161
}
6262

63-
func (k *Kubernetes) NamespaceOrDefault(namespace string) string {
63+
func (m *Manager) NamespaceOrDefault(namespace string) string {
6464
if namespace == "" {
65-
return k.configuredNamespace()
65+
return m.configuredNamespace()
6666
}
6767
return namespace
6868
}
6969

70+
func (k *Kubernetes) NamespaceOrDefault(namespace string) string {
71+
return k.manager.NamespaceOrDefault(namespace)
72+
}
73+
7074
// ToRESTConfig returns the rest.Config object (genericclioptions.RESTClientGetter)
71-
func (k *Kubernetes) ToRESTConfig() (*rest.Config, error) {
72-
return k.cfg, nil
75+
func (m *Manager) ToRESTConfig() (*rest.Config, error) {
76+
return m.cfg, nil
7377
}
7478

7579
// ToRawKubeConfigLoader returns the clientcmd.ClientConfig object (genericclioptions.RESTClientGetter)
76-
func (k *Kubernetes) ToRawKubeConfigLoader() clientcmd.ClientConfig {
77-
return k.clientCmdConfig
80+
func (m *Manager) ToRawKubeConfigLoader() clientcmd.ClientConfig {
81+
return m.clientCmdConfig
7882
}
7983

80-
func (k *Kubernetes) ConfigurationView(minify bool) (runtime.Object, error) {
84+
func (m *Manager) ConfigurationView(minify bool) (runtime.Object, error) {
8185
var cfg clientcmdapi.Config
8286
var err error
83-
if k.IsInCluster() {
87+
if m.IsInCluster() {
8488
cfg = *clientcmdapi.NewConfig()
8589
cfg.Clusters["cluster"] = &clientcmdapi.Cluster{
86-
Server: k.cfg.Host,
87-
InsecureSkipTLSVerify: k.cfg.Insecure,
90+
Server: m.cfg.Host,
91+
InsecureSkipTLSVerify: m.cfg.Insecure,
8892
}
8993
cfg.AuthInfos["user"] = &clientcmdapi.AuthInfo{
90-
Token: k.cfg.BearerToken,
94+
Token: m.cfg.BearerToken,
9195
}
9296
cfg.Contexts["context"] = &clientcmdapi.Context{
9397
Cluster: "cluster",
9498
AuthInfo: "user",
9599
}
96100
cfg.CurrentContext = "context"
97-
} else if cfg, err = k.clientCmdConfig.RawConfig(); err != nil {
101+
} else if cfg, err = m.clientCmdConfig.RawConfig(); err != nil {
98102
return nil, err
99103
}
100104
if minify {

‎pkg/kubernetes/configuration_test.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import (
1212

1313
func TestKubernetes_IsInCluster(t *testing.T) {
1414
t.Run("with explicit kubeconfig", func(t *testing.T) {
15-
k := Kubernetes{
15+
m := Manager{
1616
Kubeconfig: "kubeconfig",
1717
}
18-
if k.IsInCluster() {
18+
if m.IsInCluster() {
1919
t.Errorf("expected not in cluster, got in cluster")
2020
}
2121
})
@@ -27,10 +27,10 @@ func TestKubernetes_IsInCluster(t *testing.T) {
2727
defer func() {
2828
InClusterConfig = originalFunction
2929
}()
30-
k := Kubernetes{
30+
m := Manager{
3131
Kubeconfig: "",
3232
}
33-
if !k.IsInCluster() {
33+
if !m.IsInCluster() {
3434
t.Errorf("expected in cluster, got not in cluster")
3535
}
3636
})
@@ -42,10 +42,10 @@ func TestKubernetes_IsInCluster(t *testing.T) {
4242
defer func() {
4343
InClusterConfig = originalFunction
4444
}()
45-
k := Kubernetes{
45+
m := Manager{
4646
Kubeconfig: "",
4747
}
48-
if k.IsInCluster() {
48+
if m.IsInCluster() {
4949
t.Errorf("expected not in cluster, got in cluster")
5050
}
5151
})
@@ -57,10 +57,10 @@ func TestKubernetes_IsInCluster(t *testing.T) {
5757
defer func() {
5858
InClusterConfig = originalFunction
5959
}()
60-
k := Kubernetes{
60+
m := Manager{
6161
Kubeconfig: "",
6262
}
63-
if k.IsInCluster() {
63+
if m.IsInCluster() {
6464
t.Errorf("expected not in cluster, got in cluster")
6565
}
6666
})
@@ -72,8 +72,8 @@ func TestKubernetes_ResolveKubernetesConfigurations_Explicit(t *testing.T) {
7272
t.Skip("Skipping test on non-linux platforms")
7373
}
7474
tempDir := t.TempDir()
75-
k := Kubernetes{Kubeconfig: path.Join(tempDir, "config")}
76-
err := resolveKubernetesConfigurations(&k)
75+
m := Manager{Kubeconfig: path.Join(tempDir, "config")}
76+
err := resolveKubernetesConfigurations(&m)
7777
if err == nil {
7878
t.Errorf("expected error, got nil")
7979
}
@@ -90,8 +90,8 @@ func TestKubernetes_ResolveKubernetesConfigurations_Explicit(t *testing.T) {
9090
if err := os.WriteFile(kubeconfigPath, []byte(""), 0644); err != nil {
9191
t.Fatalf("failed to create kubeconfig file: %v", err)
9292
}
93-
k := Kubernetes{Kubeconfig: kubeconfigPath}
94-
err := resolveKubernetesConfigurations(&k)
93+
m := Manager{Kubeconfig: kubeconfigPath}
94+
err := resolveKubernetesConfigurations(&m)
9595
if err == nil {
9696
t.Errorf("expected error, got nil")
9797
}
@@ -123,16 +123,16 @@ users:
123123
if err := os.WriteFile(kubeconfigPath, []byte(kubeconfigContent), 0644); err != nil {
124124
t.Fatalf("failed to create kubeconfig file: %v", err)
125125
}
126-
k := Kubernetes{Kubeconfig: kubeconfigPath}
127-
err := resolveKubernetesConfigurations(&k)
126+
m := Manager{Kubeconfig: kubeconfigPath}
127+
err := resolveKubernetesConfigurations(&m)
128128
if err != nil {
129129
t.Fatalf("expected no error, got %v", err)
130130
}
131-
if k.cfg == nil {
131+
if m.cfg == nil {
132132
t.Errorf("expected non-nil config, got nil")
133133
}
134-
if k.cfg.Host != "https://example.com" {
135-
t.Errorf("expected host https://example.com, got %s", k.cfg.Host)
134+
if m.cfg.Host != "https://example.com" {
135+
t.Errorf("expected host https://example.com, got %s", m.cfg.Host)
136136
}
137137
})
138138
}

‎pkg/kubernetes/kubernetes.go‎

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ const (
2727
type CloseWatchKubeConfig func() error
2828

2929
type Kubernetes struct {
30+
manager *Manager
31+
}
32+
33+
type Manager struct {
3034
// Kubeconfig path override
3135
Kubeconfig string
3236
cfg *rest.Config
@@ -38,11 +42,10 @@ type Kubernetes struct {
3842
discoveryClient discovery.CachedDiscoveryInterface
3943
deferredDiscoveryRESTMapper *restmapper.DeferredDiscoveryRESTMapper
4044
dynamicClient *dynamic.DynamicClient
41-
Helm *helm.Helm
4245
}
4346

44-
func NewKubernetes(kubeconfig string) (*Kubernetes, error) {
45-
k8s := &Kubernetes{
47+
func NewManager(kubeconfig string) (*Manager, error) {
48+
k8s := &Manager{
4649
Kubeconfig: kubeconfig,
4750
}
4851
if err := resolveKubernetesConfigurations(k8s); err != nil {
@@ -68,15 +71,14 @@ func NewKubernetes(kubeconfig string) (*Kubernetes, error) {
6871
return nil, err
6972
}
7073
k8s.parameterCodec = runtime.NewParameterCodec(k8s.scheme)
71-
k8s.Helm = helm.NewHelm(k8s)
7274
return k8s, nil
7375
}
7476

75-
func (k *Kubernetes) WatchKubeConfig(onKubeConfigChange func() error) {
76-
if k.clientCmdConfig == nil {
77+
func (m *Manager) WatchKubeConfig(onKubeConfigChange func() error) {
78+
if m.clientCmdConfig == nil {
7779
return
7880
}
79-
kubeConfigFiles := k.clientCmdConfig.ConfigAccess().GetLoadingPrecedence()
81+
kubeConfigFiles := m.clientCmdConfig.ConfigAccess().GetLoadingPrecedence()
8082
if len(kubeConfigFiles) == 0 {
8183
return
8284
}
@@ -102,33 +104,33 @@ func (k *Kubernetes) WatchKubeConfig(onKubeConfigChange func() error) {
102104
}
103105
}
104106
}()
105-
if k.CloseWatchKubeConfig != nil {
106-
_ = k.CloseWatchKubeConfig()
107+
if m.CloseWatchKubeConfig != nil {
108+
_ = m.CloseWatchKubeConfig()
107109
}
108-
k.CloseWatchKubeConfig = watcher.Close
110+
m.CloseWatchKubeConfig = watcher.Close
109111
}
110112

111-
func (k *Kubernetes) Close() {
112-
if k.CloseWatchKubeConfig != nil {
113-
_ = k.CloseWatchKubeConfig()
113+
func (m *Manager) Close() {
114+
if m.CloseWatchKubeConfig != nil {
115+
_ = m.CloseWatchKubeConfig()
114116
}
115117
}
116118

117-
func (k *Kubernetes) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
118-
return k.discoveryClient, nil
119+
func (m *Manager) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
120+
return m.discoveryClient, nil
119121
}
120122

121-
func (k *Kubernetes) ToRESTMapper() (meta.RESTMapper, error) {
122-
return k.deferredDiscoveryRESTMapper, nil
123+
func (m *Manager) ToRESTMapper() (meta.RESTMapper, error) {
124+
return m.deferredDiscoveryRESTMapper, nil
123125
}
124126

125-
func (k *Kubernetes) Derived(ctx context.Context) *Kubernetes {
127+
func (m *Manager) Derived(ctx context.Context) *Kubernetes {
126128
authorization, ok := ctx.Value(AuthorizationHeader).(string)
127129
if !ok || !strings.HasPrefix(authorization, "Bearer ") {
128-
return k
130+
return &Kubernetes{manager: m}
129131
}
130132
klog.V(5).Infof("%s header found (Bearer), using provided bearer token", AuthorizationHeader)
131-
derivedCfg := rest.CopyConfig(k.cfg)
133+
derivedCfg := rest.CopyConfig(m.cfg)
132134
derivedCfg.BearerToken = strings.TrimPrefix(authorization, "Bearer ")
133135
derivedCfg.BearerTokenFile = ""
134136
derivedCfg.Username = ""
@@ -137,28 +139,42 @@ func (k *Kubernetes) Derived(ctx context.Context) *Kubernetes {
137139
derivedCfg.AuthConfigPersister = nil
138140
derivedCfg.ExecProvider = nil
139141
derivedCfg.Impersonate = rest.ImpersonationConfig{}
140-
clientCmdApiConfig, err := k.clientCmdConfig.RawConfig()
142+
clientCmdApiConfig, err := m.clientCmdConfig.RawConfig()
141143
if err != nil {
142-
return k
144+
return &Kubernetes{manager: m}
143145
}
144146
clientCmdApiConfig.AuthInfos = make(map[string]*clientcmdapi.AuthInfo)
145-
derived := &Kubernetes{
146-
Kubeconfig: k.Kubeconfig,
147+
derived := &Kubernetes{manager: &Manager{
148+
Kubeconfig: m.Kubeconfig,
147149
clientCmdConfig: clientcmd.NewDefaultClientConfig(clientCmdApiConfig, nil),
148150
cfg: derivedCfg,
149-
scheme: k.scheme,
150-
parameterCodec: k.parameterCodec,
151-
}
152-
derived.clientSet, err = kubernetes.NewForConfig(derived.cfg)
151+
scheme: m.scheme,
152+
parameterCodec: m.parameterCodec,
153+
}}
154+
derived.manager.clientSet, err = kubernetes.NewForConfig(derived.manager.cfg)
153155
if err != nil {
154-
return k
156+
return &Kubernetes{manager: m}
155157
}
156-
derived.discoveryClient = memory.NewMemCacheClient(discovery.NewDiscoveryClient(derived.clientSet.CoreV1().RESTClient()))
157-
derived.deferredDiscoveryRESTMapper = restmapper.NewDeferredDiscoveryRESTMapper(derived.discoveryClient)
158-
derived.dynamicClient, err = dynamic.NewForConfig(derived.cfg)
158+
derived.manager.discoveryClient = memory.NewMemCacheClient(discovery.NewDiscoveryClient(derived.manager.clientSet.CoreV1().RESTClient()))
159+
derived.manager.deferredDiscoveryRESTMapper = restmapper.NewDeferredDiscoveryRESTMapper(derived.manager.discoveryClient)
160+
derived.manager.dynamicClient, err = dynamic.NewForConfig(derived.manager.cfg)
159161
if err != nil {
160-
return k
162+
return &Kubernetes{manager: m}
161163
}
162-
derived.Helm = helm.NewHelm(derived)
163164
return derived
164165
}
166+
167+
// TODO: check test to see why cache isn't getting invalidated automatically https://github.com/manusa/kubernetes-mcp-server/pull/125#discussion_r2152194784
168+
func (k *Kubernetes) CacheInvalidate() {
169+
if k.manager.discoveryClient != nil {
170+
k.manager.discoveryClient.Invalidate()
171+
}
172+
if k.manager.deferredDiscoveryRESTMapper != nil {
173+
k.manager.deferredDiscoveryRESTMapper.Reset()
174+
}
175+
}
176+
177+
func (k *Kubernetes) NewHelm() *helm.Helm {
178+
// This is a derived Kubernetes, so it already has the Helm initialized
179+
return helm.NewHelm(k.manager)
180+
}

‎pkg/kubernetes/openshift.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import (
66
"k8s.io/apimachinery/pkg/runtime/schema"
77
)
88

9-
func (k *Kubernetes) IsOpenShift(ctx context.Context) bool {
9+
func (m *Manager) IsOpenShift(ctx context.Context) bool {
1010
// This method should be fast and not block (it's called at startup)
1111
timeoutSeconds := int64(5)
12-
if _, err := k.dynamicClient.Resource(schema.GroupVersionResource{
12+
if _, err := m.dynamicClient.Resource(schema.GroupVersionResource{
1313
Group: "project.openshift.io",
1414
Version: "v1",
1515
Resource: "projects",

0 commit comments

Comments
 (0)