Skip to content

Commit 822bb57

Browse files
krutskokrutsko
committed
fix: get-dr by name
1 parent 1654c13 commit 822bb57

File tree

5 files changed

+239
-22
lines changed

5 files changed

+239
-22
lines changed

‎pkg/istio/destination_rules_test.go‎

Lines changed: 180 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,116 @@ func TestGetDestinationRulesContextCancellation(t *testing.T) {
230230
})
231231
}
232232

233+
// TestGetDestinationRule tests the GetDestinationRule method with various scenarios
234+
func TestGetDestinationRule(t *testing.T) {
235+
// Create a mock server that simulates Istio API responses
236+
mockServer := createMockDestinationRuleServer()
237+
defer mockServer.Close()
238+
239+
// Create temporary directory and kubeconfig
240+
tempDir, err := os.MkdirTemp("", "istio-dr-test-*")
241+
if err != nil {
242+
t.Fatalf("Failed to create temp dir: %v", err)
243+
}
244+
defer os.RemoveAll(tempDir)
245+
246+
kubeconfigPath := filepath.Join(tempDir, "config")
247+
kubeconfig := createTestKubeconfigForDR(mockServer.URL)
248+
249+
if err := os.WriteFile(kubeconfigPath, []byte(kubeconfig), 0644); err != nil {
250+
t.Fatalf("Failed to write kubeconfig: %v", err)
251+
}
252+
253+
// Create Istio client
254+
istio, err := NewIstio(kubeconfigPath)
255+
if err != nil {
256+
t.Fatalf("Failed to create Istio client: %v", err)
257+
}
258+
defer istio.Close()
259+
260+
ctx := context.Background()
261+
262+
t.Run("GetDestinationRule with specific DestinationRule name", func(t *testing.T) {
263+
result, err := istio.GetDestinationRule(ctx, "production", "payment-dr")
264+
if err != nil {
265+
t.Fatalf("Failed to get destination rule: %v", err)
266+
}
267+
268+
// Verify result contains expected information
269+
if !strings.Contains(result, "Destination Rule 'payment-dr'") {
270+
t.Errorf("Expected result to contain 'Destination Rule 'payment-dr'', got: %s", result)
271+
}
272+
273+
if !strings.Contains(result, "namespace 'production'") {
274+
t.Errorf("Expected result to mention namespace 'production', got: %s", result)
275+
}
276+
})
277+
278+
t.Run("GetDestinationRule result format validation", func(t *testing.T) {
279+
result, err := istio.GetDestinationRule(ctx, "production", "payment-dr")
280+
if err != nil {
281+
t.Fatalf("Failed to get destination rule: %v", err)
282+
}
283+
284+
// Check that result has proper structure
285+
lines := strings.Split(result, "\n")
286+
if len(lines) < 1 {
287+
t.Error("Result should have at least one line")
288+
}
289+
290+
// First line should contain DestinationRule name and namespace info
291+
firstLine := lines[0]
292+
if !strings.Contains(firstLine, "Destination Rule 'payment-dr'") || !strings.Contains(firstLine, "namespace 'production'") {
293+
t.Errorf("First line should contain DestinationRule information, got: %s", firstLine)
294+
}
295+
296+
// Result should end with newline for proper formatting
297+
if !strings.HasSuffix(result, "\n") {
298+
t.Error("Result should end with newline for proper formatting")
299+
}
300+
})
301+
}
302+
303+
// TestGetDestinationRuleErrorHandling tests error scenarios
304+
func TestGetDestinationRuleErrorHandling(t *testing.T) {
305+
// Create a mock server that returns errors
306+
mockServer := createErrorMockServer()
307+
defer mockServer.Close()
308+
309+
tempDir, err := os.MkdirTemp("", "istio-dr-error-test-*")
310+
if err != nil {
311+
t.Fatalf("Failed to create temp dir: %v", err)
312+
}
313+
defer os.RemoveAll(tempDir)
314+
315+
kubeconfigPath := filepath.Join(tempDir, "config")
316+
kubeconfig := createTestKubeconfigForDR(mockServer.URL)
317+
318+
if err := os.WriteFile(kubeconfigPath, []byte(kubeconfig), 0644); err != nil {
319+
t.Fatalf("Failed to write kubeconfig: %v", err)
320+
}
321+
322+
istio, err := NewIstio(kubeconfigPath)
323+
if err != nil {
324+
t.Fatalf("Failed to create Istio client: %v", err)
325+
}
326+
defer istio.Close()
327+
328+
ctx := context.Background()
329+
330+
t.Run("GetDestinationRule handles API errors gracefully", func(t *testing.T) {
331+
_, err := istio.GetDestinationRule(ctx, "default", "nonexistent-dr")
332+
if err == nil {
333+
t.Fatal("Expected error when DestinationRule doesn't exist")
334+
}
335+
336+
// Error should be wrapped with context
337+
if !strings.Contains(err.Error(), "failed to get destination rule") {
338+
t.Errorf("Expected error to be wrapped with context, got: %v", err)
339+
}
340+
})
341+
}
342+
233343
// Helper functions for creating mock servers
234344

235345
func createMockDestinationRuleServer() *httptest.Server {
@@ -240,14 +350,72 @@ func createMockDestinationRuleServer() *httptest.Server {
240350
w.Header().Set("Content-Type", "application/json")
241351
w.WriteHeader(http.StatusOK)
242352

243-
// Extract namespace from URL path
353+
// Extract namespace and resource name from URL path
244354
namespace := extractNamespaceFromPath(r.URL.Path)
355+
resourceName := extractResourceNameFromPath(r.URL.Path)
245356

246357
var response string
247-
switch namespace {
248-
case "production":
249-
// Mock response with destination rules
250-
response = `{
358+
359+
// Handle GET request for specific DestinationRule
360+
if resourceName != "" {
361+
switch {
362+
case namespace == "production" && resourceName == "payment-dr":
363+
response = `{
364+
"apiVersion": "networking.istio.io/v1alpha3",
365+
"kind": "DestinationRule",
366+
"metadata": {
367+
"name": "payment-dr",
368+
"namespace": "production"
369+
},
370+
"spec": {
371+
"host": "api.payment.com",
372+
"trafficPolicy": {
373+
"loadBalancer": {
374+
"simple": "ROUND_ROBIN"
375+
},
376+
"connectionPool": {
377+
"tcp": {
378+
"maxConnections": 10
379+
}
380+
},
381+
"outlierDetection": {
382+
"consecutiveErrors": 3
383+
}
384+
},
385+
"subsets": [
386+
{
387+
"name": "v1",
388+
"labels": {
389+
"version": "v1"
390+
}
391+
}
392+
]
393+
}
394+
}`
395+
default:
396+
// Return 404 for non-existent DestinationRule
397+
w.WriteHeader(http.StatusNotFound)
398+
response = `{
399+
"kind": "Status",
400+
"apiVersion": "v1",
401+
"metadata": {},
402+
"status": "Failure",
403+
"message": "destinationrules.networking.istio.io \"` + resourceName + `\" not found",
404+
"reason": "NotFound",
405+
"details": {
406+
"name": "` + resourceName + `",
407+
"group": "networking.istio.io",
408+
"kind": "destinationrules"
409+
},
410+
"code": 404
411+
}`
412+
}
413+
} else {
414+
// Handle LIST request for all DestinationRules
415+
switch namespace {
416+
case "production":
417+
// Mock response with destination rules
418+
response = `{
251419
"apiVersion": "networking.istio.io/v1alpha3",
252420
"kind": "DestinationRuleList",
253421
"items": [
@@ -284,20 +452,21 @@ func createMockDestinationRuleServer() *httptest.Server {
284452
}
285453
]
286454
}`
287-
case "empty-namespace":
288-
// Mock response with no destination rules
289-
response = `{
455+
case "empty-namespace":
456+
// Mock response with no destination rules
457+
response = `{
290458
"apiVersion": "networking.istio.io/v1alpha3",
291459
"kind": "DestinationRuleList",
292460
"items": []
293461
}`
294-
default:
295-
// Default response for other namespaces
296-
response = `{
462+
default:
463+
// Default response for other namespaces
464+
response = `{
297465
"apiVersion": "networking.istio.io/v1alpha3",
298466
"kind": "DestinationRuleList",
299467
"items": []
300468
}`
469+
}
301470
}
302471

303472
w.Write([]byte(response))

‎pkg/istio/istio.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,40 @@ func (i *Istio) GetDestinationRules(ctx context.Context, namespace string) (stri
137137
return result, nil
138138
}
139139

140+
// GetDestinationRule retrieves a specific Destination Rule by name from the specified namespace
141+
func (i *Istio) GetDestinationRule(ctx context.Context, namespace string, drName string) (string, error) {
142+
dr, err := i.istioClient.NetworkingV1alpha3().DestinationRules(namespace).Get(ctx, drName, metav1.GetOptions{})
143+
if err != nil {
144+
return "", fmt.Errorf("failed to get destination rule '%s': %w", drName, err)
145+
}
146+
147+
result := fmt.Sprintf("Destination Rule '%s' in namespace '%s':\n", dr.Name, namespace)
148+
result += fmt.Sprintf("- %s\n", dr.Name)
149+
if dr.Spec.Host != "" {
150+
result += fmt.Sprintf(" Host: %s\n", dr.Spec.Host)
151+
}
152+
if dr.Spec.TrafficPolicy != nil {
153+
if dr.Spec.TrafficPolicy.LoadBalancer != nil {
154+
result += fmt.Sprintf(" Load Balancer: %s\n", dr.Spec.TrafficPolicy.LoadBalancer.GetSimple().String())
155+
}
156+
if dr.Spec.TrafficPolicy.ConnectionPool != nil {
157+
result += " Connection Pool: configured\n"
158+
}
159+
if dr.Spec.TrafficPolicy.OutlierDetection != nil {
160+
result += " Outlier Detection: configured\n"
161+
}
162+
}
163+
if len(dr.Spec.Subsets) > 0 {
164+
result += fmt.Sprintf(" Subsets: %d\n", len(dr.Spec.Subsets))
165+
for _, subset := range dr.Spec.Subsets {
166+
result += fmt.Sprintf(" - %s\n", subset.Name)
167+
}
168+
}
169+
result += "\n"
170+
171+
return result, nil
172+
}
173+
140174
func (i *Istio) GetServiceEntries(ctx context.Context, namespace string) (string, error) {
141175
seList, err := i.istioClient.NetworkingV1alpha3().ServiceEntries(namespace).List(ctx, metav1.ListOptions{})
142176
if err != nil {

‎pkg/istio/istio_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,10 @@ func extractNamespaceFromPath(path string) string {
367367

368368
func extractResourceNameFromPath(path string) string {
369369
// Extract resource name from path like: /apis/networking.istio.io/v1alpha3/namespaces/default/virtualservices/bookinfo-vs
370+
// or: /apis/networking.istio.io/v1alpha3/namespaces/default/destinationrules/payment-dr
370371
parts := strings.Split(path, "/")
371372
for i, part := range parts {
372-
if part == "virtualservices" && i+1 < len(parts) {
373+
if (part == "virtualservices" || part == "destinationrules") && i+1 < len(parts) {
373374
return parts[i+1]
374375
}
375376
}

‎pkg/mcp/profile.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,20 @@ func (s *Server) initNetworkingTools() []server.ServerTool {
7777
Handler: s.getVirtualService,
7878
},
7979
{
80-
Tool: mcp.NewTool("get-destination-rules",
81-
mcp.WithDescription("Get Istio Destination Rules from any namespace. Destination Rules define policies for traffic to services, including load balancing, connection pooling, and outlier detection. Essential for understanding service mesh traffic policies."),
80+
Tool: mcp.NewTool("get-destination-rule",
81+
mcp.WithDescription("Get a specific Istio Destination Rule configuration by name. Destination Rules define policies for traffic to services, including load balancing, connection pooling, and outlier detection. Use this to inspect detailed configuration of a specific DestinationRule."),
8282
mcp.WithString("namespace",
83-
mcp.Description("Namespace to query (defaults to 'default'). Check multiple namespaces for complete Istio configuration."),
83+
mcp.Description("Namespace to query (defaults to 'default'). Istio services can span multiple namespaces."),
84+
),
85+
mcp.WithString("destination-rule-name",
86+
mcp.Description("DestinationRule resource name to get"),
87+
mcp.Required(),
8488
),
85-
mcp.WithTitleAnnotation("Istio: Destination Rules"),
89+
mcp.WithTitleAnnotation("Istio: Destination Rule"),
8690
mcp.WithReadOnlyHintAnnotation(true),
8791
mcp.WithDestructiveHintAnnotation(false),
8892
),
89-
Handler: s.getDestinationRules,
93+
Handler: s.getDestinationRule,
9094
},
9195
{
9296
Tool: mcp.NewTool("get-service-entries",
@@ -321,12 +325,21 @@ func (s *Server) getVirtualService(ctx context.Context, ctr mcp.CallToolRequest)
321325
return NewTextResult(content, err), nil
322326
}
323327

324-
func (s *Server) getDestinationRules(ctx context.Context, ctr mcp.CallToolRequest) (*mcp.CallToolResult, error) {
328+
func (s *Server) getDestinationRule(ctx context.Context, ctr mcp.CallToolRequest) (*mcp.CallToolResult, error) {
325329
namespace := "default"
326330
if ns := ctr.GetArguments()["namespace"]; ns != nil {
327331
namespace = ns.(string)
328332
}
329-
content, err := s.i.GetDestinationRules(ctx, namespace)
333+
334+
drName := ""
335+
if dr := ctr.GetArguments()["destination-rule-name"]; dr != nil {
336+
drName = dr.(string)
337+
}
338+
if drName == "" {
339+
return NewTextResult("", fmt.Errorf("destination-rule-name is required")), nil
340+
}
341+
342+
content, err := s.i.GetDestinationRule(ctx, namespace, drName)
330343
return NewTextResult(content, err), nil
331344
}
332345

‎pkg/mcp/profile_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func TestFullProfile(t *testing.T) {
9292
for _, tool := range tools {
9393
toolName := tool.Tool.Name
9494
switch {
95-
case toolName == "get-virtual-service" || toolName == "get-destination-rules":
95+
case toolName == "get-virtual-service" || toolName == "get-destination-rule":
9696
categories["networking"] = true
9797
// No configuration tools currently available
9898
case toolName == "get-proxy-clusters" || toolName == "get-proxy-listeners" || toolName == "get-proxy-routes":
@@ -134,7 +134,7 @@ func TestToolAnnotations(t *testing.T) {
134134
t.Run("read-only tools are properly annotated", func(t *testing.T) {
135135
readOnlyTools := []string{
136136
"get-virtual-service",
137-
"get-destination-rules",
137+
"get-destination-rule",
138138
"get-service-entries",
139139
"get-proxy-clusters",
140140
"get-proxy-bootstrap",
@@ -250,7 +250,7 @@ func TestToolParameters(t *testing.T) {
250250
t.Run("namespace tools have namespace parameter", func(t *testing.T) {
251251
namespaceTools := []string{
252252
"get-virtual-service",
253-
"get-destination-rules",
253+
"get-destination-rule",
254254
"get-service-entries",
255255
}
256256

0 commit comments

Comments
 (0)