[pve-devel] [PATCH storage/qemu-server/pve-manager] implement ova/ovf import for directory type storages

[Fabian Grünbichler]

> Dominik Csapak <d.csapak at proxmox.com> hat am 17.04.2024 15:19 CEST geschrieben:
> On 4/17/24 15:11, Fabian Grünbichler wrote:
> > On April 16, 2024 3:18 pm, Dominik Csapak wrote:
> >> This series enables importing ova/ovf from directory based storages,
> >> inclusive upload/download via the webui (ova only).
> >>
> >> It also improves the ovf importer by parsing the ostype, nics, bootorder
> >> (and firmware from vmware exported files).
> >>
> >> I currently opted to move the OVF.pm to pve-storage, since there is no
> >> real other place where we could put it. Building a seperate package
> >> from qemu-servers git repo would also not be ideal, since we still
> >> have a cyclic dev dependency then
> >> (If someone has a better idea how to handle that, please do tell, and
> >> i can do that in a v2)
> >>
> >> There are surely some wrinkles left i did not think of, but all in all,
> >> it should be pretty usable. E.g. i downloaded some ovas, uploaded them
> >> on my cephfs in my virtual cluster, and successfully imported that with
> >> live-import.
> >>
> >> The biggest caveat when importing from ovas is that we have to
> >> temporarily extract the disk images. I opted for doing that into the
> >> import storage, but if we have a better idea where to put that, i can
> >> implement it in a v2 (or as a follow up). For example, we could add a
> >> new 'tmpdir' parameter to the create call and use that for extractig.
> > 
> > something is wrong with the permissions, since the import images are not
> > added to check_volume_access, I can now upload an OVA, but not see it
> > afterwards ;)
> > 
> > I guess if a user has upload rights for improt images
> > (Datastore.AllocateTemplate), they should also be able to see and use
> > (and remove) import images?
> > 
> 
> ah yes, i forgot to add it there.
> 
> but FWICS isos can have the same problem?
> upload only requires 'Datastore.AllocateTemplate' but seeing them requires
> 'Datastore.AllocateSpace' or 'Datastore.Audit'
> 
> is that a mistake?

that's a slightly less problematic variant of a similar issue, yes. Datastore.AllocateSpace and Datastore.Audit are the "weaker cousins" of Datastore.AllocateTemplate, in most configurations if you have the latter you'll also have (one of) the former. IMHO it wouldn't hurt to allow Datastore.AllocateTemplate users access to iso files (and container templates), since they can upload them the behaviour is weird as it is.

for the OVA files right now you'd need Datastore.Allocate, which is a higher privilege than those others. I guess treating OVA files like iso and templates w.r.t. ACLs kind of makes sense, even if they have a slightly bigger attack surface behind the scenes. this would also allow just giving trusted admins the option to upload new OVAs, while allowing users to create VMs based on that trusted set.