[pve-devel] New API endpoint to manage snippets

[pve-devel] New API endpoint to manage snippets

[Patryk Ściborek]

Hi Guys!

I'm using Terraform to manage VMs running on Proxmox. I need to be able to
upload snippets with custom cloud-init configuration. Right now I have to
use scp/sftp to upload snippets so I have to use a system account. It looks
that I'm not the only one having this issue:
https://bugzilla.proxmox.com/show_bug.cgi?id=2208

So I thought that I could implement a new API endpoint which would allow to
create, read, update and delete snippets so other tools like Terraform
would be able to use it.

What do you think about this idea?

Best regards,
Patryk

Re: [pve-devel] New API endpoint to manage snippets / bugzilla 2208 / updated patch but missing something, need help

[Laurent GUERBY]

On Thu, 2022-04-07 at 12:27 +0200, Patryk Ściborek wrote:
> Hi Guys!
> 
> I'm using Terraform to manage VMs running on Proxmox. I need to be
> able to
> upload snippets with custom cloud-init configuration. Right now I
> have to
> use scp/sftp to upload snippets so I have to use a system account. It
> looks
> that I'm not the only one having this issue:
> https://bugzilla.proxmox.com/show_bug.cgi?id=2208
> 
> So I thought that I could implement a new API endpoint which would
> allow to
> create, read, update and delete snippets so other tools like
> Terraform
> would be able to use it.
> 
> What do you think about this idea?
> 
> Best regards,
> Patryk

Hi,

I've tried to adapt the patch to current PVE 7.1-12 (see below) but I
still get

proxmoxer.core.ResourceException: 501 Not Implemented: upload failed -
{'errors': b''}

When I try to upload a snippet.

My proxmoxer setup works for iso, the following succeeds:

proxmox.nodes(h).storage(s).upload.post(content="iso",filename=f)

But the following fails (f being read "rb" from some xxx.yaml)

proxmox.nodes(h).storage(s).upload.post(content="snippets",filename=f)

Not having snippet upload makes it impossible to use PVE auth realm
tokens to control permissions, you have to give a priviledged system
account to users *just* to be able to do cloud init with a yaml which
is not great security wise (and not practical).

I'm probably missing a few things to have a patch that works, I'm
willing to put some time on it if someone gives me directions.

Thanks!

Sincerely,

Laurent (paying PVE+PBS customer at work)

root@test:/usr/share/perl5# diff -u PVE/Storage.pm{-orig,}
--- PVE/Storage.pm-orig	2022-04-08 09:15:52.443943197 +0200
+++ PVE/Storage.pm	2022-04-08 09:17:23.457073570 +0200
@@ -412,6 +412,15 @@
     return $plugin->get_subdir($scfg, 'iso');
 }
 
+sub get_snippet_dir {
+    my ($cfg, $storeid) = @_;
+
+    my $scfg = storage_config($cfg, $storeid);
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+    return $plugin->get_subdir($scfg, 'snippets');
+}
+
 sub get_vztmpl_dir {
     my ($cfg, $storeid) = @_;
 
root@test:/usr/share/perl5# diff -u ./PVE/API2/Storage/Status.pm{-orig,}
--- ./PVE/API2/Storage/Status.pm-orig	2022-04-08 09:15:43.883836880 +0200
+++ ./PVE/API2/Storage/Status.pm	2022-04-08 10:23:43.914401204 +0200
@@ -381,7 +381,7 @@
 	    content => {
 		description => "Content type.",
 		type => 'string', format => 'pve-storage-content',
-		enum => ['iso', 'vztmpl'],
+		enum => ['iso', 'vztmpl', 'snippets'],
 	    },
 	    filename => {
 		description => "The name of the file to create. Caution: This will be normalized!",
@@ -446,8 +446,10 @@
 		raise_param_exc({ filename => "wrong file extension" });
 	    }
 	    $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
-	} else {
-	    raise_param_exc({ content => "upload content type '$content' not allowed" });
+	} elsif ($content eq 'snippets') {
+	    $path = PVE::Storage::get_snippet_dir($cfg, $param->{storage});
+        } else {
+            raise_param_exc({ content => "upload content type '$content' not allowed" });
 	}
 
 	die "storage '$param->{storage}' does not support '$content' content\n"
@@ -564,7 +566,7 @@
 	    content => {
 		description => "Content type.", # TODO: could be optional & detected in most cases
 		type => 'string', format => 'pve-storage-content',
-		enum => ['iso', 'vztmpl'],
+		enum => ['iso', 'vztmpl', 'snippets'],
 	    },
 	    filename => {
 		description => "The name of the file to create. Caution: This will be normalized!",
@@ -627,6 +629,8 @@
 		raise_param_exc({ filename => "wrong file extension" });
 	    }
 	    $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
+	} elsif ($content eq 'snippets') {
+	    $path = PVE::Storage::get_snippet_dir($cfg, $storage);    
 	} else {
 	    raise_param_exc({ content => "upload content-type '$content' is not allowed" });
 	}

Re: [pve-devel] New API endpoint to manage snippets / bugzilla 2208 / updated patch but missing something, need help

[Laurent GUERBY]

> Hi,
> 
> I've tried to adapt the patch to current PVE 7.1-12 (see below) but I
> still get
> 
> proxmoxer.core.ResourceException: 501 Not Implemented: upload failed
> -
> {'errors': b''}
> 
> When I try to upload a snippet.
> 
> My proxmoxer setup works for iso, the following succeeds:
> 
> proxmox.nodes(h).storage(s).upload.post(content="iso",filename=f)
> 
> But the following fails (f being read "rb" from some xxx.yaml)
> 
> proxmox.nodes(h).storage(s).upload.post(content="snippets",filename=f
> )
> 
> Not having snippet upload makes it impossible to use PVE auth realm
> tokens to control permissions, you have to give a priviledged system
> account to users *just* to be able to do cloud init with a yaml which
> is not great security wise (and not practical).
> 
> I'm probably missing a few things to have a patch that works, I'm
> willing to put some time on it if someone gives me directions.

I instrumented some PVE perl code and failure to recognize the upload
API data seems to happen somewhere in the logic of
/usr/share/perl5/PVE/APIServer/AnyEvent.pm:file_upload_multipart

With my patch installed for content=snippet filename=yoyobig.yaml if I
upload a 16137 or more byte files it works, but for 16136 byte and
below it fails with 501 not implemented which is suspicious.

By adding max debug to python http libs I was able to check that the
headers are strictly identical except for Content-Length as expected,
see below.

There's a comment in AnyEvent.pm about "# assume we have single line
headers" and also some complex state handling while parsing the request
 but for now I wasn't able to pinpoint what triggers the failure.

Help welcomed :)

Sincerely,

Laurent

Debug logs:

import http.client
http.client.HTTPConnection.debuglevel=5
import logging
logging.basicConfig(stream=sys.stdout, level=logging.DEBUG)


Works:

INFO:proxmoxer.core:POST https://nuc2:8006/api2/json/nodes/nuc2/storage/element2dir/upload {'content': 'snippets', 'filename': <_io.BufferedReader name='yoyobig.yaml'>}
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): nuc2:8006
send: b'POST /api2/json/nodes/nuc2/storage/element2dir/upload HTTP/1.1\r\nHost: nuc2:8006\r\nUser-Agent: python-requests/2.23.0\r\nAccept-Encoding: gzip, deflate\r\naccept: application/json, application/x-javasc
ript, text/javascript, text/x-javascript, text/x-json\r\nConnection: keep-alive\r\nContent-Length: 16385\r\nContent-Type: multipart/form-data; boundary=790f8b2d9dcdc452863bff66e1d262bc\r\nAuthorization: PVEAPITo
ken=root@pam!testpython1=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\r\n\r\n'
send: b'--790f8b2d9dcdc452863bff66e1d262bc\r\nContent-Disposition: form-data; name="content"\r\n\r\nsnippets\r\n--790f8b2d9dcdc452863bff66e1d262bc\r\nContent-Disposition: form-data; name="filename"; filename="yo
yobig.yaml"\r\n\r\n#cloud-config\xhostname: test...

Fails:

INFO:proxmoxer.core:POST https://nuc2:8006/api2/json/nodes/nuc2/storage/element2dir/upload {'content': 'snippets', 'filename': <_io.BufferedReader name='yoyobig.yaml'>}
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): nuc2:8006
send: b'POST /api2/json/nodes/nuc2/storage/element2dir/upload HTTP/1.1\r\nHost: nuc2:8006\r\nUser-Agent: python-requests/2.23.0\r\nAccept-Encoding: gzip, deflate\r\naccept: application/json, application/x-javasc
ript, text/javascript, text/x-javascript, text/x-json\r\nConnection: keep-alive\r\nContent-Length: 16384\r\nContent-Type: multipart/form-data; boundary=1ae9ce4d260e3f514d7c12ec03e4389f\r\nAuthorization: PVEAPITo
ken=root@pam!testpython1=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\r\n\r\n'
send: b'--1ae9ce4d260e3f514d7c12ec03e4389f\r\nContent-Disposition: form-data; name="content"\r\n\r\nsnippets\r\n--1ae9ce4d260e3f514d7c12ec03e4389f\r\nContent-Disposition: form-data; name="filename"; filename="yo
yobig.yaml"\r\n\r\n#cloud-config\nhostname: test...

> Thanks!
> 
> Sincerely,
> 
> Laurent (paying PVE+PBS customer at work)
> 
> root@test:/usr/share/perl5# diff -u PVE/Storage.pm{-orig,}
> --- PVE/Storage.pm-orig	2022-04-08 09:15:52.443943197 +0200
> +++ PVE/Storage.pm	2022-04-08 09:17:23.457073570 +0200
> @@ -412,6 +412,15 @@
>      return $plugin->get_subdir($scfg, 'iso');
>  }
>  
> +sub get_snippet_dir {
> +    my ($cfg, $storeid) = @_;
> +
> +    my $scfg = storage_config($cfg, $storeid);
> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> +
> +    return $plugin->get_subdir($scfg, 'snippets');
> +}
> +
>  sub get_vztmpl_dir {
>      my ($cfg, $storeid) = @_;
>  
> root@test:/usr/share/perl5# diff -u ./PVE/API2/Storage/Status.pm{-
> orig,}
> --- ./PVE/API2/Storage/Status.pm-orig	2022-04-08
> 09:15:43.883836880 +0200
> +++ ./PVE/API2/Storage/Status.pm	2022-04-08 10:23:43.914401204
> +0200
> @@ -381,7 +381,7 @@
>  	    content => {
>  		description => "Content type.",
>  		type => 'string', format => 'pve-storage-content',
> -		enum => ['iso', 'vztmpl'],
> +		enum => ['iso', 'vztmpl', 'snippets'],
>  	    },
>  	    filename => {
>  		description => "The name of the file to create.
> Caution: This will be normalized!",
> @@ -446,8 +446,10 @@
>  		raise_param_exc({ filename => "wrong file extension"
> });
>  	    }
>  	    $path = PVE::Storage::get_vztmpl_dir($cfg, $param-
> >{storage});
> -	} else {
> -	    raise_param_exc({ content => "upload content type
> '$content' not allowed" });
> +	} elsif ($content eq 'snippets') {
> +	    $path = PVE::Storage::get_snippet_dir($cfg, $param-
> >{storage});
> +        } else {
> +            raise_param_exc({ content => "upload content type
> '$content' not allowed" });
>  	}
>  
>  	die "storage '$param->{storage}' does not support '$content'
> content\n"
> @@ -564,7 +566,7 @@
>  	    content => {
>  		description => "Content type.", # TODO: could be
> optional & detected in most cases
>  		type => 'string', format => 'pve-storage-content',
> -		enum => ['iso', 'vztmpl'],
> +		enum => ['iso', 'vztmpl', 'snippets'],
>  	    },
>  	    filename => {
>  		description => "The name of the file to create.
> Caution: This will be normalized!",
> @@ -627,6 +629,8 @@
>  		raise_param_exc({ filename => "wrong file extension"
> });
>  	    }
>  	    $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
> +	} elsif ($content eq 'snippets') {
> +	    $path = PVE::Storage::get_snippet_dir($cfg,
> $storage);    
>  	} else {
>  	    raise_param_exc({ content => "upload content-type
> '$content' is not allowed" });
>  	}
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] New API endpoint to manage snippets / bugzilla 2208 / updated patch but missing something, need help

[Matthias Heiserer]

Hi,

please take a look at our developer documentation.
https://pve.proxmox.com/wiki/Developer_Documentation

Using `git format-patch` makes applying the changes a whole lot easier :)

As far as I can tell from a quick test, your changes seem to work.
Would you mind using `pvesh create` to test?

If it doesn't work there either, make sure to include the exact 
command/code you used so people can try to reproduce the problem.

Re: [pve-devel] New API endpoint to manage snippets / bugzilla 2208 / updated patch but missing something, need help

[Laurent GUERBY]

On Mon, 2022-04-11 at 11:03 +0200, Matthias Heiserer wrote:
> Hi,
> 
> please take a look at our developer documentation.
> https://pve.proxmox.com/wiki/Developer_Documentation
> 
> Using `git format-patch` makes applying the changes a whole lot
> easier :)
> 
> As far as I can tell from a quick test, your changes seem to work.
> Would you mind using `pvesh create` to test?
> 
> If it doesn't work there either, make sure to include the exact 
> command/code you used so people can try to reproduce the problem.
> 

Hi,

Thanks for the pointer I'll give it a spin later this week.

I opened a bugzilla on the "501" error since I'm able to reproduce the
"upload small file" bug on an unmodified PVE 7:

https://bugzilla.proxmox.com/show_bug.cgi?id=3990

Since snippets yaml are likely to be small they'll be impacted by this
prior bug.

Sincerely,

Laurent