feat(kubevirt): add tool for QEMU guest agent access#811
feat(kubevirt): add tool for QEMU guest agent access#811codingben wants to merge 2 commits intocontainers:mainfrom
Conversation
| description: "Use vm_guest_info to audit which users are currently logged into VMs for security compliance" | ||
| steps: | ||
| setup: | ||
| inline: |- |
There was a problem hiding this comment.
can you please use new format of tasks as e.g. we are using here: #756?
lyarwood
left a comment
lyarwood
left a comment
There was a problem hiding this comment.
Lets start with the evals, the use cases here are far too high level and model dependant for now. Can you simplify these and break them out into their own commits before any tooling is introduced.
| echo " - User 'admin' logged in at 2024-02-25 14:30 ✓" | ||
| echo " - User 'unknown_user' logged in at 2024-02-25 03:00 ⚠ Investigate!" | ||
| echo "" | ||
| echo "✓ Security audit eval complete" |
There was a problem hiding this comment.
Does the judge actually use this? If not it's just slop output that's not asserting anything.
| kubectl delete namespace "$NS" --ignore-not-found | ||
| prompt: | ||
| inline: | | ||
| As part of a security compliance audit, you need to check who is currently logged into the production VirtualMachine "prod-app" in the ${EVAL_NAMESPACE:-vm-test} namespace. |
There was a problem hiding this comment.
I like idea of eventually testing use cases like this but we really need to start with simple building blocks. Something like "list defined users in the VM" and asserting the returned list etc given the image used.
| kubectl delete namespace "$NS" --ignore-not-found | ||
| prompt: | ||
| inline: | | ||
| A user reports that they cannot connect to the web server running inside the VirtualMachine named "web-server" in the ${EVAL_NAMESPACE:-vm-test} namespace. |
There was a problem hiding this comment.
Again just simplify and fetch and assert the IP?
Add five simple evaluation tasks that test basic vm_guest_info functionality: - get-vm-filesystems: Get filesystem information from inside a VM - get-vm-ip-address: Get IP address from inside the guest OS - get-vm-os-info: Get OS name and version from a VM - list-vm-users: List currently logged-in users in a VM - vm-guest-info: General test for retrieving guest agent information These evals focus on testing the basic building blocks rather than high-level use cases, making them more reliable and model-independent. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Ben Oukhanov <boukhanov@redhat.com>
Add new vm_guest_info tool that enables querying information from inside VirtualMachines using the QEMU guest agent, without requiring SSH access or credentials. The tool supports querying: - os: Operating system information (name, version, kernel, hostname) - filesystem: Mounted filesystems and disk usage - network: Network interfaces and IP addresses - users: Currently logged-in users and sessions This provides a secure way to gather runtime information from VMs for monitoring, troubleshooting, and compliance purposes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Ben Oukhanov <boukhanov@redhat.com>
codingben
force-pushed
the
add-guest-info
branch
from
cf6615c to
6d2456a
Compare
|
|
||
| // getGuestOSInfo retrieves operating system information from the guest agent | ||
| func getGuestOSInfo(ctx context.Context, dynamicClient dynamic.Interface, namespace, name string) (map[string]any, error) { | ||
| gvr := schema.GroupVersionResource{ |
There was a problem hiding this comment.
This GVR is defined 4x times in this file. Can you export it as package variable?
|
|
||
| // Collect all info types, but don't fail if one is unavailable | ||
| osInfo, err := getGuestOSInfo(ctx, dynamicClient, namespace, name) | ||
| if err == nil { |
There was a problem hiding this comment.
Nit - I would prefer here the normal go error handling
if err != nil{
...
} else {
...
}
| result[k] = v | ||
| } | ||
| } else { | ||
| result["guestOSInfo"] = map[string]string{"error": err.Error()} |
There was a problem hiding this comment.
Why are you saving here the error, and on line 261 you are replacing it with general error? Would it make sense to create error from returned errors?
|
|
||
| // getAllGuestInfo retrieves all available guest agent information | ||
| func getAllGuestInfo(ctx context.Context, dynamicClient dynamic.Interface, namespace, name string) (map[string]any, error) { | ||
| result := make(map[string]any) |
There was a problem hiding this comment.
Would it make sense to create regular struct instead of map[string]any?
| } | ||
|
|
||
| // If all failed, return an error | ||
| if len(result) == 4 && |
There was a problem hiding this comment.
If just e.g. 1 guest info query fails do we want to still capture it as success?
| }) | ||
| s.Nilf(err, "call tool failed %v", err) | ||
| s.Truef(toolResult.IsError, "expected call tool to fail for non-existent VM") | ||
| s.Contains(toolResult.Content[0].(*mcp.TextContent).Text, "VirtualMachineInstance not found") |
There was a problem hiding this comment.
Should this not say VirtualMachine not found. There is a distinct difference between a VirtualMachine and VirtualMachineInstance. One can start a VMI without a matching VM, but the description here say no VM, not no VMI (which can be either, not running VM, or no VMI)
4 participants
Implements vm_guest_info tool to retrieve information from inside running VMs via QEMU guest agent without requiring SSH credentials.
Assisted-By: Claude noreply@anthropic.com