[pve-devel] [PATCH qemu-server] feature #3937: config: store user in meta property

[pve-devel] [PATCH qemu-server] feature #3937: config: store user in meta property

[Leo Nunner]

Adds a field to the "meta" config property which stores the user who
created the VM.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 PVE/QemuServer.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a0e16dc..28ed8e7 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -281,6 +281,11 @@ my $meta_info_fmt = {
 	pattern => '\d+(\.\d+)+',
 	optional => 1,
     },
+    'user' => {
+	type => 'string',
+	description => "The user who created the VM.",
+	optional => 1,
+    },
 };
 
 my $confdesc = {
@@ -2184,10 +2189,13 @@ sub parse_meta_info {
 sub new_meta_info_string {
     my () = @_; # for now do not allow to override any value
 
+    my $rpcenv = PVE::RPCEnvironment->get();
+
     return PVE::JSONSchema::print_property_string(
 	{
 	    'creation-qemu' => kvm_user_version(),
 	    ctime => "". int(time()),
+	    user => $rpcenv->get_user(),
 	},
 	$meta_info_fmt
     );
-- 
2.30.2

Re: [pve-devel] [PATCH qemu-server] feature #3937: config: store user in meta property

[Thomas Lamprecht]

On 13/02/2023 11:24, Leo Nunner wrote:
> Adds a field to the "meta" config property which stores the user who
> created the VM.

Should also get this finally added to CTs, I know it's a bit unfair to
add the burden to this patch series, but otherwise we might never add
it.. 

> 
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
>  PVE/QemuServer.pm | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a0e16dc..28ed8e7 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -281,6 +281,11 @@ my $meta_info_fmt = {
>  	pattern => '\d+(\.\d+)+',
>  	optional => 1,
>      },
> +    'user' => {

It adds a bit of property length, but it might be good to follow the other
properties and use a bit more self-explanatory 'creation-user' here?

I mean, I don't hope that we add to much properties here, but in retrospect
the property name "meta" might have been a bit to general, something like
"creation-env" could have been a better choice - but as said, I still
hope that we don't add to much there anyway.

otoh, maybe this is even "to much" for such a thing, a dedicated audit log
might be better in general? (I got that with some rough planning on our
internal wiki) 

Re: [pve-devel] [PATCH qemu-server] feature #3937: config: store user in meta property

[Leo Nunner]

On 2023-02-14 10:41, Thomas Lamprecht wrote:
> On 13/02/2023 11:24, Leo Nunner wrote:
>> Adds a field to the "meta" config property which stores the user who
>> created the VM.
> Should also get this finally added to CTs, I know it's a bit unfair to
> add the burden to this patch series, but otherwise we might never add
> it.. 
>
>> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
>> ---
>>  PVE/QemuServer.pm | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index a0e16dc..28ed8e7 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -281,6 +281,11 @@ my $meta_info_fmt = {
>>  	pattern => '\d+(\.\d+)+',
>>  	optional => 1,
>>      },
>> +    'user' => {
> It adds a bit of property length, but it might be good to follow the other
> properties and use a bit more self-explanatory 'creation-user' here?

Good idea, I'll change it accordingly.

> I mean, I don't hope that we add to much properties here, but in retrospect
> the property name "meta" might have been a bit to general, something like
> "creation-env" could have been a better choice - but as said, I still
> hope that we don't add to much there anyway.
>
> otoh, maybe this is even "to much" for such a thing, a dedicated audit log
> might be better in general? (I got that with some rough planning on our
> internal wiki) 

FWIW, I actually started working on the CT implementation already (after
Fiona pointed out that the report was requesting it for both VMs and
CTs). I read through the ideas for the auditing framework and it does
seem like it would be a much better fit there - but then again, the
property might make accountability a bit easier, since instead of
filtering logs (which could probably be quite some time in the past) one
just needs to read the meta property in the config file… Maybe both
would be good?