From: Stefan Sterz <s.sterz@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH manager 2/2] api2: use JSONSchema to validate commands for "nodes/{node}/execute"
Date: Wed, 27 Jul 2022 16:56:12 +0200 [thread overview]
Message-ID: <20220727145612.451691-2-s.sterz@proxmox.com> (raw)
In-Reply-To: <20220727145612.451691-1-s.sterz@proxmox.com>
this also makes it more explicit what the different values should be
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
not sure how sensible this is because most of the information here
won't show up in the api viewer. i couldn't figure out how to make it
show up and not make breaking changes to the endpoint or change how
the api definition hash is handled.
fabian gruenbichler and i also discussed off list that this is kind of
redudant functionality as you can likely just call multiple api
endpoints from whatever script/program/client you are using anyway
instead of using this limited and flawed endpoint. perhaps the was a
reason for this before tokens existed?
so maybe we should either drop it in pve 8 or let it accept proper
json not just "a string that when parsed as json needs to be of this
format". i.e. accept an array of objects here instead of a string.
it has also confused users in the past already, e.g. see:
https://forum.proxmox.com/threads/execute-command-in-node-with-api.112290/
PVE/API2/Nodes.pm | 51 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index ef946301..5cc9111d 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -66,6 +66,49 @@ eval {
use base qw(PVE::RESTHandler);
+PVE::JSONSchema::register_format('pve-command-batch', \&verify_command_batch);
+sub verify_command_batch {
+ my ($value, $noerr) = @_;
+ my $commands = eval { decode_json($value); };
+
+ return undef if $noerr && $@;
+ die "commands param did not contain valid JSON: $@" if $@;
+
+ eval {
+ PVE::JSONSchema::validate($commands, {
+ description => "An array of objects describing endpoints, methods and arguments.",
+ type => "array",
+ items => {
+ type => "object",
+ properties => {
+ path => {
+ description => "A relative path to an API endpoint on this node.",
+ type => "string",
+ optional => 0,
+ },
+ method => {
+ description => "A method related to the API endpoint (GET, POST etc.).",
+ type => "string",
+ pattern => "(GET|POST|PUT|DELETE)",
+ optional => 0,
+ },
+ args => {
+ description => "A set of parameter names and their values.",
+ type => "object",
+ optional => 1,
+ },
+ },
+ }
+ });
+ };
+
+ return $commands if !$@;
+
+ return undef if $noerr;
+
+ die "commands is not a valid array of commands: $@";
+}
+
__PACKAGE__->register_method ({
subclass => "PVE::API2::Qemu",
path => 'qemu',
@@ -433,6 +476,7 @@ __PACKAGE__->register_method({
commands => {
description => "JSON encoded array of commands.",
type => "string",
+ format => "pve-command-batch",
}
},
},
@@ -449,16 +493,11 @@ __PACKAGE__->register_method({
my $rpcenv = PVE::RPCEnvironment::get();
my $user = $rpcenv->get_user();
-
+ # just parse the json again, it should already be validated
my $commands = eval { decode_json($param->{commands}); };
- die "commands param did not contain valid JSON: $@" if $@;
- die "commands is not an array" if ref($commands) ne "ARRAY";
-
foreach my $cmd (@$commands) {
eval {
- die "$cmd is not a valid command" if (ref($cmd) ne "HASH" || !$cmd->{path} || !$cmd->{method});
-
$cmd->{args} //= {};
my $path = "nodes/$param->{node}/$cmd->{path}";
--
2.30.2
next prev parent reply other threads:[~2022-07-27 14:57 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-27 14:56 [pve-devel] [PATCH manager 1/2] fix: api2: add return type to nodes/{node}/execute endpoint Stefan Sterz
2022-07-27 14:56 ` Stefan Sterz [this message]
2022-08-02 12:47 ` [pve-devel] applied+follow-up: [PATCH manager 2/2] api2: use JSONSchema to validate commands for "nodes/{node}/execute" Fabian Grünbichler
2022-08-02 12:45 ` [pve-devel] applied: [PATCH manager 1/2] fix: api2: add return type to nodes/{node}/execute endpoint Fabian Grünbichler
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=20220727145612.451691-2-s.sterz@proxmox.com \
--to=s.sterz@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.