Re: [pve-devel] [PATCH manager v5 08/16] api: notification: add API for getting known metadata fields/values
On 2024-04-19 15:45, Fiona Ebner wrote:
> Am 15.04.24 um 10:26 schrieb Lukas Wagner:
>> +
>> +__PACKAGE__->register_method ({
>> +name => 'get_field_values',
>> +path => 'values',
>> +method => 'GET',
>> +description => 'Returns known notification metadata fields and their
>> known values',
>> +permissions => {
>> +check => ['or',
>> +['perm', '/mapping/notifications', ['Mapping.Modify']],
>> +['perm', '/mapping/notifications', ['Mapping.Audit']],
>> +],
>> +},
>> +protected => 1,
>> +parameters => {
>> +additionalProperties => 0,
>> +},
>> +returns => {
>> +type => 'array',
>> +items => {
>> +type => 'object',
>> +properties => {
>> +'value' => {
>> +description => 'Notification metadata value known by the
>> system.',
>> +type => 'string'
>> +},
>> +'comment' => {
>> +description => 'Additional comment for this value.',
>> +type => 'string',
>> +optional => 1,
>> +},
>> +'field' => {
>> +description => 'Field this value belongs to.',
>> +type => 'string',
>> +optional => 1,
>> +},
>> +'internal' => {
>> +description => 'Set if "value" was generated by the system
>> and can'
>> + . ' safely be used as base for translations.',
>> +type => 'boolean',
>> +optional => 1,
>> +},
>
> And wouldn't it be nicer to return already grouped by field? Then maybe
> we also don't really need the dedicated fields API endpoint either as
> those are just the top-level of the result (with empty array when there
> are no values so we don't ever miss any fields).
>
The design of both endpoints was mostly driven by the intention
of keeping the ExtJS side as simple as possible.
Two comboboxes, each with their own api endpoint to fetch data from,
one setting a filter for the other one.
I tried using a single endpoint at first, but was quickly frustrated
by ExtJS and its documentation and settled for this approach as a consequence.
So I'd prefer to leave it as is :D
Regarding the 'internal' flag: Yes, you are right, right now we only need it
for 'type'.
I'll leave it out then and handle everything in the UI.
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v5 08/16] api: notification: add API for getting known metadata fields/values
Am 15.04.24 um 10:26 schrieb Lukas Wagner:
> +
> +__PACKAGE__->register_method ({
> +name => 'get_field_values',
> +path => 'values',
> +method => 'GET',
> +description => 'Returns known notification metadata fields and their
> known values',
> +permissions => {
> + check => ['or',
> + ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + ['perm', '/mapping/notifications', ['Mapping.Audit']],
> + ],
> +},
> +protected => 1,
> +parameters => {
> + additionalProperties => 0,
> +},
> +returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + 'value' => {
> + description => 'Notification metadata value known by the
> system.',
> + type => 'string'
> + },
> + 'comment' => {
> + description => 'Additional comment for this value.',
> + type => 'string',
> + optional => 1,
> + },
> + 'field' => {
> + description => 'Field this value belongs to.',
> + type => 'string',
> + optional => 1,
> + },
> + 'internal' => {
> + description => 'Set if "value" was generated by the system
> and can'
> +. ' safely be used as base for translations.',
> + type => 'boolean',
> + optional => 1,
> + },
And wouldn't it be nicer to return already grouped by field? Then maybe
we also don't really need the dedicated fields API endpoint either as
those are just the top-level of the result (with empty array when there
are no values so we don't ever miss any fields).
> + },
> + },
> +},
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v5 08/16] api: notification: add API for getting known metadata fields/values
Am 15.04.24 um 10:26 schrieb Lukas Wagner:
> This new API route returns known notification metadata fields and
> a list of known possible values. This will be used by the UI to
> provide suggestions when adding/modifying match rules.
>
> Signed-off-by: Lukas Wagner
> ---
> PVE/API2/Cluster/Notifications.pm | 152 ++
> 1 file changed, 152 insertions(+)
>
> diff --git a/PVE/API2/Cluster/Notifications.pm
> b/PVE/API2/Cluster/Notifications.pm
> index 68fdda2a..16548bec 100644
> --- a/PVE/API2/Cluster/Notifications.pm
> +++ b/PVE/API2/Cluster/Notifications.pm
> @@ -79,12 +79,164 @@ __PACKAGE__->register_method ({
> { name => 'endpoints' },
> { name => 'matchers' },
> { name => 'targets' },
> + { name => 'fields' },
matcher-fields ?
> + { name => 'values' },
(matcher-)field-values ?
> ];
>
> return $result;
> }
> });
>
> +__PACKAGE__->register_method ({
> +name => 'get_fields',
> +path => 'fields',
> +method => 'GET',
> +description => 'Returns known notification metadata fields and their
> known values',
It does not return their values.
> +permissions => {
> + check => ['or',
> + ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + ['perm', '/mapping/notifications', ['Mapping.Audit']],
> + ],
> +},
> +protected => 1,
No need for protected when all it's doing is returning static info.
(This would have the privileged pvedaemon execute the request).
> +parameters => {
> + additionalProperties => 0,
> + properties => {},
> +},
> +returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + name => {
> + description => 'Name of the field.',
> + type => 'string',
> + },
> + },
> + },
> + links => [ { rel => 'child', href => '{name}' } ],
> +},
> +code => sub {
> + # TODO: Adapt this API handler once we have a 'notification registry'
> +
> + my $result = [
> + { name => 'type' },
> + { name => 'hostname' },
> + { name => 'backup-job' },
> + { name => 'replication-job' },
> + ];
> +
> + return $result;
> +}
> +});
> +
> +__PACKAGE__->register_method ({
> +name => 'get_field_values',
> +path => 'values',
> +method => 'GET',
> +description => 'Returns known notification metadata fields and their
> known values',
> +permissions => {
> + check => ['or',
> + ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + ['perm', '/mapping/notifications', ['Mapping.Audit']],
> + ],
> +},
> +protected => 1,
> +parameters => {
> + additionalProperties => 0,
> +},
> +returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + 'value' => {
> + description => 'Notification metadata value known by the
> system.',
> + type => 'string'
> + },
> + 'comment' => {
> + description => 'Additional comment for this value.',
> + type => 'string',
> + optional => 1,
> + },
> + 'field' => {
> + description => 'Field this value belongs to.',
> + type => 'string',
> + optional => 1,
Why is the field optional? All values are associated to a field below.
> + },
> + 'internal' => {
> + description => 'Set if "value" was generated by the system
> and can'
> +. ' safely be used as base for translations.',
> + type => 'boolean',
> + optional => 1,
> + },
Hmm, not sure about this one. It's only used for the type field right?
Can't we just handle that specially in the UI?
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v5 08/16] api: notification: add API for getting known metadata fields/values
This new API route returns known notification metadata fields and
a list of known possible values. This will be used by the UI to
provide suggestions when adding/modifying match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Cluster/Notifications.pm | 152 ++
1 file changed, 152 insertions(+)
diff --git a/PVE/API2/Cluster/Notifications.pm
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..16548bec 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,164 @@ __PACKAGE__->register_method ({
{ name => 'endpoints' },
{ name => 'matchers' },
{ name => 'targets' },
+ { name => 'fields' },
+ { name => 'values' },
];
return $result;
}
});
+__PACKAGE__->register_method ({
+name => 'get_fields',
+path => 'fields',
+method => 'GET',
+description => 'Returns known notification metadata fields and their known
values',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 1,
+parameters => {
+ additionalProperties => 0,
+ properties => {},
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ name => {
+ description => 'Name of the field.',
+ type => 'string',
+ },
+ },
+ },
+ links => [ { rel => 'child', href => '{name}' } ],
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+
+ my $result = [
+ { name => 'type' },
+ { name => 'hostname' },
+ { name => 'backup-job' },
+ { name => 'replication-job' },
+ ];
+
+ return $result;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_field_values',
+path => 'values',
+method => 'GET',
+description => 'Returns known notification metadata fields and their known
values',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 1,
+parameters => {
+ additionalProperties => 0,
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ 'value' => {
+ description => 'Notification metadata value known by the
system.',
+ type => 'string'
+ },
+ 'comment' => {
+ description => 'Additional comment for this value.',
+ type => 'string',
+ optional => 1,
+ },
+ 'field' => {
+ description => 'Field this value belongs to.',
+ type => 'string',
+ optional => 1,
+ },
+ 'internal' => {
+ description => 'Set if "value" was generated by the system
and can'
+ . ' safely be used as base for translations.',
+ type => 'boolean',
+ optional => 1,
+ },
+ },
+ },
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $user = $rpcenv->get_user();
+
+ my $values = [
+ {
+ value => 'package-updates',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'fencing',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'replication',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'vzdump',
+ internal => 1,
+ field => 'type',
+ },
+ {
+ value => 'system-mail',
+ internal => 1,
+ field => 'type',
+ },
+ ];
+
+ # Here we need a manual permission check.
+ if ($rpcenv->check($user, "/", ["Sys.Audit"], 1)) {
+ for my $backup_job (@{PVE::API2::Backup->index({})}) {
+ push @$values, {
+ value => $backup_job->{id},
+ comment => $backup_job->{comment},
+ field => 'backup-job'
+ };
+ }
+ }
+ # The API call returns only returns jobs for which the user
+ # has adequate permissions.
+ for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+ push @$values, {
+ value => $sync_job->{id},
+
