feat(kubevirt): add vm_create_from_template tool - #1321
Conversation
|
👋 Heads up — this pull request changes files owned by @ksimon1 @lyarwood. You are listed as an owner of one or more of the changed areas in |
| return nil, fmt.Errorf("failed to marshal ProcessOptions: %w", err) | ||
| } | ||
|
|
||
| result := &unstructured.Unstructured{} |
There was a problem hiding this comment.
You could perhaps use the types from virt-template-api package instead of unstructured
There was a problem hiding this comment.
We currently do not have any kubevirt package imported in this project. Might be worth checking if virt-template package is easy to import in this PR
There was a problem hiding this comment.
@lyarwood can drop your opinion about this?
There was a problem hiding this comment.
Something for a follow up but I'd assume we want to stay unstructured for now.
| | [Kiali](https://kiali.io) | `kiali` | 16 | | ||
| | [Kubernetes](https://kubernetes.io) | - | 32 | | ||
| | [KubeVirt](https://kubevirt.io) | `kubevirt`, `tekton` | 19 | | ||
| | [KubeVirt](https://kubevirt.io) | `kubevirt`, `tekton` | 24 | |
There was a problem hiding this comment.
Is this number correct? Since you added just a single tool
There was a problem hiding this comment.
It's auto-generated by make update-readme-tools and represents the amount of KubeVirt eval scenarios and not the tools.
|
You should probably add similar test as is here: https://github.com/containers/kubernetes-mcp-server/blob/main/pkg/mcp/toolsets_test.go#L129 |
lyarwood
left a comment
There was a problem hiding this comment.
Thanks for this @codingben, a few pieces of feedback:
Rebase on #1308 and route through AccessControlRoundTripper
CreateVMFromTemplate in pkg/kubevirt/template.go creates its own REST client via rest.CopyConfig(restConfig), which means the request to subresources.template.kubevirt.io completely bypasses the AccessControlRoundTripper. This is the same pattern as the existing guest agent code in guestagent.go, but I think we should take the opportunity to fix this going forward.
#1308 adds AllowedAPIGroups support to the toolset API — toolsets can declare virtual API groups (like subresources.kubevirt.io) that should be allowed through the AccessControlRoundTripper even though they're not in standard API discovery. This PR would be a great second use case for that mechanism.
I'd suggest:
- Rebase this PR on #1308
- Add
"subresources.template.kubevirt.io"to the kubevirt toolset'sGetAllowedAPIGroups()return value - Rework
CreateVMFromTemplateto go through the standard Kubernetes client (which has theAccessControlRoundTripperinstalled) rather than building its ownrest.RESTClientFor
That way the request goes through the same access control path as everything else — denied_resources, confirmation rules, and any future validators would all apply. The AllowedAPIGroups mechanism lets it past the REST mapper check, and the API server still enforces RBAC on the user's credentials.
Evals in a separate commit
I'd like to see the eval tasks (the 5 task YAMLs + verify-vm.sh helpers) in their own commit, separate from the tool implementation. This way the evals demonstrate and justify the need for the new vm_create_from_template tool before the implementation commit adds it. It also makes it easier to review the evals and tool independently.
Note that only 2 of the 5 evals (create-vm-from-template, template-required-params) actually exercise the new tool — the other 3 (list-vm-templates, inspect-vm-template, create-template-from-vm) test the agent's ability to work with template CRDs via existing generic tools, which is fine for broader coverage but worth calling out.
|
@lyarwood I'd like to avoid rebasing this PR on yours, can we get the PR you opened merged faster instead? It seems not much changes, no reason to keep it open for longer. |
|
@lyarwood Your PR is closed now. How we should proceed here? |
|
This PR is now blocked by: #1361 |
|
PR rebased. @lyarwood PTAL |
Add MCP tool to create VirtualMachines from VirtualMachineTemplates via the virt-template process subresource API, with eval tasks and tests. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Ben Oukhanov <boukhanov@redhat.com>
Add MCP tool to create VirtualMachines from
VirtualMachineTemplates via the virt-template
process subresource API, with eval tasks
and tests.