From: Lukas Wagner <l.wagner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Max Carrara <m.carrara@proxmox.com>,
pbs-devel@lists.proxmox.com
Subject: Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
Date: Mon, 22 Jul 2024 09:50:14 +0200 [thread overview]
Message-ID: <1a4b5e7e-cb9f-4b1b-a7ac-c24a2a2768e2@proxmox.com> (raw)
In-Reply-To: <D2RXA36IQTVQ.3B1C6C8G6E5TB@proxmox.com>
On 2024-07-17 17:34, Max Carrara wrote:
> On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
>> Sending as an RFC because I don't want this merged yet; that being
>> said, the feature should be mostly finished at this point, I'd
>> appreciate any reviews and feedback.
>>
>> This series adds support for webhook notification targets to PVE
>> and PBS.
>>
>> A webhook is a HTTP API route provided by a third-party service that
>> can be used to inform the third-party about an event. In our case,
>> we can easily interact with various third-party notification/messaging
>> systems and send PVE/PBS notifications via this service.
>> The changes were tested against ntfy.sh, Discord and Slack.
>>
>> The configuration of webhook targets allows one to configure:
>> - The URL
>> - The HTTP method (GET/POST/PUT)
>> - HTTP Headers
>> - Body
>>
>> One can use handlebar templating to inject notification text and metadata
>> in the url, headers and body.
>>
>> One challenge is the handling of sensitve tokens and other secrets.
>> Since the endpoint is completely generic, we cannot know in advance
>> whether the body/header/url contains sensitive values.
>> Thus we add 'secrets' which are stored in the protected config only
>> accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
>> secrets are accessible in URLs/headers/body via templating:
>>
>> Url: https://example.com/{{ secrets.token }}
>>
>> Secrets can only be set and updated, but never retrieved via the API.
>> In the UI, secrets are handled like other secret tokens/passwords.
>>
>> Bumps for PVE:
>> - libpve-rs-perl needs proxmox-notify bumped
>> - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
>> - proxmox-mail-forward needs proxmox-notify bumped
>>
>> Bumps for PBS:
>> - proxmox-backup needs proxmox-notify bumped
>> - proxmox-mail-forward needs proxmox-notify bumped
>
> Since this is an RFC, I mainly just did some proofreading; I haven't
> really spotted anything out of the ordinary, apart from a few *very
> small* things I commented on inline.
>
> I like the overall idea of adding webhooks, so this looks pretty solid
> to me. At first I thought that this might be a bit of a niche use case,
> but I feel like it might actually be quite interesting for orgs that are
> e.g. on Slack: You could e.g. just "route" all notifications via a
> webhook to Slack, and Slack then sends a push notification to one's
> phone. The same can obviously done with other applications / services as
> well. So, pretty cool stuff :)
>
> Not sure if this has been discussed somewhere already (off list etc.),
> but could you elaborate on why you don't want this merged yet? The
> patches look pretty solid to me, IMHO. Then again, I haven't really
> tested them yet due to all the required package bumps, so take this with
> a grain of salt.
>
> If you want to have this RFC tested, I can of course give it a shot - do
> let me know if that's the case :)
>
I posted this as an RFC because while I consider this as mostly finished,
it did not yet go through my own rigorous self-review/testing.
I had to switch to some other task and wanted to get this version out to get some
general feedback.
There are no changes planned unless I or somebody else discovers any issues,
so I'd very much welcome any testing :)
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
WARNING: multiple messages have this Message-ID
From: Lukas Wagner <l.wagner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Max Carrara <m.carrara@proxmox.com>,
pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
Date: Mon, 22 Jul 2024 09:50:14 +0200 [thread overview]
Message-ID: <1a4b5e7e-cb9f-4b1b-a7ac-c24a2a2768e2@proxmox.com> (raw)
In-Reply-To: <D2RXA36IQTVQ.3B1C6C8G6E5TB@proxmox.com>
On 2024-07-17 17:34, Max Carrara wrote:
> On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
>> Sending as an RFC because I don't want this merged yet; that being
>> said, the feature should be mostly finished at this point, I'd
>> appreciate any reviews and feedback.
>>
>> This series adds support for webhook notification targets to PVE
>> and PBS.
>>
>> A webhook is a HTTP API route provided by a third-party service that
>> can be used to inform the third-party about an event. In our case,
>> we can easily interact with various third-party notification/messaging
>> systems and send PVE/PBS notifications via this service.
>> The changes were tested against ntfy.sh, Discord and Slack.
>>
>> The configuration of webhook targets allows one to configure:
>> - The URL
>> - The HTTP method (GET/POST/PUT)
>> - HTTP Headers
>> - Body
>>
>> One can use handlebar templating to inject notification text and metadata
>> in the url, headers and body.
>>
>> One challenge is the handling of sensitve tokens and other secrets.
>> Since the endpoint is completely generic, we cannot know in advance
>> whether the body/header/url contains sensitive values.
>> Thus we add 'secrets' which are stored in the protected config only
>> accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
>> secrets are accessible in URLs/headers/body via templating:
>>
>> Url: https://example.com/{{ secrets.token }}
>>
>> Secrets can only be set and updated, but never retrieved via the API.
>> In the UI, secrets are handled like other secret tokens/passwords.
>>
>> Bumps for PVE:
>> - libpve-rs-perl needs proxmox-notify bumped
>> - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
>> - proxmox-mail-forward needs proxmox-notify bumped
>>
>> Bumps for PBS:
>> - proxmox-backup needs proxmox-notify bumped
>> - proxmox-mail-forward needs proxmox-notify bumped
>
> Since this is an RFC, I mainly just did some proofreading; I haven't
> really spotted anything out of the ordinary, apart from a few *very
> small* things I commented on inline.
>
> I like the overall idea of adding webhooks, so this looks pretty solid
> to me. At first I thought that this might be a bit of a niche use case,
> but I feel like it might actually be quite interesting for orgs that are
> e.g. on Slack: You could e.g. just "route" all notifications via a
> webhook to Slack, and Slack then sends a push notification to one's
> phone. The same can obviously done with other applications / services as
> well. So, pretty cool stuff :)
>
> Not sure if this has been discussed somewhere already (off list etc.),
> but could you elaborate on why you don't want this merged yet? The
> patches look pretty solid to me, IMHO. Then again, I haven't really
> tested them yet due to all the required package bumps, so take this with
> a grain of salt.
>
> If you want to have this RFC tested, I can of course give it a shot - do
> let me know if that's the case :)
>
I posted this as an RFC because while I consider this as mostly finished,
it did not yet go through my own rigorous self-review/testing.
I had to switch to some other task and wanted to get this version out to get some
general feedback.
There are no changes planned unless I or somebody else discovers any issues,
so I'd very much welcome any testing :)
--
- Lukas
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2024-07-22 7:50 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-12 11:27 Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-17 15:35 ` [pve-devel] " Max Carrara
2024-07-17 15:35 ` Max Carrara
2024-07-22 7:30 ` [pve-devel] " Lukas Wagner
2024-07-22 7:30 ` [pbs-devel] [pve-devel] " Lukas Wagner
2024-07-22 9:41 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-22 9:41 ` [pbs-devel] [pve-devel] " Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 02/12] notify: add api for " Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-17 15:35 ` [pve-devel] " Max Carrara
2024-07-17 15:35 ` [pbs-devel] " Max Carrara
2024-07-22 7:32 ` Lukas Wagner
2024-07-22 7:32 ` [pbs-devel] " Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-17 15:35 ` [pve-devel] " Max Carrara
2024-07-17 15:35 ` [pbs-devel] " Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-17 15:36 ` [pve-devel] " Max Carrara
2024-07-17 15:36 ` Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH widget-toolkit v2 05/12] notification: add UI for adding/updating webhook targets Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 06/12] api: notifications: use get_targets impl from proxmox-notify Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-17 15:36 ` [pve-devel] " Max Carrara
2024-07-17 15:36 ` Max Carrara
2024-07-22 7:37 ` [pve-devel] " Lukas Wagner
2024-07-22 7:37 ` [pbs-devel] [pve-devel] " Lukas Wagner
2024-07-22 9:50 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-22 9:50 ` [pbs-devel] [pve-devel] " Max Carrara
2024-07-22 13:56 ` [pve-devel] [pbs-devel] " Thomas Lamprecht
2024-07-22 13:56 ` [pbs-devel] [pve-devel] " Thomas Lamprecht
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency Lukas Wagner
2024-07-12 11:27 ` [pbs-devel] " Lukas Wagner
2024-07-17 15:34 ` [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Max Carrara
2024-07-17 15:34 ` [pbs-devel] " Max Carrara
2024-07-22 7:50 ` Lukas Wagner [this message]
2024-07-22 7:50 ` Lukas Wagner
2024-07-22 12:10 ` Stefan Hanreich
2024-07-22 12:10 ` [pbs-devel] " Stefan Hanreich
2024-07-22 12:29 ` Lukas Wagner
2024-07-22 12:29 ` [pbs-devel] " Lukas Wagner
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=1a4b5e7e-cb9f-4b1b-a7ac-c24a2a2768e2@proxmox.com \
--to=l.wagner@proxmox.com \
--cc=m.carrara@proxmox.com \
--cc=pbs-devel@lists.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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal