public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>,
	"d.csapak@proxmox.com" <d.csapak@proxmox.com>,
	"aderumier@odiso.com" <aderumier@odiso.com>
Subject: Re: [pve-devel] [PATCH-SERIES pve-http-server/pve-manager] fix#4689: rewrite_uri: autofind nodename for qemu/lxc
Date: Wed, 31 May 2023 14:05:56 +0000	[thread overview]
Message-ID: <dd9e5798ec60f391938436304a58fbacb465b202.camel@groupe-cyllene.com> (raw)
In-Reply-To: <ef020f53-4225-0a2a-221a-941567ead319@proxmox.com>

> 
> 
> hi, while this can work, it introduces a very specific rewriting into
> the
> http server, which might not be obvious and AFAICT won't show up in
> the
> auto-generated api docs?
> 
yes, indeed, it doesnt show up in the doc.


> wouldn't it be better to have some api calls defined for real
> (the ones used most often for example) that make use of the
> proxyto_callback method instead?
> 
I'm quoting Fabian from bugzilla:
"
Fabian Grünbichler 2023-04-24 13:48:07 CEST
yeah, some parts of the machinery are already in place, e.g., it's
possible to define a "proxyto_callback" that gets called instead of
resolving proxyto straight via the parameter. AFAICT that is not used
anywhere though ;)

probably it would mean caching the vmlist in each http server worker to
avoid expensive queries, plus implementing filtering of the vmid list
for the requesting user/token since we'd not want to leak existence or
location of guests that the user doesn't have access to.
"

So, it seem that Fabian have same opinion than you.

I was doing a simply rewrite to keep it simple, but
I'll have a look at proxyto_callback.

(BTW, vmlist don't seem to be so slow, i'm around 10 extra ms. I don't
think it need to be cached.  (it's still faster than doing 1
cluster/ressource api call to find the node + the final call).


> that way they would at least show up in the api documentation,
> even though it's probably a bit of code duplication
> (altough we could maybe just insert the api at a different place
> and do some perl magic refactoring to have it once
> with node parameter and once without?)
> 
> or does that seem like a bad idea?
> 


      reply	other threads:[~2023-05-31 14:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-31 11:19 Alexandre Derumier
2023-05-31 11:19 ` [pve-devel] [PATCH pve-http-server 1/1] anyevent: add rewrite_uri Alexandre Derumier
2023-05-31 11:19 ` [pve-devel] [PATCH pve-manager 1/1] httpserver: add rewrite uri for /nodes/(qemu/lxc)/<vmid> Alexandre Derumier
2023-05-31 11:49 ` [pve-devel] [PATCH-SERIES pve-http-server/pve-manager] fix#4689: rewrite_uri: autofind nodename for qemu/lxc Wolfgang Bumiller
2023-05-31 14:12   ` DERUMIER, Alexandre
2023-05-31 13:19 ` Dominik Csapak
2023-05-31 14:05   ` DERUMIER, Alexandre [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dd9e5798ec60f391938436304a58fbacb465b202.camel@groupe-cyllene.com \
    --to=alexandre.derumier@groupe-cyllene.com \
    --cc=aderumier@odiso.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal