feat(v1/proxmox): storage + per-VM snapshot endpoints (direct httpx)#99
Conversation
Adds StoragePool schema and a direct-PVE httpx route that returns a paginated list of storage pools from a registered Proxmox host.
…rl scheme/filename
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35ee70a318
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if body.vmstate is not None: | ||
| form["vmstate"] = 1 if body.vmstate else 0 |
There was a problem hiding this comment.
When callers create an LXC snapshot (?vmtype=lxc) and include vmstate, this branch forwards vmstate to /lxc/.../snapshot; PVE rejects that qemu-only option, so the newly advertised LXC snapshot path returns a 502 instead of creating the snapshot. The current community.proxmox implementation handles the same distinction by omitting vmstate for vm['type'] == 'lxc' (https://raw.githubusercontent.com/ansible-collections/community.proxmox/main/plugins/modules/proxmox_snap.py), so gate this field on vmtype == "qemu" or reject the combination before calling PVE.
Useful? React with 👍 / 👎.
PVE rejects vmstate on /lxc/.../snapshot, so forwarding it broke the vmtype=lxc create path (502). Gate the field on vmtype==qemu, matching community.proxmox proxmox_snap behavior.
1 participant
Summary
Adds v1 direct-httpx Proxmox endpoints for storage and per-VM snapshots, replacing the v0 Ansible-backed equivalents. Advances #87 (migrate remaining v0 admin/proxmox ops to v1). All endpoints address Proxmox by registered
host_id(node derived from the host record — noproxmox_nodeparam) and talk to the PVE REST API directly, mirroring the existingvms.py.First commit extracts the shared host/PVE helpers (
_session,_get_host,_auth_headers,_assert_vmid_safe,_unreachable, new_raise_for_pve) into_helpers.py, removing prior duplication betweenvms.pyandhosts.py.Endpoints
Storage:
GET /v1/proxmox/hosts/{host_id}/storage→Page[StoragePool]GET /v1/proxmox/hosts/{host_id}/storage/{store}/content?content=iso|vztmpl→Page[StorageContent](derivesnamefromvolid— PVE doesn't return it)POST /v1/proxmox/hosts/{host_id}/storage/{store}/download-url→VmActionResult(UPID)Per-VM snapshots (qemu + lxc):
GET /v1/proxmox/hosts/{host_id}/vms/{vmid}/snapshots→Page[SnapshotItem](filters PVE's syntheticcurrent)POST /v1/proxmox/hosts/{host_id}/vms/{vmid}/snapshots→VmActionResult(UPID)DELETE /v1/proxmox/hosts/{host_id}/vms/{vmid}/snapshots/{name}→VmActionResult(UPID)POST /v1/proxmox/hosts/{host_id}/vms/{vmid}/snapshots/{name}/rollback→VmActionResult(UPID) — vmid-guarded (protected VMIDs refused before any outbound call)Security hardening
A review pass added input validation:
Path(pattern=...)constraints on{store}and{name}(block path-segment injection into outbound URLs), an http(s)-only scheme guard ondownload-urlurl(SSRF), and a bare-filename guard onfilename(file-plant).Notes
httpx.AsyncClient(verify=False)matches existingvms.py/hosts.py; TLS pinning tracked separately in Pin Proxmox TLS (ssl_fingerprint) instead of verify=False #85 (out of scope here).vmtype=lxcand thevmstateflag are net-new vs v0 (v0 was qemu-only).Tests
tests/routes/test_proxmox_{storage,snapshots,vms,hosts}.py), house pattern (_boottemp-sqlite + fakedhttpx.AsyncClient).test_alembic,test_expand_replication) are pre-existing — reproduced at the base commit and untouched by this diff (which is confined to 9 proxmox files).Follow-up (separate PR)
deployer-ui: repoint
storage.*/snapshot.*insrc/services/proxmox/api.tsat these v1 paths (.items-unwrap thePage), dropproxmox_nodeargs.