8000 Merge pull request #173 from 3mbe/internal-k8s-docs · Azure/aks-mcp@cbd5f90 · GitHub
[go: up one dir, main page]
Skip to content

Commit cbd5f90

Browse files
feiskyerfeiskyer
authored
Merge pull request #173 from 3mbe/internal-k8s-docs
docs/tests: add GoDoc comments and unit tests for k8s adapter
2 parents 9bc3415 + 883e731 commit cbd5f90

File tree

2 files changed

+243
-10
lines changed

2 files changed

+243
-10
lines changed

‎internal/k8s/adapter.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Package k8s provides adapters that let aks-mcp interoperate with the
2+
// mcp-kubernetes libraries. It maps aks-mcp configuration and executors
3+
// to the types expected by mcp-kubernetes without altering behavior.
14
package k8s
25

36
import (
@@ -9,16 +12,13 @@ import (
912
k8stools "github.com/Azure/mcp-kubernetes/pkg/tools"
1013
)
1114

12-
// ConfigAdapter converts aks-mcp config to mcp-kubernetes config
15+
// ConvertConfig maps an aks-mcp ConfigData into the equivalent
16+
// mcp-kubernetes ConfigData without mutating the input.
1317
func ConvertConfig(cfg *config.ConfigData) *k8sconfig.ConfigData {
14-
// Create K8s security config
1518
k8sSecurityConfig := k8ssecurity.NewSecurityConfig()
16-
17-
// Map allowed namespaces
1819
k8sSecurityConfig.SetAllowedNamespaces(cfg.AllowNamespaces)
1920
k8sSecurityConfig.AccessLevel = k8ssecurity.AccessLevel(cfg.AccessLevel)
2021

21-
// Create K8s config
2222
k8sCfg := &k8sconfig.ConfigData{
2323
AdditionalTools: cfg.AdditionalTools,
2424
Timeout: cfg.Timeout,
@@ -35,20 +35,21 @@ func ConvertConfig(cfg *config.ConfigData) *k8sconfig.ConfigData {
3535
return k8sCfg
3636
}
3737

38-
// WrapK8sExecutor wraps a mcp-kubernetes executor to work with aks-mcp config
38+
// WrapK8sExecutor makes an mcp-kubernetes CommandExecutor
39+
// compatible with the aks-mcp tools.CommandExecutor interface.
3940
func WrapK8sExecutor(k8sExecutor k8stools.CommandExecutor) tools.CommandExecutor {
4041
return &executorAdapter{k8sExecutor: k8sExecutor}
4142
}
4243

43-
// executorAdapter adapts between aks-mcp and mcp-kubernetes configs
44+
// executorAdapter bridges aks-mcp execution to mcp-kubernetes.
45+
// Unexported; behavior is defined by the wrapped executor.
4446
type executorAdapter struct {
4547
k8sExecutor k8stools.CommandExecutor
4648
}
4749

50+
// Execute adapts aks-mcp execution by converting its config
51+
// and delegating to the wrapped mcp-kubernetes executor.
4852
func (a *executorAdapter) Execute(params map[string]interface{}, cfg *config.ConfigData) (string, error) {
49-
// Convert aks-mcp config to k8s config
5053
k8sCfg := ConvertConfig(cfg)
51-
52-
// Execute using the k8s executor
5354
return a.k8sExecutor.Execute(params, k8sCfg)
5455
}

‎internal/k8s/adapter_test.go‎

Lines changed: 232 additions & 0 deletions
4CF3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
package k8s
2+
3+
import (
4+
"errors"
5+
"reflect"
6+
"testing"
7+
8+
"github.com/Azure/aks-mcp/internal/config"
9+
k8sconfig "github.com/Azure/mcp-kubernetes/pkg/config"
10+
k8ssecurity "github.com/Azure/mcp-kubernetes/pkg/security"
11+
k8stools "github.com/Azure/mcp-kubernetes/pkg/tools"
12+
)
13+
14+
var benchOut *k8sconfig.ConfigData
15+
16+
// This test suite verifies config mapping (without mutating input), adapter delegation,
17+
// error propagation, and current nil-config behavior. The benchmark provides a baseline
18+
// for detecting performance regressions.
19+
20+
// mustEqual keeps assertions concise with consistent failure messages.
21+
func mustEqual[T comparable](t *testing.T, got, want T, msg string) {
22+
t.Helper()
23+
if got != want {
24+
t.Fatalf("%s: got %v, want %v", msg, got, want)
25+
}
26+
}
27+
28+
// mustDeepEqual keeps deep-structure assertions concise with consistent messages.
29+
func mustDeepEqual(t *testing.T, got, want interface{}, msg string) {
30+
t.Helper()
31+
if !reflect.DeepEqual(got, want) {
32+
t.Fatalf("%s: got %#v, want %#v", msg, got, want)
33+
}
34+
}
35+
36+
// fakeExecutor captures inputs and returns preset output/error to observe delegation.
37+
type fakeExecutor struct {
38+
lastParams map[string]interface{}
39+
lastCfg *k8sconfig.ConfigData
40+
out string
41+
err error
42+
}
43+
44+
var _ k8stools.CommandExecutor = (*fakeExecutor)(nil)
45+
46+
func (f *fakeExecutor) Execute(params map[string]interface{}, cfg *k8sconfig.ConfigData) (string, error) {
47+
f.lastParams = params
48+
f.lastCfg = cfg
49+
return f.out, f.err
50+
}
51+
52+
func TestConvertConfig_MapsFields(t *testing.T) {
53+
t.Parallel()
54+
55+
in := &config.ConfigData{
56+
Timeout: 600,
57+
Transport: "stdio",
58+
Host: "127.0.0.1",
59+
Port: 8000,
60+
AccessLevel: "readonly",
61+
AdditionalTools: map[string]bool{"helm": true, "cilium": false},
62+
AllowNamespaces: "default,platform",
63+
OTLPEndpoint: "otel:4317",
64+
}
65+
66+
got := ConvertConfig(in)
67+
if got == nil {
68+
t.Fatal("ConvertConfig returned nil")
69+
}
70+
71+
mustEqual(t, got.Timeout, in.Timeout, "Timeout")
72+
mustEqual(t, got.Transport, in.Transport, "Transport")
73+
mustEqual(t, got.Host, in.Host, "Host")
74+
mustEqual(t, got.Port, in.Port, "Port")
75+
mustEqual(t, got.AccessLevel, in.AccessLevel, "AccessLevel")
76+
mustEqual(t, got.OTLPEndpoint, in.OTLPEndpoint, "OTLPEndpoint")
77+
mustDeepEqual(t, got.AdditionalTools, in.AdditionalTools, "AdditionalTools")
78+
mustEqual(t, got.AllowNamespaces, in.AllowNamespaces, "AllowNamespaces")
79+
80+
if got.SecurityConfig == nil {
81+
t.Fatal("SecurityConfig is nil")
82+
}
83+
mustEqual(t, got.SecurityConfig.AccessLevel, k8ssecurity.AccessLevel(in.AccessLevel), "SecurityConfig.AccessLevel")
84+
}
85+
86+
func TestConvertConfig_DoesNotMutateInput(t *testing.T) {
87+
t.Parallel()
88+
89+
in := &config.ConfigData{
90+
Timeout: 42,
91+
Transport: "stdio",
92+
Host: 802E "127.0.0.1",
93+
Port: 8000,
94+
AccessLevel: "readonly",
95+
AdditionalTools: map[string]bool{"helm": true},
96+
AllowNamespaces: "default",
97+
OTLPEndpoint: "otel:4317",
98+
}
99+
100+
// Verify the “no input mutation” guarantee by comparing to a copy.
101+
orig := *in
102+
orig.AdditionalTools = map[string]bool{}
103+
for k, v := range in.AdditionalTools {
104+
orig.AdditionalTools[k] = v
105+
}
106+
107+
out := ConvertConfig(in)
108+
mustDeepEqual(t, in, &orig, "input should remain unchanged")
109+
110+
if out == nil || out.SecurityConfig == nil {
111+
t.Fatalf("expected non-nil output and SecurityConfig, got %#v", out)
112+
}
113+
114+
mustEqual(t, out.Timeout, in.Timeout, "Timeout")
115+
mustEqual(t, out.Transport, in.Transport, "Transport")
116+
mustEqual(t, out.Host, in.Host, "Host")
117+
mustEqual(t, out.Port, in.Port, "Port")
118+
mustEqual(t, out.AccessLevel, in.AccessLevel, "AccessLevel")
119+
mustEqual(t, out.OTLPEndpoint, in.OTLPEndpoint, "OTLPEndpoint")
120+
mustEqual(t, out.AllowNamespaces, in.AllowNamespaces, "AllowNamespaces")
121+
mustDeepEqual(t, out.AdditionalTools, in.AdditionalTools, "AdditionalTools")
122+
mustEqual(t, out.SecurityConfig.AccessLevel, k8ssecurity.AccessLevel(in.AccessLevel), "SecurityConfig.AccessLevel")
123+
}
124+
125+
func TestConvertConfig_ZeroValueCfg(t *testing.T) {
126+
t.Parallel()
127+
128+
// Document current behavior when callers pass an uninitialized config.
129+
in := &config.ConfigData{}
130+
orig := *in
131+
132+
out := ConvertConfig(in)
133+
134+
mustDeepEqual(t, in, &orig, "input unchanged")
135+
136+
if out == nil || out.SecurityConfig == nil {
137+
t.Fatalf("non-nil out and SecurityConfig required, got %#v", out)
138+
}
139+
140+
mustEqual(t, out.Timeout, 0, "Timeout")
141+
mustEqual(t, out.Transport, "", "Transport")
142+
mustEqual(t, out.Host, "", "Host")
143+
mustEqual(t, out.Port, 0, "Port")
144+
mustEqual(t, out.AccessLevel, "", "AccessLevel")
145+
mustEqual(t, out.OTLPEndpoint, "", "OTLPEndpoint")
146+
mustEqual(t, out.AllowNamespaces, "", "AllowNamespaces")
147+
mustDeepEqual(t, out.AdditionalTools, map[string]bool(nil), "AdditionalTools")
148+
mustEqual(t, out.SecurityConfig.AccessLevel, k8ssecurity.AccessLevel(""), "SecurityConfig.AccessLevel")
149+
}
150+
151+
func TestExecutorAdapter_DelegatesAndForwards(t *testing.T) {
152+
t.Parallel()
153+
154+
fe := &fakeExecutor{out: "ok"}
155+
adapter := WrapK8sExecutor(fe)
156+
157+
params := map[string]interface{}{"k": "v"}
158+
inCfg := &config.ConfigData{
159+
Timeout: 10,
160+
Transport: "stdio",
161+
Host: "127.0.0.1",
162+
Port: 8000,
163+
AccessLevel: "readonly",
164+
AdditionalTools: map[string]bool{"helm": true},
165+
AllowNamespaces: "default",
166+
}
167+
168+
got, err := adapter.Execute(params, inCfg)
169+
if err != nil {
170+
t.Fatalf("unexpected error: %v", err)
171+
}
172+
173+
mustEqual(t, got, "ok", "adapter output")
174+
mustDeepEqual(t, fe.lastParams, params, "params forwarded")
175+
176+
if fe.lastCfg == nil || fe.lastCfg.SecurityConfig == nil {
177+
t.Fatalf("expected non-nil converted cfg + SecurityConfig, got %#v", fe.lastCfg)
178+
}
179+
180+
mustEqual(t, fe.lastCfg.Port, inCfg.Port, "Port")
181+
mustEqual(t, fe.lastCfg.AccessLevel, inCfg.AccessLevel, "AccessLevel")
182+
mustDeepEqual(t, fe.lastCfg.AdditionalTools, inCfg.AdditionalTools, "AdditionalTools")
183+
mustEqual(t, fe.lastCfg.AllowNamespaces, inCfg.AllowNamespaces, "AllowNamespaces")
184+
mustEqual(t, fe.lastCfg.SecurityConfig.AccessLevel, k8ssecurity.AccessLevel("readonly"), "SecurityConfig.AccessLevel")
185+
}
186+
187+
func TestExecutorAdapter_PropagatesError(t *testing.T) {
188+
t.Parallel()
189+
190+
fe := &fakeExecutor{err: errors.New("boom")}
191+
adapter := WrapK8sExecutor(fe)
192+
193+
_, err := adapter.Execute(map[string]interface{}{"x": 1}, &config.ConfigData{})
194+
if err == nil {
195+
t.Fatalf("expected error, got nil")
196+
}
197+
}
198+
199+
func TestExecutorAdapter_PanicsOnNilConfig_CurrentBehavior(t *testing.T) {
200+
t.Parallel()
201+
202+
// Document the current precondition: cfg must be non-nil.
203+
defer func() {
204+
if r := recover(); r == nil {
205+
t.Fatalf("expected panic when cfg is nil")
206+
}
207+
}()
208+
209+
fe := &fakeExecutor{}
210+
adapter := WrapK8sExecutor(fe)
211+
_, _ = adapter.Execute(map[string]interface{}{"x": 1}, nil)
212+
}
213+
214+
// BenchmarkConvertConfig tracks drift in allocation/time costs over time.
215+
// Helps detect subtle regressions when config mapping logic evolves.
216+
func BenchmarkConvertConfig(b *testing.B) {
217+
in := &config.ConfigData{
218+
Timeout: 600,
219+
Transport: "stdio",
220+
Host: "127.0.0.1",
221+
Port: 8000,
222+
AccessLevel: "readonly",
223+
AdditionalTools: map[string]bool{"helm": true, "cilium": false},
224+
AllowNamespaces: "default,platform",
225+
OTLPEndpoint: "otel:4317",
226+
}
227+
228+
b.ResetTimer()
229+
for i := 0; i < b.N; i++ {
230+
benchOut = ConvertConfig(in)
231+
}
232+
}

0 commit comments

Comments
 (0)
0