Skip to content

Commit 5f80a54

Browse files
ardagucluardaguclu
committed
Removal of rigid binding to kubeconfig env var and skip some tests in OCP
Relying on KUBECONFIG env variable causes issues in cases where env var is statically set by the environments. So that default namespace is modified to something different than default with less permissions. That causes failures of the tests. Additionally, all namespaces tests are broken in CI environments that the given temporary namespace can't access to query the all namespaces in the cluster. This commit skips these tests.
1 parent 7a3d668 commit 5f80a54

14 files changed

+230
-147
lines changed

‎pkg/http/http_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ func (c *httpContext) beforeEach(t *testing.T) {
6969
mockKubeConfig := c.mockServer.KubeConfig()
7070
kubeConfig := filepath.Join(t.TempDir(), "config")
7171
_ = clientcmd.WriteToFile(*mockKubeConfig, kubeConfig)
72-
_ = os.Setenv("KUBECONFIG", kubeConfig)
7372
// Capture logging
7473
c.klogState = klog.CaptureState()
7574
flags := flag.NewFlagSet("test", flag.ContinueOnError)
@@ -86,6 +85,7 @@ func (c *httpContext) beforeEach(t *testing.T) {
8685
t.Fatalf("Failed to close random port listener: %v", randomPortErr)
8786
}
8887
c.StaticConfig.Port = fmt.Sprintf("%d", ln.Addr().(*net.TCPAddr).Port)
88+
c.StaticConfig.KubeConfig = kubeConfig
8989
mcpServer, err := mcp.NewServer(mcp.Configuration{
9090
Profile: mcp.Profiles[0],
9191
StaticConfig: c.StaticConfig,

‎pkg/kubernetes/configuration_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ func TestKubernetes_ResolveKubernetesConfigurations_Explicit(t *testing.T) {
9898
}
9999
})
100100
t.Run("with empty file", func(t *testing.T) {
101+
if val := os.Getenv("OPENSHIFT_CI"); val != "" {
102+
t.Skip("this test does not work on OpenShift CI. So we are skipping...")
103+
}
101104
tempDir := t.TempDir()
102105
kubeconfigPath := path.Join(tempDir, "config")
103106
if err := os.WriteFile(kubeconfigPath, []byte(""), 0644); err != nil {

‎pkg/mcp/common_test.go‎

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,38 @@ type mcpContext struct {
104104
listOutput output.Output
105105
logLevel int
106106

107-
staticConfig *config.StaticConfig
108-
clientOptions []transport.ClientOption
109-
before func(*mcpContext)
110-
after func(*mcpContext)
111-
ctx context.Context
112-
tempDir string
113-
cancel context.CancelFunc
114-
mcpServer *Server
115-
mcpHttpServer *httptest.Server
116-
mcpClient *client.Client
117-
klogState klog.State
118-
logBuffer bytes.Buffer
107+
staticConfig *config.StaticConfig
108+
clientOptions []transport.ClientOption
109+
before func(*mcpContext)
110+
after func(*mcpContext)
111+
ctx context.Context
112+
tempDir string
113+
cancel context.CancelFunc
114+
mcpServer *Server
115+
mcpHttpServer *httptest.Server
116+
mcpClient *client.Client
117+
klogState klog.State
118+
logBuffer bytes.Buffer
119+
useEnvTestKubeConfig bool
120+
useInClusterKubeConfig bool
121+
customKubeConfig *rest.Config
119122
}
120123

121124
func (c *mcpContext) beforeEach(t *testing.T) {
122125
var err error
123126
c.ctx, c.cancel = context.WithCancel(t.Context())
124127
c.tempDir = t.TempDir()
125-
c.withKubeConfig(nil)
128+
var kubeConfig string
129+
if c.useEnvTestKubeConfig {
130+
if envTestRestConfig == nil {
131+
panic("shouldn't be empty")
132+
}
133+
_, kubeConfig = c.withKubeConfig(envTestRestConfig)
134+
} else if c.customKubeConfig != nil {
135+
_, kubeConfig = c.withKubeConfig(c.customKubeConfig)
136+
} else if !c.useInClusterKubeConfig {
137+
_, kubeConfig = c.withKubeConfig(nil)
138+
}
126139
if c.profile == nil {
127140
c.profile = &FullProfile{}
128141
}
@@ -135,6 +148,7 @@ func (c *mcpContext) beforeEach(t *testing.T) {
135148
DisableDestructive: false,
136149
}
137150
}
151+
c.staticConfig.KubeConfig = kubeConfig
138152
if c.before != nil {
139153
c.before(c)
140154
}
@@ -184,8 +198,13 @@ func (c *mcpContext) afterEach() {
184198
c.klogState.Restore()
185199
}
186200

187-
func testCase(t *testing.T, test func(c *mcpContext)) {
188-
testCaseWithContext(t, &mcpContext{profile: &FullProfile{}}, test)
201+
func testCase(t *testing.T, envTestKubeConfig bool, inClusterKubeConfig bool, customKubeConfig *rest.Config, test func(c *mcpContext)) {
202+
testCaseWithContext(t, &mcpContext{
203+
profile: &FullProfile{},
204+
useEnvTestKubeConfig: envTestKubeConfig,
205+
useInClusterKubeConfig: inClusterKubeConfig,
206+
customKubeConfig: customKubeConfig,
207+
}, test)
189208
}
190209

191210
func testCaseWithContext(t *testing.T, mcpCtx *mcpContext, test func(c *mcpContext)) {
@@ -195,7 +214,7 @@ func testCaseWithContext(t *testing.T, mcpCtx *mcpContext, test func(c *mcpConte
195214
}
196215

197216
// withKubeConfig sets up a fake kubeconfig in the temp directory based on the provided rest.Config
198-
func (c *mcpContext) withKubeConfig(rc *rest.Config) *api.Config {
217+
func (c *mcpContext) withKubeConfig(rc *rest.Config) (*api.Config, string) {
199218
fakeConfig := api.NewConfig()
200219
fakeConfig.Clusters["fake"] = api.NewCluster()
201220
fakeConfig.Clusters["fake"].Server = "https://127.0.0.1:6443"
@@ -217,23 +236,17 @@ func (c *mcpContext) withKubeConfig(rc *rest.Config) *api.Config {
217236
fakeConfig.CurrentContext = "fake-context"
218237
kubeConfig := filepath.Join(c.tempDir, "config")
219238
_ = clientcmd.WriteToFile(*fakeConfig, kubeConfig)
220-
_ = os.Setenv("KUBECONFIG", kubeConfig)
221239
if c.mcpServer != nil {
240+
c.mcpServer.configuration.StaticConfig.KubeConfig = kubeConfig
222241
if err := c.mcpServer.reloadKubernetesClient(); err != nil {
223242
panic(err)
224243
}
225244
}
226-
return fakeConfig
227-
}
228-
229-
// withEnvTest sets up the environment for kubeconfig to be used with envTest
230-
func (c *mcpContext) withEnvTest() {
231-
c.withKubeConfig(envTestRestConfig)
245+
return fakeConfig, kubeConfig
232246
}
233247

234248
// inOpenShift sets up the kubernetes environment to seem to be running OpenShift
235249
func inOpenShift(c *mcpContext) {
236-
c.withEnvTest()
237250
crdTemplate := `
238251
{
239252
"apiVersion": "apiextensions.k8s.io/v1",

‎pkg/mcp/configuration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
)
1111

1212
func TestConfigurationView(t *testing.T) {
13-
testCase(t, func(c *mcpContext) {
13+
testCase(t, false, false, nil, func(c *mcpContext) {
1414
toolResult, err := c.callTool("configuration_view", map[string]interface{}{})
1515
t.Run("configuration_view returns configuration", func(t *testing.T) {
1616
if err != nil {
@@ -122,7 +122,7 @@ func TestConfigurationViewInCluster(t *testing.T) {
122122
defer func() {
123123
kubernetes.InClusterConfig = rest.InClusterConfig
124124
}()
125-
testCase(t, func(c *mcpContext) {
125+
testCase(t, false, true, nil, func(c *mcpContext) {
126126
toolResult, err := c.callTool("configuration_view", map[string]interface{}{})
127127
t.Run("configuration_view returns configuration", func(t *testing.T) {
128128
if err != nil {

‎pkg/mcp/events_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ import (
99
)
1010

1111
func TestEventsList(t *testing.T) {
12-
testCase(t, func(c *mcpContext) {
13-
c.withEnvTest()
12+
testCase(t, true, false, nil, func(c *mcpContext) {
1413
toolResult, err := c.callTool("events_list", map[string]interface{}{})
1514
t.Run("events_list with no events returns OK", func(t *testing.T) {
1615
if err != nil {
@@ -97,8 +96,7 @@ func TestEventsList(t *testing.T) {
9796

9897
func TestEventsListDenied(t *testing.T) {
9998
deniedResourcesServer := &config.StaticConfig{DeniedResources: []config.GroupVersionKind{{Version: "v1", Kind: "Event"}}}
100-
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer}, func(c *mcpContext) {
101-
c.withEnvTest()
99+
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer, useEnvTestKubeConfig: true}, func(c *mcpContext) {
102100
eventList, _ := c.callTool("events_list", map[string]interface{}{})
103101
t.Run("events_list has error", func(t *testing.T) {
104102
if !eventList.IsError {

‎pkg/mcp/helm_test.go‎

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,40 @@ package mcp
33
import (
44
"context"
55
"encoding/base64"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
611
"github.com/containers/kubernetes-mcp-server/pkg/config"
712
"github.com/mark3labs/mcp-go/mcp"
813
corev1 "k8s.io/api/core/v1"
914
"k8s.io/apimachinery/pkg/api/errors"
1015
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1116
"k8s.io/client-go/kubernetes"
12-
"path/filepath"
13-
"runtime"
17+
"k8s.io/klog/v2"
1418
"sigs.k8s.io/yaml"
15-
"strings"
16-
"testing"
1719
)
1820

1921
func TestHelmInstall(t *testing.T) {
20-
testCase(t, func(c *mcpContext) {
21-
c.withEnvTest()
22-
_, file, _, _ := runtime.Caller(0)
22+
testCase(t, true, false, nil, func(c *mcpContext) {
23+
_, file, _, ok := runtime.Caller(0)
24+
if !ok {
25+
t.Fatalf("could not get caller info")
26+
}
27+
ns := c.mcpServer.k.NamespaceOrDefault("default")
28+
klog.Infof("namespace: %s will be used for helm chart installation", ns)
2329
chartPath := filepath.Join(filepath.Dir(file), "testdata", "helm-chart-no-op")
2430
toolResult, err := c.callTool("helm_install", map[string]interface{}{
25-
"chart": chartPath,
31+
"chart": chartPath,
32+
"namespace": ns,
2633
})
2734
t.Run("helm_install with local chart and no release name, returns installed chart", func(t *testing.T) {
2835
if err != nil {
2936
t.Fatalf("call tool failed %v", err)
3037
}
3138
if toolResult.IsError {
32-
t.Fatalf("call tool failed")
39+
t.Fatalf("call tool failed %s", toolResult.Content)
3340
}
3441
var decoded []map[string]interface{}
3542
err = yaml.Unmarshal([]byte(toolResult.Content[0].(mcp.TextContent).Text), &decoded)
@@ -60,12 +67,16 @@ func TestHelmInstall(t *testing.T) {
6067

6168
func TestHelmInstallDenied(t *testing.T) {
6269
deniedResourcesServer := &config.StaticConfig{DeniedResources: []config.GroupVersionKind{{Version: "v1", Kind: "Secret"}}}
63-
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer}, func(c *mcpContext) {
64-
c.withEnvTest()
70+
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer, useEnvTestKubeConfig: true}, func(c *mcpContext) {
6571
_, file, _, _ := runtime.Caller(0)
6672
chartPath := filepath.Join(filepath.Dir(file), "testdata", "helm-chart-secret")
73+
74+
ns := c.mcpServer.k.NamespaceOrDefault("default")
75+
klog.Infof("namespace: %s will be used for helm chart installation", ns)
76+
6777
helmInstall, _ := c.callTool("helm_install", map[string]interface{}{
68-
"chart": chartPath,
78+
"chart": chartPath,
79+
"namespace": ns,
6980
})
7081
t.Run("helm_install has error", func(t *testing.T) {
7182
if !helmInstall.IsError {
@@ -83,11 +94,16 @@ func TestHelmInstallDenied(t *testing.T) {
8394
}
8495

8596
func TestHelmList(t *testing.T) {
86-
testCase(t, func(c *mcpContext) {
87-
c.withEnvTest()
97+
testCase(t, true, false, nil, func(c *mcpContext) {
8898
kc := c.newKubernetesClient()
8999
clearHelmReleases(c.ctx, kc)
90-
toolResult, err := c.callTool("helm_list", map[string]interface{}{})
100+
101+
ns := c.mcpServer.k.NamespaceOrDefault("default")
102+
klog.Infof("namespace: %s will be used for helm chart installation", ns)
103+
104+
toolResult, err := c.callTool("helm_list", map[string]interface{}{
105+
"namespace": ns,
106+
})
91107
t.Run("helm_list with no releases, returns not found", func(t *testing.T) {
92108
if err != nil {
93109
t.Fatalf("call tool failed %v", err)
@@ -99,7 +115,7 @@ func TestHelmList(t *testing.T) {
99115
t.Fatalf("unexpected result %v", toolResult.Content[0].(mcp.TextContent).Text)
100116
}
101117
})
102-
_, _ = kc.CoreV1().Secrets("default").Create(c.ctx, &corev1.Secret{
118+
_, _ = kc.CoreV1().Secrets(ns).Create(c.ctx, &corev1.Secret{
103119
ObjectMeta: metav1.ObjectMeta{
104120
Name: "sh.helm.release.v1.release-to-list",
105121
Labels: map[string]string{"owner": "helm", "name": "release-to-list"},
@@ -111,7 +127,9 @@ func TestHelmList(t *testing.T) {
111127
"}"))),
112128
},
113129
}, metav1.CreateOptions{})
114-
toolResult, err = c.callTool("helm_list", map[string]interface{}{})
130+
toolResult, err = c.callTool("helm_list", map[string]interface{}{
131+
"namespace": ns,
132+
})
115133
t.Run("helm_list with deployed release, returns release", func(t *testing.T) {
116134
if err != nil {
117135
t.Fatalf("call tool failed %v", err)
@@ -173,12 +191,12 @@ func TestHelmList(t *testing.T) {
173191
}
174192

175193
func TestHelmUninstall(t *testing.T) {
176-
testCase(t, func(c *mcpContext) {
177-
c.withEnvTest()
194+
testCase(t, true, false, nil, func(c *mcpContext) {
178195
kc := c.newKubernetesClient()
179196
clearHelmReleases(c.ctx, kc)
180197
toolResult, err := c.callTool("helm_uninstall", map[string]interface{}{
181-
"name": "release-to-uninstall",
198+
"name": "release-to-uninstall",
199+
"namespace": "default",
182200
})
183201
t.Run("helm_uninstall with no releases, returns not found", func(t *testing.T) {
184202
if err != nil {
@@ -204,7 +222,8 @@ func TestHelmUninstall(t *testing.T) {
204222
},
205223
}, metav1.CreateOptions{})
206224
toolResult, err = c.callTool("helm_uninstall", map[string]interface{}{
207-
"name": "existent-release-to-uninstall",
225+
"name": "existent-release-to-uninstall",
226+
"namespace": "default",
208227
})
209228
t.Run("helm_uninstall with deployed release, returns uninstalled", func(t *testing.T) {
210229
if err != nil {
@@ -226,8 +245,7 @@ func TestHelmUninstall(t *testing.T) {
226245

227246
func TestHelmUninstallDenied(t *testing.T) {
228247
deniedResourcesServer := &config.StaticConfig{DeniedResources: []config.GroupVersionKind{{Version: "v1", Kind: "Secret"}}}
229-
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer}, func(c *mcpContext) {
230-
c.withEnvTest()
248+
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer, useEnvTestKubeConfig: true}, func(c *mcpContext) {
231249
kc := c.newKubernetesClient()
232250
clearHelmReleases(c.ctx, kc)
233251
_, _ = kc.CoreV1().Secrets("default").Create(c.ctx, &corev1.Secret{
@@ -244,7 +262,8 @@ func TestHelmUninstallDenied(t *testing.T) {
244262
},
245263
}, metav1.CreateOptions{})
246264
helmUninstall, _ := c.callTool("helm_uninstall", map[string]interface{}{
247-
"name": "existent-release-to-uninstall",
265+
"name": "existent-release-to-uninstall",
266+
"namespace": "default",
248267
})
249268
t.Run("helm_uninstall has error", func(t *testing.T) {
250269
if !helmUninstall.IsError {

‎pkg/mcp/mcp_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func TestWatchKubeConfig(t *testing.T) {
1818
if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
1919
t.Skip("Skipping test on non-Unix-like platforms")
2020
}
21-
testCase(t, func(c *mcpContext) {
21+
testCase(t, false, false, nil, func(c *mcpContext) {
2222
// Given
2323
withTimeout, cancel := context.WithTimeout(c.ctx, 5*time.Second)
2424
defer cancel()

‎pkg/mcp/mcp_tools_test.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
package mcp
22

33
import (
4-
"github.com/mark3labs/mcp-go/client/transport"
5-
"github.com/mark3labs/mcp-go/mcp"
6-
"k8s.io/utils/ptr"
74
"regexp"
85
"strings"
96
"testing"
107

8+
"github.com/mark3labs/mcp-go/client/transport"
9+
"github.com/mark3labs/mcp-go/mcp"
10+
"k8s.io/utils/ptr"
11+
1112
"github.com/containers/kubernetes-mcp-server/pkg/config"
1213
)
1314

1415
func TestUnrestricted(t *testing.T) {
15-
testCase(t, func(c *mcpContext) {
16+
testCase(t, false, false, nil, func(c *mcpContext) {
1617
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})
1718
t.Run("ListTools returns tools", func(t *testing.T) {
1819
if err != nil {
@@ -32,7 +33,12 @@ func TestUnrestricted(t *testing.T) {
3233
}
3334

3435
func TestReadOnly(t *testing.T) {
35-
readOnlyServer := func(c *mcpContext) { c.staticConfig = &config.StaticConfig{ReadOnly: true} }
36+
readOnlyServer := func(c *mcpContext) {
37+
c.staticConfig = &config.StaticConfig{
38+
ReadOnly: true,
39+
KubeConfig: c.staticConfig.KubeConfig,
40+
}
41+
}
3642
testCaseWithContext(t, &mcpContext{before: readOnlyServer}, func(c *mcpContext) {
3743
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})
3844
t.Run("ListTools returns tools", func(t *testing.T) {
@@ -54,7 +60,12 @@ func TestReadOnly(t *testing.T) {
5460
}
5561

5662
func TestDisableDestructive(t *testing.T) {
57-
disableDestructiveServer := func(c *mcpContext) { c.staticConfig = &config.StaticConfig{DisableDestructive: true} }
63+
disableDestructiveServer := func(c *mcpContext) {
64+
c.staticConfig = &config.StaticConfig{
65+
DisableDestructive: true,
66+
KubeConfig: c.staticConfig.KubeConfig,
67+
}
68+
}
5869
testCaseWithContext(t, &mcpContext{before: disableDestructiveServer}, func(c *mcpContext) {
5970
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})
6071
t.Run("ListTools returns tools", func(t *testing.T) {

0 commit comments

Comments
 (0)