diff --git a/pkg/kiali/kiali.go b/pkg/kiali/kiali.go index eea7a18de..9c82030cc 100644 --- a/pkg/kiali/kiali.go +++ b/pkg/kiali/kiali.go @@ -5,32 +5,41 @@ import ( "crypto/tls" "fmt" "io" - "k8s.io/klog/v2" "net/http" "net/url" "strings" + + "github.com/containers/kubernetes-mcp-server/pkg/config" + "k8s.io/client-go/rest" + "k8s.io/klog/v2" ) type Kiali struct { - manager *Manager + bearerToken string + kialiURL string + kialiInsecure bool } -func (m *Manager) GetKiali() *Kiali { - return &Kiali{manager: m} -} - -func (k *Kiali) GetKiali() *Kiali { - return k +// NewKiali creates a new Kiali instance +func NewKiali(config *config.StaticConfig, kubernetes *rest.Config) *Kiali { + kiali := &Kiali{bearerToken: kubernetes.BearerToken} + if cfg, ok := config.GetToolsetConfig("kiali"); ok { + if kc, ok := cfg.(*Config); ok && kc != nil { + kiali.kialiURL = kc.Url + kiali.kialiInsecure = kc.Insecure + } + } + return kiali } // validateAndGetURL validates the Kiali client configuration and returns the full URL // by safely concatenating the base URL with the provided endpoint, avoiding duplicate // or missing slashes regardless of trailing/leading slashes. func (k *Kiali) validateAndGetURL(endpoint string) (string, error) { - if k == nil || k.manager == nil || k.manager.KialiURL == "" { + if k == nil || k.kialiURL == "" { return "", fmt.Errorf("kiali client not initialized") } - baseStr := strings.TrimSpace(k.manager.KialiURL) + baseStr := strings.TrimSpace(k.kialiURL) if baseStr == "" { return "", fmt.Errorf("kiali server URL not configured") } @@ -52,7 +61,7 @@ func (k *Kiali) createHTTPClient() *http.Client { return &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ - InsecureSkipVerify: k.manager.KialiInsecure, + InsecureSkipVerify: k.kialiInsecure, }, }, } @@ -62,10 +71,10 @@ func (k *Kiali) createHTTPClient() *http.Client { // Kiali client is currently configured to use (Bearer ), or empty // if no bearer token is configured. func (k *Kiali) authorizationHeader() string { - if k == nil || k.manager == nil { + if k == nil { return "" } - token := strings.TrimSpace(k.manager.BearerToken) + token := strings.TrimSpace(k.bearerToken) if token == "" { return "" } diff --git a/pkg/kiali/kiali_test.go b/pkg/kiali/kiali_test.go index 2e520d0b6..2db55aa03 100644 --- a/pkg/kiali/kiali_test.go +++ b/pkg/kiali/kiali_test.go @@ -1,79 +1,126 @@ package kiali import ( - "context" + "fmt" "net/http" - "net/http/httptest" "net/url" "testing" + "github.com/containers/kubernetes-mcp-server/internal/test" "github.com/containers/kubernetes-mcp-server/pkg/config" + "github.com/stretchr/testify/suite" ) -func TestValidateAndGetURL_JoinsProperly(t *testing.T) { - cfg := config.Default() - cfg.SetToolsetConfig("kiali", &Config{Url: "https://kiali.example/"}) - m := NewManager(cfg) - k := m.GetKiali() - - full, err := k.validateAndGetURL("/api/path") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if full != "https://kiali.example/api/path" { - t.Fatalf("unexpected url: %s", full) - } - - m.KialiURL = "https://kiali.example" - full, err = k.validateAndGetURL("api/path") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if full != "https://kiali.example/api/path" { - t.Fatalf("unexpected url: %s", full) - } - - // preserve query - m.KialiURL = "https://kiali.example" - full, err = k.validateAndGetURL("/api/path?x=1&y=2") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - u, _ := url.Parse(full) - if u.Path != "/api/path" || u.Query().Get("x") != "1" || u.Query().Get("y") != "2" { - t.Fatalf("unexpected parsed url: %s", full) - } +type KialiSuite struct { + suite.Suite + MockServer *test.MockServer + Config *config.StaticConfig +} + +func (s *KialiSuite) SetupTest() { + s.MockServer = test.NewMockServer() + s.MockServer.Config().BearerToken = "" + s.Config = config.Default() +} + +func (s *KialiSuite) TearDownTest() { + s.MockServer.Close() +} + +func (s *KialiSuite) TestNewKiali_SetsFields() { + s.Config = test.Must(config.ReadToml([]byte(` + [toolset_configs.kiali] + url = "https://kiali.example/" + insecure = true + `))) + s.MockServer.Config().BearerToken = "bearer-token" + k := NewKiali(s.Config, s.MockServer.Config()) + + s.Run("URL is set", func() { + s.Equal("https://kiali.example/", k.kialiURL, "Unexpected Kiali URL") + }) + s.Run("Insecure is set", func() { + s.True(k.kialiInsecure, "Expected Kiali Insecure to be true") + }) + s.Run("BearerToken is set", func() { + s.Equal("bearer-token", k.bearerToken, "Unexpected Kiali BearerToken") + }) +} + +func (s *KialiSuite) TestNewKiali_InvalidConfig() { + cfg, err := config.ReadToml([]byte(` + [toolset_configs.kiali] + url = "://invalid-url" + `)) + s.Error(err, "Expected error reading invalid config") + s.ErrorContains(err, "kiali-url must be a valid URL", "Unexpected error message") + s.Nil(cfg, "Unexpected Kiali config") +} + +func (s *KialiSuite) TestValidateAndGetURL() { + s.Config = test.Must(config.ReadToml([]byte(` + [toolset_configs.kiali] + url = "https://kiali.example/" + `))) + k := NewKiali(s.Config, s.MockServer.Config()) + + s.Run("Computes full URL", func() { + s.Run("with leading slash", func() { + full, err := k.validateAndGetURL("/api/path") + s.Require().NoError(err, "Expected no error validating URL") + s.Equal("https://kiali.example/api/path", full, "Unexpected full URL") + }) + + s.Run("without leading slash", func() { + full, err := k.validateAndGetURL("api/path") + s.Require().NoError(err, "Expected no error validating URL") + s.Equal("https://kiali.example/api/path", full, "Unexpected full URL") + }) + + s.Run("with query parameters, preserves query", func() { + full, err := k.validateAndGetURL("/api/path?x=1&y=2") + s.Require().NoError(err, "Expected no error validating URL") + u, err := url.Parse(full) + s.Require().NoError(err, "Expected to parse full URL") + s.Equal("/api/path", u.Path, "Unexpected path in parsed URL") + s.Equal("1", u.Query().Get("x"), "Unexpected query parameter x") + s.Equal("2", u.Query().Get("y"), "Unexpected query parameter y") + }) + }) } // CurrentAuthorizationHeader behavior is now implicit via executeRequest using Manager.BearerToken -func TestExecuteRequest_SetsAuthAndCallsServer(t *testing.T) { +func (s *KialiSuite) TestExecuteRequest() { // setup test server to assert path and auth header var seenAuth string var seenPath string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.MockServer.Config().BearerToken = "token-xyz" + s.MockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { seenAuth = r.Header.Get("Authorization") seenPath = r.URL.String() _, _ = w.Write([]byte("ok")) })) - defer srv.Close() - - cfg := config.Default() - cfg.SetToolsetConfig("kiali", &Config{Url: srv.URL}) - m := NewManager(cfg) - m.BearerToken = "token-xyz" - k := m.GetKiali() - out, err := k.executeRequest(context.Background(), "/api/ping?q=1") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if out != "ok" { - t.Fatalf("unexpected body: %s", out) - } - if seenAuth != "Bearer token-xyz" { - t.Fatalf("expected auth header to be set, got '%s'", seenAuth) - } - if seenPath != "/api/ping?q=1" { - t.Fatalf("unexpected path: %s", seenPath) - } + + s.Config = test.Must(config.ReadToml([]byte(fmt.Sprintf(` + [toolset_configs.kiali] + url = "%s" + `, s.MockServer.Config().Host)))) + k := NewKiali(s.Config, s.MockServer.Config()) + + out, err := k.executeRequest(s.T().Context(), "/api/ping?q=1") + s.Require().NoError(err, "Expected no error executing request") + s.Run("auth header set", func() { + s.Equal("Bearer token-xyz", seenAuth, "Unexpected Authorization header") + }) + s.Run("path is correct", func() { + s.Equal("/api/ping?q=1", seenPath, "Unexpected path") + }) + s.Run("response body is correct", func() { + s.Equal("ok", out, "Unexpected response body") + }) +} + +func TestKiali(t *testing.T) { + suite.Run(t, new(KialiSuite)) } diff --git a/pkg/kiali/manager.go b/pkg/kiali/manager.go deleted file mode 100644 index 276f22216..000000000 --- a/pkg/kiali/manager.go +++ /dev/null @@ -1,32 +0,0 @@ -package kiali - -import ( - "context" - - "github.com/containers/kubernetes-mcp-server/pkg/config" -) - -type Manager struct { - BearerToken string - KialiURL string - KialiInsecure bool -} - -func NewManager(config *config.StaticConfig) *Manager { - m := &Manager{ - BearerToken: "", - KialiURL: "", - KialiInsecure: false, - } - if cfg, ok := config.GetToolsetConfig("kiali"); ok { - if kc, ok := cfg.(*Config); ok && kc != nil { - m.KialiURL = kc.Url - m.KialiInsecure = kc.Insecure - } - } - return m -} - -func (m *Manager) Derived(_ context.Context) (*Kiali, error) { - return &Kiali{manager: m}, nil -} diff --git a/pkg/kiali/manager_test.go b/pkg/kiali/manager_test.go deleted file mode 100644 index d8aa275aa..000000000 --- a/pkg/kiali/manager_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package kiali - -import ( - "context" - "testing" - - "github.com/containers/kubernetes-mcp-server/pkg/config" -) - -func TestNewManagerUsesConfigFields(t *testing.T) { - cfg := config.Default() - cfg.SetToolsetConfig("kiali", &Config{Url: "https://kiali.example", Insecure: true}) - m := NewManager(cfg) - if m == nil { - t.Fatalf("expected manager, got nil") - } - if m.KialiURL != "https://kiali.example" { - t.Fatalf("expected KialiURL %s, got %s", "https://kiali.example", m.KialiURL) - } - if m.KialiInsecure != true { - t.Fatalf("expected KialiInsecure %v, got %v", true, m.KialiInsecure) - } -} - -func TestDerivedWithoutAuthorizationReturnsOriginalManager(t *testing.T) { - cfg := config.Default() - cfg.SetToolsetConfig("kiali", &Config{Url: "https://kiali.example"}) - m := NewManager(cfg) - k, err := m.Derived(context.Background()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if k == nil || k.manager != m { - t.Fatalf("expected derived Kiali to keep original manager") - } -} - -func TestDerivedPreservesURLAndToken(t *testing.T) { - cfg := config.Default() - cfg.SetToolsetConfig("kiali", &Config{Url: "https://kiali.example", Insecure: true}) - m := NewManager(cfg) - m.BearerToken = "token-abc" - k, err := m.Derived(context.Background()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if k == nil || k.manager == nil { - t.Fatalf("expected derived Kiali with manager") - } - if k.manager.BearerToken != "token-abc" { - t.Fatalf("expected bearer token 'token-abc', got '%s'", k.manager.BearerToken) - } - if k.manager.KialiURL != m.KialiURL || k.manager.KialiInsecure != m.KialiInsecure { - t.Fatalf("expected Kiali URL/insecure preserved") - } -} diff --git a/pkg/kiali/mesh_test.go b/pkg/kiali/mesh_test.go index 8af6f11ba..a729015d7 100644 --- a/pkg/kiali/mesh_test.go +++ b/pkg/kiali/mesh_test.go @@ -1,43 +1,40 @@ package kiali import ( - "context" + "fmt" "net/http" - "net/http/httptest" "net/url" - "testing" + "github.com/containers/kubernetes-mcp-server/internal/test" "github.com/containers/kubernetes-mcp-server/pkg/config" ) -func TestMeshStatus_CallsGraphWithExpectedQuery(t *testing.T) { +func (s *KialiSuite) TestMeshStatus() { var capturedURL *url.URL - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.MockServer.Config().BearerToken = "token-xyz" + s.MockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { u := *r.URL capturedURL = &u _, _ = w.Write([]byte("graph")) })) - defer srv.Close() - cfg := config.Default() - cfg.SetToolsetConfig("kiali", &Config{Url: srv.URL}) - m := NewManager(cfg) - m.BearerToken = "tkn" - k := m.GetKiali() - out, err := k.MeshStatus(context.Background()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if out != "graph" { - t.Fatalf("unexpected response: %s", out) - } - if capturedURL == nil { - t.Fatalf("expected request to be captured") - } - if capturedURL.Path != "/api/mesh/graph" { - t.Fatalf("unexpected path: %s", capturedURL.Path) - } - if capturedURL.Query().Get("includeGateways") != "false" || capturedURL.Query().Get("includeWaypoints") != "false" { - t.Fatalf("unexpected query: %s", capturedURL.RawQuery) - } + s.Config = test.Must(config.ReadToml([]byte(fmt.Sprintf(` + [toolset_configs.kiali] + url = "%s" + `, s.MockServer.Config().Host)))) + k := NewKiali(s.Config, s.MockServer.Config()) + + out, err := k.MeshStatus(s.T().Context()) + s.Require().NoError(err, "Expected no error executing request") + s.Run("response body is correct", func() { + s.Equal("graph", out, "Unexpected response body") + }) + s.Run("path is correct", func() { + s.Equal("/api/mesh/graph", capturedURL.Path, "Unexpected path") + }) + s.Run("query parameters are correct", func() { + s.Equal("false", capturedURL.Query().Get("includeGateways"), "Unexpected includeGateways query parameter") + s.Equal("false", capturedURL.Query().Get("includeWaypoints"), "Unexpected includeWaypoints query parameter") + }) + } diff --git a/pkg/kubernetes-mcp-server/cmd/root.go b/pkg/kubernetes-mcp-server/cmd/root.go index 3bb0f8216..1a81ce1f5 100644 --- a/pkg/kubernetes-mcp-server/cmd/root.go +++ b/pkg/kubernetes-mcp-server/cmd/root.go @@ -292,20 +292,20 @@ func (m *MCPServerOptions) Validate() error { klog.Warningf("authorization-url is using http://, this is not recommended production use") } } - /* If Kiali tools are enabled, validate Kiali toolset configuration */ - if slices.Contains(m.StaticConfig.Toolsets, "kiali") { - cfg, ok := m.StaticConfig.GetToolsetConfig("kiali") - if !ok { - return fmt.Errorf("kiali-url is required when kiali tools are enabled") - } - if err := cfg.Validate(); err != nil { - // Normalize error message for missing URL to match expected UX/tests - if strings.Contains(err.Error(), "kiali-url is required") { - return fmt.Errorf("kiali-url is required when kiali tools are enabled") - } - return fmt.Errorf("invalid kiali configuration: %w", err) - } - } + /* If Kiali tools are enabled, validate Kiali toolset configuration */ + if slices.Contains(m.StaticConfig.Toolsets, "kiali") { + cfg, ok := m.StaticConfig.GetToolsetConfig("kiali") + if !ok { + return fmt.Errorf("kiali-url is required when kiali tools are enabled") + } + if err := cfg.Validate(); err != nil { + // Normalize error message for missing URL to match expected UX/tests + if strings.Contains(err.Error(), "kiali-url is required") { + return fmt.Errorf("kiali-url is required when kiali tools are enabled") + } + return fmt.Errorf("invalid kiali configuration: %w", err) + } + } return nil } diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index d5da83858..7de8d6ffb 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -40,14 +40,7 @@ func (k *Kubernetes) NewHelm() *helm.Helm { } // NewKiali returns a Kiali client initialized with the same StaticConfig and bearer token -// as the underlying Kubernetes manager. The token is taken from the manager rest.Config. +// as the underlying derived Kubernetes manager. func (k *Kubernetes) NewKiali() *kiali.Kiali { - if k == nil || k.manager == nil || k.manager.staticConfig == nil { - return nil - } - km := kiali.NewManager(k.manager.staticConfig) - if k.manager.cfg != nil { - km.BearerToken = k.manager.cfg.BearerToken - } - return km.GetKiali() + return kiali.NewKiali(k.manager.staticConfig, k.manager.cfg) } diff --git a/pkg/mcp/m3labs.go b/pkg/mcp/m3labs.go index db3b7752f..ade0f56be 100644 --- a/pkg/mcp/m3labs.go +++ b/pkg/mcp/m3labs.go @@ -45,6 +45,7 @@ func ServerToolToM3LabsServerTool(s *Server, tools []api.ServerTool) ([]server.S if err != nil { return nil, err } + result, err := tool.Handler(api.ToolHandlerParams{ Context: ctx, Kubernetes: k,