Conversation
Signed-off-by: Samantha Coyle <sam@diagrid.io>
There was a problem hiding this comment.
Pull request overview
Adds a new Kubernetes-mode CLI command (dapr mcpservers -k) to list Dapr MCPServer CRD resources, aligning output/flags with existing components and configurations listing patterns.
Changes:
- Introduces Kubernetes client helpers to list/print MCPServer resources with table and JSON/YAML output.
- Adds a new
dapr mcpserversCobra command with--name,--namespace,--all-namespaces, and-oflags. - Adds unit tests for listing/filtering behavior and endpoint-derived fields (transport/URL).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
pkg/kubernetes/mcpservers.go |
Implements listing, filtering, table output, and JSON/YAML detail output for MCPServers. |
pkg/kubernetes/mcpservers_test.go |
Adds tests for write path, transport detection, and URL derivation. |
cmd/mcpservers.go |
Adds the new dapr mcpservers CLI command and flags wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Name string `json:"name"` | ||
| Namespace string `json:"namespace"` | ||
| Spec v1alpha1.MCPServerSpec `json:"spec"` |
There was a problem hiding this comment.
For consistency with other *DetailedOutput structs used by utils.PrintDetail (e.g., configurationDetailedOutput), include yaml tags alongside json tags here so YAML output is guaranteed to have the intended field names regardless of the YAML marshaller implementation.
| Name string `json:"name"` | |
| Namespace string `json:"namespace"` | |
| Spec v1alpha1.MCPServerSpec `json:"spec"` | |
| Name string `json:"name" yaml:"name"` | |
| Namespace string `json:"namespace" yaml:"namespace"` | |
| Spec v1alpha1.MCPServerSpec `json:"spec" yaml:"spec"` |
| // ListMCPServers lists MCPServer resources from Kubernetes. | ||
| func ListMCPServers(client versioned.Interface, namespace string) (*v1alpha1.MCPServerList, error) { | ||
| list, err := client.MCPServerV1alpha1().MCPServers(namespace).List(meta_v1.ListOptions{}) | ||
| // This means that the Dapr MCPServer CRD is not installed and | ||
| // therefore no MCPServer items exist. | ||
| if apierrors.IsNotFound(err) { | ||
| list = &v1alpha1.MCPServerList{ | ||
| Items: []v1alpha1.MCPServer{}, | ||
| } | ||
| } else if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return list, nil |
There was a problem hiding this comment.
This introduces a dependency on the MCPServer typed client (client.MCPServerV1alpha1()), but this repo’s current github.com/dapr/dapr module version may not include the generated clientset for MCPServer yet (as noted in the PR description). Please update go.mod (or add a temporary replace) to a dapr/dapr commit that contains the MCPServer clientset changes, otherwise this will not compile in CI.
| testCases := []struct { | ||
| name string | ||
| serverName string | ||
| outputFormat string | ||
| errorExpected bool | ||
| errString string | ||
| mcpServers []v1alpha1.MCPServer | ||
| expectedOutput string |
There was a problem hiding this comment.
The test case struct includes errString, but the test always returns assert.AnError and never asserts the error message. Either remove errString entirely or return errors.New(tc.errString) and assert err.Error() matches, so the intent of the test is reflected in the assertions.
| var McpserversCmd = &cobra.Command{ | ||
| Use: "mcpservers", | ||
| Short: "List all Dapr MCPServer resources. Supported platforms: Kubernetes", |
There was a problem hiding this comment.
Go naming for initialisms typically keeps them fully-capitalized (e.g., MTLSCmd). Consider renaming McpserversCmd to MCPServersCmd (and file-local references accordingly) to match the established command naming style.
Description
Summary
dapr mcpservers -kcommand to list MCPServer CRD resources in Kubernetes--name,--namespace,--all-namespaces, and-o json/yamlflagsdapr components -kanddapr configurations -kThis will fail until my upstream PR here is merged with the clientset changes required for this:
dapr/dapr#9742
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: