[pve-devel] [RFC] towards automated integration testing
Hello, I am currently doing the groundwork that should eventually enable us to write automated integration tests for our products. Part of that endeavor will be to write a custom test runner, which will - setup a specified test environment - execute test cases in that environment - create some sort of test report What will follow is a description of how that test runner would roughly work. The main point is to get some feedback on some of the ideas/ approaches before I start with the actual implementation. Let me know what you think! ## Introduction The goal is to establish a framework that allows us to write automated integration tests for our products. These tests are intended to run in the following situations: - When new packages are uploaded to the staging repos (by triggering a test run from repoman, or similar) - Later, this tests could also be run when patch series are posted to our mailing lists. This requires a mechanism to automatically discover, fetch and build patches, which will be a separate, follow-up project. - Additionally, it should be easy to run these integration tests locally on a developer's workstation in order to write new test cases, as well as troubleshooting and debugging existing test cases. The local test environment should match the one being used for automated testing as closely as possible As a main mode of operation, the Systems under Test (SUTs) will be virtualized on top of a Proxmox VE node. This has the following benefits: - it is easy to create various test setups (fixtures), including but not limited to single Proxmox VE nodes, clusters, Backup servers and auxiliary services (e.g. an LDAP server for testing LDAP authentication) - these test setups can easily be brought to a well-defined state: cloning from a template/restoring a backup/rolling back to snapshot - it makes it easy to run the integration tests on a developers workstation in identical configuration For the sake of completeness, some of the drawbacks of not running the tests on bare-metal: - Might be unable to detect regressions that only occur on real hardware In theory, the test runner would also be able to drive tests on real hardware, but of course with some limitations (harder to have a predictable, reproducible environment, etc.) ## Terminology - Template: A backup/VM template that can be instantiated by the test runner - Test Case: Some script/executable executed by the test runner, success is determined via exit code. - Fixture: Description of a test setup (e.g. which templates are needed, additional setup steps to run, etc.) ## Approach Test writers write template, fixture, test case definition in declarative configuration files (most likely TOML). The test case references a test executable/script, which performs the actual test. The test script is executed by the test runner; the test outcome is determined by the exit code of the script. Test scripts could be written in any language, e.g. they could be Perl scripts that use the official `libpve-apiclient-perl` to test-drive the SUTs. If we notice any emerging patterns, we could write additional helper libs that reduce the amount of boilerplate in test scripts. In essence, the test runner would do the following: - Group testcases by fixture - For every fixture: - Instantiate needed templates from their backup snapshot - Start VMs - Run any specified `setup-hooks` (update system, deploy packages, etc.) - Take a snapshot, including RAM - For every testcase using that fixture: - Run testcase (execute test executable, check exit code) - Rollback to snapshot (iff `rollback = true` for that template) - destroy test instances (or at least those which are not needed by other fixtures) In the beginning, the test scripts would primarily drive the Systems under Test (SUTs) via their API. However, the system would also offer the flexibility for us to venture into the realm of automated GUI testing at some point (e.g. using selenium) - without having to change the overall test architecture. ## Mock Test Runner Config Beside the actual test scripts, test writers would write test configuration. Based on the current requirements and approach that I have chose, a example config *could* look like the one following. These would likely be split into multiple files/folders (e.g. to group test case definition and the test script logically). ```toml [template.pve-default] # Backup image to restore from, in this case this would be a previously # set up PVE installation restore = '...' # To check if node is booted successfully, also made available to hook # scripts, in case they need to SSH in to setup things. ip = "10.0.0.1" # Define credentials in separate file - most template could use a # default password/SSH key/API token etc. credentials = "default" # Update to latest packages, install test .debs # credentials are passed via env var # Maybe this could also b
Re: [pve-devel] [RFC] towards automated integration testing
Thank you for the feedback! On 10/16/23 13:20, Stefan Hanreich wrote: On 10/13/23 15:33, Lukas Wagner wrote: - Additionally, it should be easy to run these integration tests locally on a developer's workstation in order to write new test cases, as well as troubleshooting and debugging existing test cases. The local test environment should match the one being used for automated testing as closely as possible This would also include sharing those fixture templates somewhere, do you already have an idea on how to accomplish this? PBS sounds like a good option for this if I'm not missing something. Yes, these templates could be stored on some shared storage, e.g. a PBS instance, or they could also distributed via a .deb/multiple .debs (not sure if that is a good idea, since these would become huge pretty quickly). It could also be a two-step process: Use one command to get the latest test templates, restoring them from a remote backup, converting them to a local VM template. When executing tests, the test runner could then use linked clones, speeding up the test setup time quite a bit. All in all, these templates that can be used in test fixtures should be: - easily obtainable for developers, in order to have a fully functional test setup on their workstation - easily updateable (e.g. installing the latest packages, so that the setup-hook does not need to fetch a boatload of packages every time) As a main mode of operation, the Systems under Test (SUTs) will be virtualized on top of a Proxmox VE node. This has the following benefits: - it is easy to create various test setups (fixtures), including but not limited to single Proxmox VE nodes, clusters, Backup servers and auxiliary services (e.g. an LDAP server for testing LDAP authentication) I can imagine having to setup VMs inside the Test Setup as well for doing various tests. Doing this manually every time could be quite cumbersome / hard to automate. Do you have a mechanism in mind to deploy VMs inside the test system as well? Again, PBS could be an interesting option for this imo. Several options come to mind. We could use a virtualized PBS instance with a datastore containing the VM backup as part of the fixture. We could use some external backup store (so the same 'source' as for the templates themselves) - however that means that the systems under test must have network access to that. We could also think about using iPXE to boot test VMs, with the boot image either be provided by some template from the fixture, or by some external server. For both approaches, the 'as part of the fixture' approaches seem a bit nicer, as they are more self-contained. Also, the vmbd2 thingy that thomas mentioned might be interesting for this - i've only glanced at it so far though. As of now it seems that this question will not influence the design of the test runner much, so it can probably be postponed to a later stage. In theory, the test runner would also be able to drive tests on real hardware, but of course with some limitations (harder to have a predictable, reproducible environment, etc.) Maybe utilizing Aaron's installer for setting up those test systems could at least produce somewhat identical setups? Although it is really hard managing systems with different storage types, network cards, ... . In general my biggest concern with 'bare-metal' tests - and to precise, that does not really have anything to do with being 'bare-metal', more about testing on something that is harder roll back into a clean state that can be used for the next test execution, is that I'm afraid that a setup like this could become quite brittle and a maintenance burden. At some point, a test execution might leave something in an unclean state (e.g. due to a crashed test or missing something while cleanup), tripping up the following test job. As an example from personal experience: One test run might test new packages which introduce a new flag in a configuration file. If that flag is not cleanup up afterwards, another test job testing other packages might fail because it now has to deal with an 'unknown' configuration key. Maybe ZFS snapshots could help with that, but I'm not sure how that would work in practice (e.g. due to the kernel being stored on the EFI partition). The automated installer *could* certainly help here - however, right now I don't want to extend the scope of this project too much. Also, there is also the question if the installation should be refreshed after every single test run, increasing the test cycle time/resource consumption quite a bit? Or only if 'something' breaks? That being said, it might also make sense to be able to run the tests (or more likely, a subset of them, since some will inherently require a fixture) against an arbitrary PVE instance that is under full control of a developer (e
Re: [pve-devel] [RFC] towards automated integration testing
Thanks for the summary from our discussion and the additional feedback! On 10/16/23 15:57, Thomas Lamprecht wrote: - create some sort of test report As Stefan mentioned, test-output can be good to have. Our buildbot instance provides that, and while I don't look at them in 99% of the builds, when I need to its worth *a lot*. Agreed, test output is always valuable and will definitely captured. ## Introduction The goal is to establish a framework that allows us to write automated integration tests for our products. These tests are intended to run in the following situations: - When new packages are uploaded to the staging repos (by triggering a test run from repoman, or similar) *debian repos, as we could also trigger some when git commits are pushed, just like we do now through Buildbot. Doing so is IMO nice as it will catch issues before a package was bumped, but is still quite a bit simpler to implement than an "apply patch from list to git repos" thing from the next point, but could still act as a preparation for that. - Later, this tests could also be run when patch series are posted to our mailing lists. This requires a mechanism to automatically discover, fetch and build patches, which will be a separate, follow-up project. As a main mode of operation, the Systems under Test (SUTs) will be virtualized on top of a Proxmox VE node. For the fully-automated test system this can be OK as primary mode, as it indeed makes things like going back to an older software state much easier. But, if we decouple the test harness and running them from that more automated system, we can also run the harness periodically on our bare-metal test servers. ## Terminology - Template: A backup/VM template that can be instantiated by the test runner I.e., the base of the test host? I'd call this test-host, template is a bit to overloaded/generic and might focus too much on the virtual test environment. True, 'template' is a bit overloaded. Or is this some part that takes place in the test, i.e., a generalization of product to test and supplementary tool/app that helps on that test? It was intended to be a 'general VM/CT base thingy' that can be instantiated and managed by the test runner, so either a PVE/PBS/PMG base installation, or some auxiliary resource, e.g. a Debian VM with an already-set-up LDAP server. I'll see if I can find good terms with the newly added focus on bare-metal testing / the decoupling between environment setup and test execution. Is the order of test-cases guaranteed by toml parsing, or how are intra- fixture dependencies ensured? Good point. With rollbacks in between test cases it probably does not matter much, but on 'real hardware' with no rollback this could definitely be a concern. A super simple thing that could just work fine is ordering test execution by testcase-names, sorted alphabetically. Ideally you'd write test cases that do not depend on each other any way, and *if* you ever find yourself in the situation where you *need* some ordering, you could just encode the order in the test-case name by adding an integer prefix - similar how you would name config files in /etc/sysctl.d/*, for instance. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 many 0/7] notifications: add SMTP endpoint
Ping - would be great to get some reviews on this to get this merged for the next release. On 9/18/23 13:14, Lukas Wagner wrote: This patch series adds support for a new notification endpoint type, smtp. As the name suggests, this new endpoint allows PVE to talk to SMTP server directly, without using the system's MTA (postfix). -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify
Ping - would be great to get some reviews on this to get this merged for the next release. On 10/2/23 10:06, Lukas Wagner wrote: The aim of this patch series is to adapt `proxmox-mail-forward` so that it forwards emails that were sent to the local root user through the `proxmox_notify` crate. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC] towards automated integration testing
On 10/17/23 08:35, Thomas Lamprecht wrote: Is the order of test-cases guaranteed by toml parsing, or how are intra- fixture dependencies ensured? Good point. With rollbacks in between test cases it probably does not matter much, but on 'real hardware' with no rollback this could definitely be a concern. A super simple thing that could just work fine is ordering test execution by testcase-names, sorted alphabetically. Ideally you'd write test cases that do not depend on each other any way, and *if* you ever find yourself in the situation where you *need* some ordering, you could> just encode the order in the test-case name by adding an integer prefix> - similar how you would name config files in /etc/sysctl.d/*, for instance. While it can be OK to leave that for later, encoding such things in names is IMO brittle and hard to manage if more than a handful of tests, and we hopefully got lots more ;-) From top of my head I'd rather do some attribute based dependency annotation, so that one can depend on single tests, or whole fixture on others single tests or whole fixture. The more thought I spend on it, the more I believe that inter-testcase deps should be avoided as much as possible. In unit testing, (hidden) dependencies between tests are in my experience the no. 1 cause of flaky tests, and I see no reason why this would not also apply for end-to-end integration testing. I'd suggest to only allow test cases to depend on fixtures. The fixtures themselves could have setup/teardown hooks that allow setting up and cleaning up a test scenario. If needed, we could also have something like 'fixture inheritance', where a fixture can 'extend' another, supplying additional setup/teardown. Example: the 'outermost' or 'parent' fixture might define that we want a 'basic PVE installation' with the latest .debs deployed, while another fixture that inherits from that one might set up a storage of a certain type, useful for all tests that require specific that type of storage. On the other hand, instead of inheritance, a 'role/trait'-based system might also work (composition >>> inheritance, after all) - and maybe that also aligns better with the 'properties' mentioned in your other mail (I mean this here: "ostype=win*", "memory>=10G"). This is essentially a very similar pattern as in numerous other testing frameworks (xUnit, pytest, etc.); I think it makes sense to build upon this battle-proven approach. Regarding execution order, I'd now even suggest the polar opposite of my prior idea. Instead of enforcing some execution order, we could also actively shuffle execution order from run to run, at least for tests using the same fixture. The seed used for the RNG should be put into the test report and could also be provided via a flag to the test runner, in case we need to repeat a specific test sequence . In that way, the runner would actively help us to hunt down hidden inter-TC deps, making our test suite hopefully less brittle and more robust in the long term. Any way, lots of details to figure out. Thanks again for your input. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC] towards automated integration testing
On 10/17/23 18:28, Thomas Lamprecht wrote: Am 17/10/2023 um 14:33 schrieb Lukas Wagner: On 10/17/23 08:35, Thomas Lamprecht wrote: From top of my head I'd rather do some attribute based dependency annotation, so that one can depend on single tests, or whole fixture on others single tests or whole fixture. The more thought I spend on it, the more I believe that inter-testcase deps should be avoided as much as possible. In unit testing, (hidden) We don't plan unit testing here though and the dependencies I proposed are the contrary from hidden, rather explicit annotated ones. dependencies between tests are in my experience the no. 1 cause of flaky tests, and I see no reason why this would not also apply for end-to-end integration testing. Any source on that being the no 1 source of flaky tests? IMO that should not make any difference, in the end you just allow better Of course I don't have bullet-proof evidence for the 'no. 1' claim, but it's just my personal experience, which comes partly from a former job (where was I coincidentally also responsible for setting up automated testing ;) - there it was for a firmware project), partly from the work I did for my master's thesis (which was also in the broader area of software testing). I would say it's just the consequence of having multiple test cases manipulating a shared, stateful entity, be it directly or indirectly via side effects. Things get of course even more difficult and messy if concurrent test execution enters the picture ;) reuse through composition of other tests (e.g., migration builds upon clustering *set up*, not tests, if I just want to run migration I can do clustering setup without executing its tests). > Not providing that could also mean that one has to move all logic in the test-script, resulting in a single test per "fixture", reducing granularity and parallelity of some running tests. I also think that I'd suggest to only allow test cases to depend on fixtures. The fixtures themselves could have setup/teardown hooks that allow setting up and cleaning up a test scenario. If needed, we could also have something like 'fixture inheritance', where a fixture can 'extend' another, supplying additional setup/teardown. Example: the 'outermost' or 'parent' fixture might define that we want a 'basic PVE installation' with the latest .debs deployed, while another fixture that inherits from that one might set up a storage of a certain type, useful for all tests that require specific that type of storage. Maybe our disagreement stems mostly from different design pictures in our head, I probably am a bit less fixed (heh) on the fixtures, or at least the naming of that term and might use test system, or intra test system when for your design plan fixture would be the better word. I think it's mostly a terminology problem. In my previous definition of 'fixture' I was maybe too fixated (heh) on it being 'the test infrastructure/VMs that must be set up/instantatiated'. Maybe it helps to think about it more generally as 'common setup/cleanup steps for a set of test cases, which *might* include setting up test infra (although I have not figured out a good way how that would be modeled with the desired decoupling between test runner and test-VM-setup-thingy). On the other hand, instead of inheritance, a 'role/trait'-based system might also work (composition >>> inheritance, after all) - and maybe that also aligns better with the 'properties' mentioned in your other mail (I mean this here: "ostype=win*", "memory>=10G"). This is essentially a very similar pattern as in numerous other testing frameworks (xUnit, pytest, etc.); I think it makes sense to build upon this battle-proven approach. Those are all unit testing tools though that we do already in the sources and IIRC those do not really provide what we need here. While starting out simple(r) and avoiding too much complexity has certainly it's merits, I don't think we should try to draw/align too many parallels with those tools here for us. > In summary, the most important points for me is a decoupled test-system from the automation system that can manage it, ideally such that I can decide relatively flexible on manual runs, IMO that should not be to much work and it guarantees for clean cut APIs from which future development, or integration surely will benefit too. The rest is possibly hard to determine clearly on this stage, as it's easy (at least for me) to get lost in different understandings of terms and design perception, but hard to convey those very clearly about "pipe dreams", so at this stage I'll cede to add discussion churn until there's something more concrete that I can grasp on my terms (through reading/writing code)
Re: [pve-devel] [PATCH v3 many 0/7] notifications: add SMTP endpoint
On 10/17/23 09:27, Lukas Wagner wrote: Ping - would be great to get some reviews on this to get this merged for the next release. On 9/18/23 13:14, Lukas Wagner wrote: This patch series adds support for a new notification endpoint type, smtp. As the name suggests, this new endpoint allows PVE to talk to SMTP server directly, without using the system's MTA (postfix). There will be some conceptual changes to the notification system, requiring some changes to this series as well. A new version will be posted at some point. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify
On 10/17/23 09:28, Lukas Wagner wrote: Ping - would be great to get some reviews on this to get this merged for the next release. On 10/2/23 10:06, Lukas Wagner wrote: The aim of this patch series is to adapt `proxmox-mail-forward` so that it forwards emails that were sent to the local root user through the `proxmox_notify` crate. There will be some conceptual changes to the notification system, requiring some changes to this series as well. A new version will be posted at some point. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox 06/27] notify: let a matcher always match if it has no matching directives
This should be a bit more intuitive to users than the current behavior, which is 'always match' for mode==all and 'never match' for mode==any. The current behavior originates in the neutral element of the underlying logical operation (and, or). Signed-off-by: Lukas Wagner --- proxmox-notify/src/matcher.rs | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index e299fd0..553ca87 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -265,17 +265,22 @@ impl MatcherConfig { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); +// If there are no matching directives, the matcher will always match +let mut no_matchers = true; if let Some(severity_matchers) = self.match_severity.as_deref() { +no_matchers = false; is_match = mode.apply( is_match, self.check_matches(notification, severity_matchers)?, ); } if let Some(field_matchers) = self.match_field.as_deref() { +no_matchers = false; is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?); } if let Some(calendar_matchers) = self.match_calendar.as_deref() { +no_matchers = false; is_match = mode.apply( is_match, self.check_matches(notification, calendar_matchers)?, @@ -284,7 +289,7 @@ impl MatcherConfig { let invert_match = self.invert_match.unwrap_or_default(); -Ok(if is_match != invert_match { +Ok(if is_match != invert_match || no_matchers { Some(self.target.as_deref().unwrap_or_default()) } else { None @@ -455,4 +460,25 @@ mod tests { let matcher: SeverityMatcher = "info,notice,warning,error".parse().unwrap(); assert!(matcher.matches(¬ification).unwrap()); } + +#[test] +fn test_empty_matcher_matches_always() { +let notification = Notification::new_templated( +Severity::Notice, +"test", +"test", +Value::Null, +Default::default(), +); + +for mode in [MatchModeOperator::All, MatchModeOperator::Any] { +let config = MatcherConfig { +name: "matcher".to_string(), +mode: Some(mode), +..Default::default() +}; + +assert!(config.matches(¬ification).unwrap().is_some()) +} +} } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox 05/27] notify: matcher: introduce common trait for match directives
This allows us to make the match-checking code a bit shorter. Signed-off-by: Lukas Wagner --- proxmox-notify/src/matcher.rs | 92 +-- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index b03d11d..e299fd0 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -142,6 +142,11 @@ pub struct MatcherConfig { pub comment: Option, } +trait MatchDirective { +fn matches(&self, notification: &Notification) -> Result; +} + +/// Check if the notification metadata fields match #[derive(Clone, Debug)] pub enum FieldMatcher { Exact { @@ -157,9 +162,9 @@ pub enum FieldMatcher { proxmox_serde::forward_deserialize_to_from_str!(FieldMatcher); proxmox_serde::forward_serialize_to_display!(FieldMatcher); -impl FieldMatcher { -fn matches(&self, notification: &Notification) -> bool { -match self { +impl MatchDirective for FieldMatcher { +fn matches(&self, notification: &Notification) -> Result { +Ok(match self { FieldMatcher::Exact { field, matched_value, @@ -186,7 +191,7 @@ impl FieldMatcher { false } } -} +}) } } @@ -260,9 +265,22 @@ impl MatcherConfig { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); -is_match = mode.apply(is_match, self.check_severity_match(notification)); -is_match = mode.apply(is_match, self.check_field_match(notification)?); -is_match = mode.apply(is_match, self.check_calendar_match(notification)?); + +if let Some(severity_matchers) = self.match_severity.as_deref() { +is_match = mode.apply( +is_match, +self.check_matches(notification, severity_matchers)?, +); +} +if let Some(field_matchers) = self.match_field.as_deref() { +is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?); +} +if let Some(calendar_matchers) = self.match_calendar.as_deref() { +is_match = mode.apply( +is_match, +self.check_matches(notification, calendar_matchers)?, +); +} let invert_match = self.invert_match.unwrap_or_default(); @@ -273,46 +291,24 @@ impl MatcherConfig { }) } -fn check_field_match(&self, notification: &Notification) -> Result { -let mode = self.mode.unwrap_or_default(); -let mut is_match = mode.neutral_element(); - -if let Some(match_field) = self.match_field.as_deref() { -for field_matcher in match_field { -// let field_matcher: FieldMatcher = match_stmt.parse()?; -is_match = mode.apply(is_match, field_matcher.matches(notification)); -} -} - -Ok(is_match) -} - -fn check_severity_match(&self, notification: &Notification) -> bool { +/// Check if given `MatchDirectives` match a notification. +fn check_matches( +&self, +notification: &Notification, +matchers: &[impl MatchDirective], +) -> Result { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); -if let Some(matchers) = self.match_severity.as_ref() { -for severity_matcher in matchers { -is_match = mode.apply(is_match, severity_matcher.matches(notification)); -} -} - -is_match -} - -fn check_calendar_match(&self, notification: &Notification) -> Result { -let mode = self.mode.unwrap_or_default(); -let mut is_match = mode.neutral_element(); - -if let Some(matchers) = self.match_calendar.as_ref() { -for matcher in matchers { -is_match = mode.apply(is_match, matcher.matches(notification)?); -} +for field_matcher in matchers { +is_match = mode.apply(is_match, field_matcher.matches(notification)?); } Ok(is_match) } } + +/// Match severity of the notification. #[derive(Clone, Debug)] pub struct SeverityMatcher { severities: Vec, @@ -321,9 +317,11 @@ pub struct SeverityMatcher { proxmox_serde::forward_deserialize_to_from_str!(SeverityMatcher); proxmox_serde::forward_serialize_to_display!(SeverityMatcher); -impl SeverityMatcher { -fn matches(&self, notification: &Notification) -> bool { -self.severities.contains(¬ification.metadata.severity) +/// Common trait implemented by all matching directives +impl MatchDirective for SeverityMatcher { +/// Check if this directive matches a given notification +fn matches(&self, notification: &Notification) -> Result {
[pve-devel] [PATCH pve-ha-manager 10/27] env: switch to matcher-based notification system
Signed-off-by: Lukas Wagner --- src/PVE/HA/Env/PVE2.pm | 10 ++ src/PVE/HA/NodeStatus.pm | 11 +-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm index ea9e6e4..fcb60a9 100644 --- a/src/PVE/HA/Env/PVE2.pm +++ b/src/PVE/HA/Env/PVE2.pm @@ -221,16 +221,10 @@ sub log { } sub send_notification { -my ($self, $subject, $text, $properties) = @_; +my ($self, $subject, $text, $template_data, $metadata_fields) = @_; eval { - my $dc_config = PVE::Cluster::cfs_read_file('datacenter.cfg'); - my $target = $dc_config->{notify}->{'target-fencing'} // PVE::Notify::default_target(); - my $notify = $dc_config->{notify}->{fencing} // 'always'; - - if ($notify eq 'always') { - PVE::Notify::error($target, $subject, $text, $properties); - } + PVE::Notify::error($subject, $text, $template_data, $metadata_fields); }; $self->log("warning", "could not notify: $@") if $@; diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm index b264a36..e053c55 100644 --- a/src/PVE/HA/NodeStatus.pm +++ b/src/PVE/HA/NodeStatus.pm @@ -212,7 +212,7 @@ my $send_fence_state_email = sub { my $haenv = $self->{haenv}; my $status = $haenv->read_manager_status(); -my $notification_properties = { +my $template_data = { "status-data"=> { manager_status => $status, node_status=> $self->{status} @@ -222,11 +222,18 @@ my $send_fence_state_email = sub { "subject"=> $subject, }; +my $metadata_fields = { + type => 'fencing', + hostname => $node, +}; + $haenv->send_notification( $subject_template, $body_template, - $notification_properties + $template_data, + $metadata_fields, ); + }; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox 02/27] notify: factor out notification content into its own type
This will be useful later for system mail forwarding, where the content of the mail should be forwarded unchanged. This moves notification properties into this new type and calls them 'data'. They will exclusively used for template rendering. `Notification` will receive a separate field for metadata, which will be useful for notification filtering. This decouples template rendering and filtering, which enables us to be very precise about which metadata fields we allow to be used in filters. Signed-off-by: Lukas Wagner --- proxmox-notify/examples/render.rs| 4 +- proxmox-notify/src/endpoints/gotify.rs | 26 +--- proxmox-notify/src/endpoints/sendmail.rs | 62 +- proxmox-notify/src/filter.rs | 10 +-- proxmox-notify/src/lib.rs| 81 ++-- proxmox-notify/src/renderer/mod.rs | 15 ++--- 6 files changed, 109 insertions(+), 89 deletions(-) diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs index c0a6f27..d705fd0 100644 --- a/proxmox-notify/examples/render.rs +++ b/proxmox-notify/examples/render.rs @@ -53,10 +53,10 @@ fn main() -> Result<(), Error> { } }); -let output = render_template(TemplateRenderer::Html, TEMPLATE, Some(&properties))?; +let output = render_template(TemplateRenderer::Html, TEMPLATE, &properties)?; println!("{output}"); -let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, Some(&properties))?; +let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, &properties)?; println!("{output}"); Ok(()) diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs index 83df41f..af86f9c 100644 --- a/proxmox-notify/src/endpoints/gotify.rs +++ b/proxmox-notify/src/endpoints/gotify.rs @@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater}; use crate::context::context; use crate::renderer::TemplateRenderer; use crate::schema::ENTITY_NAME_SCHEMA; -use crate::{renderer, Endpoint, Error, Notification, Severity}; +use crate::{renderer, Content, Endpoint, Error, Notification, Severity}; fn severity_to_priority(level: Severity) -> u32 { match level { @@ -85,15 +85,21 @@ pub enum DeleteableGotifyProperty { impl Endpoint for GotifyEndpoint { fn send(&self, notification: &Notification) -> Result<(), Error> { -let properties = notification.properties.as_ref(); - -let title = renderer::render_template( -TemplateRenderer::Plaintext, -¬ification.title, -properties, -)?; -let message = -renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?; + +let (title, message) = match ¬ification.content { +Content::Template { +title_template, +body_template, +data +} => { +let rendered_title = +renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?; +let rendered_message = +renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?; + +(rendered_title, rendered_message) +} +}; // We don't have a TemplateRenderer::Markdown yet, so simply put everything // in code tags. Otherwise tables etc. are not formatted properly diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs index 26e2a17..c540925 100644 --- a/proxmox-notify/src/endpoints/sendmail.rs +++ b/proxmox-notify/src/endpoints/sendmail.rs @@ -8,7 +8,7 @@ use proxmox_schema::{api, Updater}; use crate::context::context; use crate::renderer::TemplateRenderer; use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA}; -use crate::{renderer, Endpoint, Error, Notification}; +use crate::{renderer, Content, Endpoint, Error, Notification}; pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail"; @@ -102,41 +102,43 @@ impl Endpoint for SendmailEndpoint { } } -let properties = notification.properties.as_ref(); - -let subject = renderer::render_template( -TemplateRenderer::Plaintext, -¬ification.title, -properties, -)?; -let html_part = -renderer::render_template(TemplateRenderer::Html, ¬ification.body, properties)?; -let text_part = -renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?; - -let author = self -.config -.author -.clone() -.unwrap_or_else(|| context().default_sendmail_author()); - +let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect(); let mailfrom = self
[pve-devel] [PATCH pve-guest-common 09/27] vzdump: deprecate mailto/mailnotification/notification-{target, policy}
The first two will be migrated to the notification system, the second were part for the first attempt for the new notification system. The first attempt only ever hit pvetest, so we simply tell the user to not use the two params. Signed-off-by: Lukas Wagner --- src/PVE/VZDump/Common.pm | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm index be605af..b93ad86 100644 --- a/src/PVE/VZDump/Common.pm +++ b/src/PVE/VZDump/Common.pm @@ -175,21 +175,22 @@ my $confdesc = { mailto => { type => 'string', format => 'email-or-username-list', - description => "Comma-separated list of email addresses or users that should" . - " receive email notifications. Has no effect if the 'notification-target' option " . - " is set at the same time.", + description => "Deprecated: Use notification targets/matchers instead." . + " Comma-separated list of email addresses or users that should" . + " receive email notifications.", optional => 1, }, mailnotification => { type => 'string', - description => "Deprecated: use 'notification-policy' instead.", + description => "Deprecated: use notification targets/matchers instead." . + " Specify when to send a notification mail", optional => 1, enum => [ 'always', 'failure' ], default => 'always', }, 'notification-policy' => { type => 'string', - description => "Specify when to send a notification", + description => "Deprecated: Do not use", optional => 1, enum => [ 'always', 'failure', 'never'], default => 'always', @@ -197,10 +198,7 @@ my $confdesc = { 'notification-target' => { type => 'string', format => 'pve-configid', - description => "Determine the target to which notifications should be sent." . - " Can either be a notification endpoint or a notification group." . - " This option takes precedence over 'mailto', meaning that if both are " . - " set, the 'mailto' option will be ignored.", + description => "Deprecated: Do not use", optional => 1, }, tmpdir => { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox 01/27] notify: introduce Error::Generic
... as leaf error-type for anything for which we do not necessarily want a separate enum variant. Signed-off-by: Lukas Wagner --- proxmox-notify/src/lib.rs | 11 +++ 1 file changed, 11 insertions(+) diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 7500778..f7d480c 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -25,13 +25,22 @@ mod config; #[derive(Debug)] pub enum Error { +/// There was an error serializing the config ConfigSerialization(Box), +/// There was an error deserializing the config ConfigDeserialization(Box), +/// An endpoint failed to send a notification NotifyFailed(String, Box), +/// A target does not exist TargetDoesNotExist(String), +/// Testing one or more notification targets failed TargetTestFailed(Vec>), +/// A filter could not be applied FilterFailed(String), +/// The notification's template string could not be rendered RenderError(Box), +/// Generic error for anything else +Generic(String), } impl Display for Error { @@ -60,6 +69,7 @@ impl Display for Error { write!(f, "could not apply filter: {message}") } Error::RenderError(err) => write!(f, "could not render notification template: {err}"), +Error::Generic(message) => f.write_str(message), } } } @@ -74,6 +84,7 @@ impl StdError for Error { Error::TargetTestFailed(errs) => Some(&*errs[0]), Error::FilterFailed(_) => None, Error::RenderError(err) => Some(&**err), +Error::Generic(_) => None, } } } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-widget-toolkit 26/27] notification ui: unprotected mailto-root target
A default notification config will now be created in pve-manager's postinst hook - which is not magic in any way and can be modified and deleted as desired. Signed-off-by: Lukas Wagner --- src/panel/NotificationConfigView.js | 6 -- 1 file changed, 6 deletions(-) diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js index ecf764d..6a9bc20 100644 --- a/src/panel/NotificationConfigView.js +++ b/src/panel/NotificationConfigView.js @@ -41,10 +41,6 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { openEditWindow: function(endpointType, endpoint) { let me = this; - if (endpoint === 'mail-to-root') { - return; - } - Ext.create('Proxmox.window.EndpointEditBase', { baseUrl: me.getView().baseUrl, type: endpointType, @@ -183,13 +179,11 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { xtype: 'proxmoxButton', text: gettext('Modify'), handler: 'openEditForSelectedItem', - enableFn: rec => rec.data.name !== 'mail-to-root', disabled: true, }, { xtype: 'proxmoxStdRemoveButton', callback: 'reload', - enableFn: rec => rec.data.name !== 'mail-to-root', getUrl: function(rec) { return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`; }, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-widget-toolkit 21/27] notification ui: add target selector for matcher
Signed-off-by: Lukas Wagner --- src/window/NotificationFilterEdit.js | 145 +++ 1 file changed, 145 insertions(+) diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationFilterEdit.js index 703a9e2..bcde4fa 100644 --- a/src/window/NotificationFilterEdit.js +++ b/src/window/NotificationFilterEdit.js @@ -49,6 +49,11 @@ Ext.define('Proxmox.panel.NotificationFilterEditPanel', { deleteDefaultValue: '{!isCreate}', }, }, + { + xtype: 'pmxNotificationTargetSelector', + name: 'target', + allowBlank: false, + }, { xtype: 'proxmoxtextfield', name: 'comment', @@ -107,3 +112,143 @@ Ext.define('Proxmox.window.NotificationFilterEdit', { } }, }); + +Ext.define('Proxmox.form.NotificationTargetSelector', { +extend: 'Ext.grid.Panel', +alias: 'widget.pmxNotificationTargetSelector', + +mixins: { + field: 'Ext.form.field.Field', +}, + +padding: '0 0 10 0', + +allowBlank: true, +selectAll: false, +isFormField: true, + +store: { + autoLoad: true, + model: 'proxmox-notification-endpoints', + sorters: 'name', +}, + +columns: [ + { + header: gettext('Target Name'), + dataIndex: 'name', + flex: 1, + }, + { + header: gettext('Type'), + dataIndex: 'type', + flex: 1, + }, + { + header: gettext('Comment'), + dataIndex: 'comment', + flex: 3, + }, +], + +selModel: { + selType: 'checkboxmodel', + mode: 'SIMPLE', +}, + +checkChangeEvents: [ + 'selectionchange', + 'change', +], + +listeners: { + selectionchange: function() { + // to trigger validity and error checks + this.checkChange(); + }, +}, + +getSubmitData: function() { + let me = this; + let res = {}; + res[me.name] = me.getValue(); + return res; +}, + +getValue: function() { + let me = this; + if (me.savedValue !== undefined) { + return me.savedValue; + } + let sm = me.getSelectionModel(); + return (sm.getSelection() ?? []).map(item => item.data.name); +}, + +setValueSelection: function(value) { + let me = this; + + let store = me.getStore(); + + let notFound = []; + let selection = value.map(item => { + let found = store.findRecord('name', item, 0, false, true, true); + if (!found) { + notFound.push(item); + } + return found; + }).filter(r => r); + + for (const name of notFound) { + let rec = store.add({ + name, + type: '-', + comment: gettext('Included target does not exist!'), + }); + selection.push(rec[0]); + } + + let sm = me.getSelectionModel(); + if (selection.length) { + sm.select(selection); + } else { + sm.deselectAll(); + } + // to correctly trigger invalid class + me.getErrors(); +}, + +setValue: function(value) { + let me = this; + + let store = me.getStore(); + if (!store.isLoaded()) { + me.savedValue = value; + store.on('load', function() { + me.setValueSelection(value); + delete me.savedValue; + }, { single: true }); + } else { + me.setValueSelection(value); + } + return me.mixins.field.setValue.call(me, value); +}, + +getErrors: function(value) { + let me = this; + if (!me.isDisabled() && me.allowBlank === false && + me.getSelectionModel().getCount() === 0) { + me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']); + return [gettext('No target selected')]; + } + + me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']); + return []; +}, + +initComponent: function() { + let me = this; + me.callParent(); + me.initField(); +}, + +}); -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-widget-toolkit 23/27] notification ui: remove notification groups
Signed-off-by: Lukas Wagner --- src/Makefile| 1 - src/Schema.js | 5 - src/panel/NotificationConfigView.js | 4 - src/panel/NotificationGroupEditPanel.js | 174 4 files changed, 184 deletions(-) delete mode 100644 src/panel/NotificationGroupEditPanel.js diff --git a/src/Makefile b/src/Makefile index 85ecea4..e07f17c 100644 --- a/src/Makefile +++ b/src/Makefile @@ -61,7 +61,6 @@ JSSRC=\ panel/LogView.js\ panel/NodeInfoRepoStatus.js \ panel/NotificationConfigView.js \ - panel/NotificationGroupEditPanel.js \ panel/JournalView.js\ panel/PermissionView.js \ panel/PruneKeepPanel.js \ diff --git a/src/Schema.js b/src/Schema.js index a7ffdf8..37ecd88 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -48,11 +48,6 @@ Ext.define('Proxmox.Schema', { // a singleton ipanel: 'pmxGotifyEditPanel', iconCls: 'fa-bell-o', }, - group: { - name: gettext('Notification Group'), - ipanel: 'pmxNotificationGroupEditPanel', - iconCls: 'fa-bell-o', - }, }, pxarFileTypes: { diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js index ff9c512..ba98395 100644 --- a/src/panel/NotificationConfigView.js +++ b/src/panel/NotificationConfigView.js @@ -191,10 +191,6 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { callback: 'reload', enableFn: rec => rec.data.name !== 'mail-to-root', getUrl: function(rec) { - if (rec.data.type === 'group') { - return `${me.baseUrl}/groups/${rec.getId()}`; - } - return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`; }, }, diff --git a/src/panel/NotificationGroupEditPanel.js b/src/panel/NotificationGroupEditPanel.js deleted file mode 100644 index 910d15a..000 --- a/src/panel/NotificationGroupEditPanel.js +++ /dev/null @@ -1,174 +0,0 @@ -Ext.define('Proxmox.panel.NotificationGroupEditPanel', { -extend: 'Proxmox.panel.InputPanel', -xtype: 'pmxNotificationGroupEditPanel', -mixins: ['Proxmox.Mixin.CBind'], - -type: 'group', - -items: [ - { - xtype: 'pmxDisplayEditField', - name: 'name', - cbind: { - value: '{name}', - editable: '{isCreate}', - }, - fieldLabel: gettext('Group Name'), - allowBlank: false, - }, - { - xtype: 'pmxNotificationEndpointSelector', - name: 'endpoint', - allowBlank: false, - }, - { - xtype: 'proxmoxtextfield', - name: 'comment', - fieldLabel: gettext('Comment'), - cbind: { - deleteEmpty: '{!isCreate}', - }, - }, -], -}); - -Ext.define('Proxmox.form.NotificationEndpointSelector', { -extend: 'Ext.grid.Panel', -alias: 'widget.pmxNotificationEndpointSelector', - -mixins: { - field: 'Ext.form.field.Field', -}, - -padding: '0 0 10 0', - -allowBlank: true, -selectAll: false, -isFormField: true, - -store: { - autoLoad: true, - model: 'proxmox-notification-endpoints', - sorters: 'name', - filters: item => item.data.type !== 'group', -}, - -columns: [ - { - header: gettext('Endpoint Name'), - dataIndex: 'name', - flex: 1, - }, - { - header: gettext('Type'), - dataIndex: 'type', - flex: 1, - }, - { - header: gettext('Comment'), - dataIndex: 'comment', - flex: 3, - }, -], - -selModel: { - selType: 'checkboxmodel', - mode: 'SIMPLE', -}, - -checkChangeEvents: [ - 'selectionchange', - 'change', -], - -listeners: { - selectionchange: function() { - // to trigger validity and error checks - this.checkChange(); - }, -}, - -getSubmitData: function() { - let me = this; - let res = {}; - res[me.name] = me.getValue(); - return res; -}, - -getValue: function() { - let me = this; - if (me.savedValue !== undefined) { - return me.savedValue
[pve-devel] [PATCH pve-manager 11/27] api: notification: remove notification groups
Signed-off-by: Lukas Wagner --- PVE/API2/Cluster/Notifications.pm | 267 +- 1 file changed, 4 insertions(+), 263 deletions(-) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index ec666903..b34802c8 100644 --- a/PVE/API2/Cluster/Notifications.pm +++ b/PVE/API2/Cluster/Notifications.pm @@ -121,7 +121,6 @@ __PACKAGE__->register_method ({ my $result = [ { name => 'endpoints' }, { name => 'filters' }, - { name => 'groups' }, { name => 'targets' }, ]; @@ -161,8 +160,7 @@ __PACKAGE__->register_method ({ name => 'get_all_targets', path => 'targets', method => 'GET', -description => 'Returns a list of all entities that can be used as notification targets' . - ' (endpoints and groups).', +description => 'Returns a list of all entities that can be used as notification targets.', permissions => { description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" . " 'Mapping.Audit' permissions on '/mapping/notification/'." @@ -180,14 +178,14 @@ __PACKAGE__->register_method ({ type => 'object', properties => { name => { - description => 'Name of the endpoint/group.', + description => 'Name of the target.', type => 'string', format => 'pve-configid', }, 'type' => { - description => 'Type of the endpoint or group.', + description => 'Type of the target.', type => 'string', - enum => [qw(sendmail gotify group)], + enum => [qw(sendmail gotify)], }, 'comment' => { description => 'Comment', @@ -221,14 +219,6 @@ __PACKAGE__->register_method ({ }; } - for my $target (@{$config->get_groups()}) { - push @$result, { - name => $target->{name}, - comment => $target->{comment}, - type => 'group', - }; - } - $result }; @@ -290,255 +280,6 @@ __PACKAGE__->register_method ({ } }); -my $group_properties = { -name => { - description => 'Name of the group.', - type => 'string', - format => 'pve-configid', -}, -'endpoint' => { - type => 'array', - items => { - type => 'string', - format => 'pve-configid', - }, - description => 'List of included endpoints', -}, -'comment' => { - description => 'Comment', - type => 'string', - optional => 1, -}, -filter => { - description => 'Name of the filter that should be applied.', - type => 'string', - format => 'pve-configid', - optional => 1, -}, -}; - -__PACKAGE__->register_method ({ -name => 'get_groups', -path => 'groups', -method => 'GET', -description => 'Returns a list of all groups', -protected => 1, -permissions => { - description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" - . " 'Mapping.Audit' permissions on '/mapping/notification/'.", - user => 'all', -}, -parameters => { - additionalProperties => 0, - properties => {}, -}, -returns => { - type => 'array', - items => { - type => 'object', - properties => $group_properties, - }, - links => [ { rel => 'child', href => '{name}' } ], -}, -code => sub { - my $config = PVE::Notify::read_config(); - my $rpcenv = PVE::RPCEnvironment::get(); - - my $entities = eval { - $config->get_groups(); - }; - raise_api_error($@) if $@; - - return filter_entities_by_privs($rpcenv, $entities); -} -}); - -__PACKAGE__->register_method ({ -name => 'get_group', -path => 'groups/{name}', -method => 'GET', -description => 'Retu
[pve-devel] [PATCH proxmox-widget-toolkit 22/27] notification ui: remove filter setting for targets
Signed-off-by: Lukas Wagner --- src/Makefile| 1 - src/form/NotificationFilterSelector.js | 58 - src/panel/GotifyEditPanel.js| 9 src/panel/NotificationGroupEditPanel.js | 9 src/panel/SendmailEditPanel.js | 9 5 files changed, 86 deletions(-) delete mode 100644 src/form/NotificationFilterSelector.js diff --git a/src/Makefile b/src/Makefile index 21fbe76..85ecea4 100644 --- a/src/Makefile +++ b/src/Makefile @@ -44,7 +44,6 @@ JSSRC=\ form/RoleSelector.js\ form/DiskSelector.js\ form/MultiDiskSelector.js \ - form/NotificationFilterSelector.js \ form/TaskTypeSelector.js\ form/ACME.js\ form/UserSelector.js\ diff --git a/src/form/NotificationFilterSelector.js b/src/form/NotificationFilterSelector.js deleted file mode 100644 index d2ab8be..000 --- a/src/form/NotificationFilterSelector.js +++ /dev/null @@ -1,58 +0,0 @@ -Ext.define('Proxmox.form.NotificationFilterSelector', { -extend: 'Proxmox.form.ComboGrid', -alias: ['widget.pmxNotificationFilterSelector'], - -// set default value to empty array, else it inits it with -// null and after the store load it is an empty array, -// triggering dirtychange -value: [], -valueField: 'name', -displayField: 'name', -deleteEmpty: true, -skipEmptyText: true, -allowBlank: true, -editable: false, -autoSelect: false, - -listConfig: { - columns: [ - { - header: gettext('Filter'), - dataIndex: 'name', - sortable: true, - hideable: false, - flex: 1, - }, - { - header: gettext('Comment'), - dataIndex: 'comment', - sortable: true, - hideable: false, - flex: 2, - }, - ], -}, - -initComponent: function() { - let me = this; - - Ext.apply(me, { - store: { - fields: ['name', 'comment'], - proxy: { - type: 'proxmox', - url: `/api2/json/${me.baseUrl}/filters`, - }, - sorters: [ - { - property: 'name', - direction: 'ASC', - }, - ], - autoLoad: true, - }, - }); - - me.callParent(); -}, -}); diff --git a/src/panel/GotifyEditPanel.js b/src/panel/GotifyEditPanel.js index 3ddcc4d..5d814e5 100644 --- a/src/panel/GotifyEditPanel.js +++ b/src/panel/GotifyEditPanel.js @@ -32,15 +32,6 @@ Ext.define('Proxmox.panel.GotifyEditPanel', { allowBlank: '{!isCreate}', }, }, - { - xtype: 'pmxNotificationFilterSelector', - name: 'filter', - fieldLabel: gettext('Filter'), - cbind: { - deleteEmpty: '{!isCreate}', - baseUrl: '{baseUrl}', - }, - }, { xtype: 'proxmoxtextfield', name: 'comment', diff --git a/src/panel/NotificationGroupEditPanel.js b/src/panel/NotificationGroupEditPanel.js index aa76810..910d15a 100644 --- a/src/panel/NotificationGroupEditPanel.js +++ b/src/panel/NotificationGroupEditPanel.js @@ -21,15 +21,6 @@ Ext.define('Proxmox.panel.NotificationGroupEditPanel', { name: 'endpoint', allowBlank: false, }, - { - xtype: 'pmxNotificationFilterSelector', - name: 'filter', - fieldLabel: gettext('Filter'), - cbind: { - deleteEmpty: '{!isCreate}', - baseUrl: '{baseUrl}', - }, - }, { xtype: 'proxmoxtextfield', name: 'comment', diff --git a/src/panel/SendmailEditPanel.js b/src/panel/SendmailEditPanel.js index ace6129..16abebc 100644 --- a/src/panel/SendmailEditPanel.js +++ b/src/panel/SendmailEditPanel.js @@ -86,15 +86,6 @@ Ext.define('Proxmox.panel.SendmailEditPanel', { return this.up('pmxSendmailEditPanel').mailValidator(); }, }, - { - xtype: 'pmxNotificationFilterSelector', - name: 'filter', - fieldLabel: gettext('Filter'), - cbind: { - deleteEmpty: '{!isCreate}', - baseUrl: '{baseUrl}', - }, - }, { xtype: 'proxmoxtextfield', name: 'comment', -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 07/27] notify: adapt to new matcher-based notification routing
Signed-off-by: Lukas Wagner --- common/src/notify.rs | 167 +-- 1 file changed, 50 insertions(+), 117 deletions(-) diff --git a/common/src/notify.rs b/common/src/notify.rs index 9f44225..4fbd705 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@ -1,10 +1,12 @@ #[perlmod::package(name = "Proxmox::RS::Notify")] mod export { +use std::collections::HashMap; +use std::sync::Mutex; + use anyhow::{bail, Error}; -use perlmod::Value; use serde_json::Value as JSONValue; -use std::sync::Mutex; +use perlmod::Value; use proxmox_http_error::HttpError; use proxmox_notify::endpoints::gotify::{ DeleteableGotifyProperty, GotifyConfig, GotifyConfigUpdater, GotifyPrivateConfig, @@ -13,10 +15,10 @@ mod export { use proxmox_notify::endpoints::sendmail::{ DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater, }; -use proxmox_notify::filter::{ -DeleteableFilterProperty, FilterConfig, FilterConfigUpdater, FilterModeOperator, +use proxmox_notify::matcher::{ +CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig, +MatcherConfigUpdater, SeverityMatcher, }; -use proxmox_notify::group::{DeleteableGroupProperty, GroupConfig, GroupConfigUpdater}; use proxmox_notify::{api, Config, Notification, Severity}; pub struct NotificationConfig { @@ -87,22 +89,22 @@ mod export { #[export(serialize_error)] fn send( #[try_from_ref] this: &NotificationConfig, -channel: &str, severity: Severity, title: String, body: String, -properties: Option, +template_data: Option, +fields: Option>, ) -> Result<(), HttpError> { let config = this.config.lock().unwrap(); - -let notification = Notification { +let notification = Notification::new_templated( severity, title, body, -properties, -}; +template_data.unwrap_or_default(), +fields.unwrap_or_default(), +); -api::common::send(&config, channel, ¬ification) +api::common::send(&config, ¬ification) } #[export(serialize_error)] @@ -114,78 +116,6 @@ mod export { api::common::test_target(&config, target) } -#[export(serialize_error)] -fn get_groups( -#[try_from_ref] this: &NotificationConfig, -) -> Result, HttpError> { -let config = this.config.lock().unwrap(); -api::group::get_groups(&config) -} - -#[export(serialize_error)] -fn get_group( -#[try_from_ref] this: &NotificationConfig, -id: &str, -) -> Result { -let config = this.config.lock().unwrap(); -api::group::get_group(&config, id) -} - -#[export(serialize_error)] -fn add_group( -#[try_from_ref] this: &NotificationConfig, -name: String, -endpoints: Vec, -comment: Option, -filter: Option, -) -> Result<(), HttpError> { -let mut config = this.config.lock().unwrap(); -api::group::add_group( -&mut config, -&GroupConfig { -name, -endpoint: endpoints, -comment, -filter, -}, -) -} - -#[export(serialize_error)] -fn update_group( -#[try_from_ref] this: &NotificationConfig, -name: &str, -endpoints: Option>, -comment: Option, -filter: Option, -delete: Option>, -digest: Option<&str>, -) -> Result<(), HttpError> { -let mut config = this.config.lock().unwrap(); -let digest = decode_digest(digest)?; - -api::group::update_group( -&mut config, -name, -&GroupConfigUpdater { -endpoint: endpoints, -comment, -filter, -}, -delete.as_deref(), -digest.as_deref(), -) -} - -#[export(serialize_error)] -fn delete_group( -#[try_from_ref] this: &NotificationConfig, -name: &str, -) -> Result<(), HttpError> { -let mut config = this.config.lock().unwrap(); -api::group::delete_group(&mut config, name) -} - #[export(serialize_error)] fn get_sendmail_endpoints( #[try_from_ref] this: &NotificationConfig, @@ -213,7 +143,6 @@ mod export { from_address: Option, author: Option, comment: Option, -filter: Option, ) -> Result<(), HttpError> { let mut config = this.config.lock().unwrap(); @@ -226,7 +155,7 @@ mod export { from_address,
[pve-devel] [PATCH proxmox 04/27] notify: add calendar matcher
This allows matching by a notification's timestamp: matcher: foo match-calendar mon..fri 8-12 Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/matcher.rs | 6 +++ proxmox-notify/src/lib.rs | 4 ++ proxmox-notify/src/matcher.rs | 65 +++ 3 files changed, 75 insertions(+) diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs index e37b74f..0592b14 100644 --- a/proxmox-notify/src/api/matcher.rs +++ b/proxmox-notify/src/api/matcher.rs @@ -80,6 +80,7 @@ pub fn update_matcher( match deleteable_property { DeleteableMatcherProperty::MatchSeverity => matcher.match_severity = None, DeleteableMatcherProperty::MatchField => matcher.match_field = None, +DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar = None, DeleteableMatcherProperty::Target => matcher.target = None, DeleteableMatcherProperty::Mode => matcher.mode = None, DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None, @@ -96,6 +97,10 @@ pub fn update_matcher( matcher.match_field = Some(match_field.clone()); } +if let Some(match_calendar) = &matcher_updater.match_calendar { +matcher.match_calendar = Some(match_calendar.clone()); +} + if let Some(mode) = matcher_updater.mode { matcher.mode = Some(mode); } @@ -200,6 +205,7 @@ matcher: matcher2 mode: Some(MatchModeOperator::Any), match_field: None, match_severity: None, +match_calendar: None, invert_match: Some(true), target: Some(vec!["foo".into()]), comment: Some("new comment".into()), diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 1f95ae0..9997cef 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -154,6 +154,8 @@ pub enum Content { pub struct Metadata { /// Notification severity severity: Severity, +/// Timestamp of the notification as a UNIX epoch +timestamp: i64, /// Additional fields for additional key-value metadata additional_fields: HashMap, } @@ -179,6 +181,7 @@ impl Notification { metadata: Metadata { severity, additional_fields: fields, +timestamp: proxmox_time::epoch_i64(), }, content: Content::Template { title_template: title.as_ref().to_string(), @@ -393,6 +396,7 @@ impl Bus { severity: Severity::Info, // TODO: what fields would make sense for test notifications? additional_fields: Default::default(), +timestamp: proxmox_time::epoch_i64(), }, content: Content::Template { title_template: "Test notification".into(), diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index c24726d..b03d11d 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -10,6 +10,7 @@ use proxmox_schema::api_types::COMMENT_SCHEMA; use proxmox_schema::{ api, const_regex, ApiStringFormat, Schema, StringSchema, Updater, SAFE_ID_REGEX_STR, }; +use proxmox_time::{parse_daily_duration, DailyDuration}; use crate::schema::ENTITY_NAME_SCHEMA; use crate::{Error, Notification, Severity}; @@ -88,6 +89,14 @@ pub const MATCH_FIELD_ENTRY_SCHEMA: Schema = StringSchema::new("Match metadata f }, optional: true, }, +"match-calendar": { +type: Array, +items: { +description: "Time stamps to match", +type: String +}, +optional: true, +}, "target": { type: Array, items: { @@ -112,6 +121,10 @@ pub struct MatcherConfig { #[serde(skip_serializing_if = "Option::is_none")] pub match_severity: Option>, +/// List of matched severity levels +#[serde(skip_serializing_if = "Option::is_none")] +pub match_calendar: Option>, + /// Decide if 'all' or 'any' match statements must match #[serde(skip_serializing_if = "Option::is_none")] pub mode: Option, @@ -249,6 +262,7 @@ impl MatcherConfig { let mut is_match = mode.neutral_element(); is_match = mode.apply(is_match, self.check_severity_match(notification)); is_match = mode.apply(is_match, self.check_field_match(notification)?); +is_match = mode.apply(is_match, self.check_calendar_match(notification)?); let invert_match = self.invert_match.unwrap_or_default(); @@ -285,6 +299,19 @@ impl MatcherConfig { is_match } + +fn check_calendar_match(
[pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters
This series replaces notification filters and groups with notification matchers. Instead of having a per-notification event target/policy setting (at the moment stored in datacenter.cfg and jobs.cfg), this shifts the routing part into the matcher completely. Config example, I think this demonstrates the principle quite nicely: sendmail: default-target mailto-user root@pam matcher: fencing-for-node mode all # all match-directives have to match, default match-field exact:hostname=pve.example.com match-field exact:type=fencing target default-target --> Send all fencing notifications for a certain host to a certain target. Right now, there are three different match-directives: - match-field: exact/regex match for notification metadata fields - match-severity: match notification severities (info,notice,warning,error) - match-calender: match notification timestamp example: match-calendar mon..fri 8-12 The old target/policy based notification was already in the pvetest repository. Thus we take special care that there is no breakage when the notification system encounters old settings/configuration keys. It will clean them out/migrate them if possible. These changes also modify pve-manager's postinst hook. It will now create a default config if /etc/pve/notifications.cfg does not exist yet. What I tested: - Made sure existing notifications continue to work (replication/fencing in a cluster setup, backups, system updates) - Made sure that the 'legacy' mailto parameter for backups also works - Tested the new UI for notification matchers - Tested postinst hook for pve-manager - Tested whether old config keys for filters and groups break anything @TESTERS: I have built packages for these changes to ease testing, they are on nasi: packages/notifications Followup work in the near future: - Adapt documentation, this will follow asap - UI code for notification matcher config is a bit messy, I will send a cleanup-patch - main focus right now was to get it working - Mark 'mailto' in backup jobs as deprecated in UI - while also migrating automatically to the new system (create an endpoint/matcher when creating/updating a backup job) proxmox: Lukas Wagner (6): notify: introduce Error::Generic notify: factor out notification content into its own type notify: replace filters and groups with matcher-based system notify: add calendar matcher notify: matcher: introduce common trait for match directives notify: let a matcher always match if it has no matching directives proxmox-notify/Cargo.toml| 2 + proxmox-notify/examples/render.rs| 4 +- proxmox-notify/src/api/common.rs | 6 +- proxmox-notify/src/api/filter.rs | 231 --- proxmox-notify/src/api/gotify.rs | 16 - proxmox-notify/src/api/group.rs | 259 proxmox-notify/src/api/matcher.rs| 260 proxmox-notify/src/api/mod.rs| 115 +- proxmox-notify/src/api/sendmail.rs | 15 - proxmox-notify/src/config.rs | 34 +- proxmox-notify/src/endpoints/gotify.rs | 41 +- proxmox-notify/src/endpoints/sendmail.rs | 76 ++-- proxmox-notify/src/filter.rs | 193 + proxmox-notify/src/group.rs | 40 +- proxmox-notify/src/lib.rs| 387 -- proxmox-notify/src/matcher.rs| 484 +++ proxmox-notify/src/renderer/mod.rs | 15 +- proxmox-notify/src/schema.rs | 11 +- 18 files changed, 1046 insertions(+), 1143 deletions(-) delete mode 100644 proxmox-notify/src/api/filter.rs delete mode 100644 proxmox-notify/src/api/group.rs create mode 100644 proxmox-notify/src/api/matcher.rs create mode 100644 proxmox-notify/src/matcher.rs proxmox-perl-rs: Lukas Wagner (1): notify: adapt to new matcher-based notification routing common/src/notify.rs | 167 +-- 1 file changed, 50 insertions(+), 117 deletions(-) pve-cluster: Lukas Wagner (1): notify: adapt to matcher based notification system src/PVE/Notify.pm | 101 +- 1 file changed, 47 insertions(+), 54 deletions(-) pve-guest-common: Lukas Wagner (1): vzdump: deprecate mailto/mailnotification/notification-{target,policy} src/PVE/VZDump/Common.pm | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) pve-ha-manager: Lukas Wagner (1): env: switch to matcher-based notification system src/PVE/HA/Env/PVE2.pm | 10 ++ src/PVE/HA/NodeStatus.pm | 11 +-- 2 files changed, 11 insertions(+), 10 deletions(-) pve-manager: Lukas Wagner (10): api: notification: remove notification groups api: notification: add new matcher-based notification API ui: dc: remove unneeded notification events panel vzdump: adapt to new matcher based notification system api:
[pve-devel] [PATCH pve-cluster 08/27] notify: adapt to matcher based notification system
This commit removes the target paramters from all notify calls. Also, the default 'mail-to-root' target is not added automatically any more - this target will be added by an dpkg hook in the future. Signed-off-by: Lukas Wagner --- src/PVE/Notify.pm | 101 +- 1 file changed, 47 insertions(+), 54 deletions(-) diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm index 419bf6d..872eb25 100644 --- a/src/PVE/Notify.pm +++ b/src/PVE/Notify.pm @@ -18,8 +18,6 @@ cfs_register_file( \&write_notification_config, ); -my $mail_to_root_target = 'mail-to-root'; - sub parse_notification_config { my ($filename, $raw) = @_; @@ -48,86 +46,81 @@ sub read_config { my $notification_config = Proxmox::RS::Notify->parse_config($config, $priv_config); -eval { - # This target should always be available... - $notification_config->add_sendmail_endpoint( - $mail_to_root_target, - undef, - ['root@pam'], - undef, - undef, - 'Send mail to root@pam\'s email address' - ); -}; - return $notification_config; } sub write_config { my ($notification_config) = @_; -eval { - # ... but don't persist it to the config. - # Rationale: If it is in the config, the user might think - # that it can be changed by editing the configuration there. - # However, since we always add it in `read_config`, any changes - # will be implicitly overridden by the default. - - # If users want's to change the configuration, they are supposed to - # create a new sendmail endpoint. - $notification_config->delete_sendmail_endpoint($mail_to_root_target); -}; - my ($config, $priv_config) = $notification_config->write_config(); cfs_write_file('notifications.cfg', $config, 1); cfs_write_file('priv/notifications.cfg', $priv_config, 1); } -sub default_target { -return $mail_to_root_target; -} - my $send_notification = sub { -my ($target, $severity, $title, $message, $properties, $config) = @_; +my ($severity, $title, $message, $template_data, $fields, $config) = @_; $config = read_config() if !defined($config); -my ($module, $file, $line) = caller(1); - -# Augment properties with the source code location of the notify call -my $props_with_source = { - %$properties, - source => { - module => $module, - file => $file, - line => $line, - } -}; - -$config->send($target, $severity, $title, $message, $props_with_source); +$config->send($severity, $title, $message, $template_data, $fields); }; sub notify { -my ($target, $severity, $title, $message, $properties, $config) = @_; -$send_notification->($target, $severity, $title, $message, $properties, $config); +my ($severity, $title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +$severity, +$title, +$message, +$template_data, +$fields, +$config +); } sub info { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'info', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'info', +$title, +$message, +$template_data, +$fields, +$config +); } sub notice { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'notice', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'notice', +$title, +$message, +$template_data, +$fields, +$config +); } sub warning { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'warning', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'warning', +$title, +$message, +$template_data, +$fields, +$config +); } sub error { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'error', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'error', +$title, +$message, +$template_data, +$fields, +$config +); } sub check_may_use_target { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 16/27] api: replication: adapt to matcher-based notification system
Signed-off-by: Lukas Wagner --- PVE/API2/Replication.pm | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm index d61518ba..0dc944c9 100644 --- a/PVE/API2/Replication.pm +++ b/PVE/API2/Replication.pm @@ -129,7 +129,7 @@ my sub _handle_job_err { # The replication job is run every 15 mins if no schedule is set. my $schedule = $job->{schedule} // '*/15'; -my $properties = { +my $template_data = { "failure-count" => $fail_count, "last-sync" => $jobstate->{last_sync}, "next-sync" => $next_sync, @@ -139,19 +139,18 @@ my sub _handle_job_err { "error" => $err, }; +my $metadata_fields = { + # TODO: Add job-id? + type => "replication", +}; + eval { - my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg'); - my $target = $dcconf->{notify}->{'target-replication'} // PVE::Notify::default_target(); - my $notify = $dcconf->{notify}->{'replication'} // 'always'; - - if ($notify eq 'always') { - PVE::Notify::error( - $target, - $replication_error_subject_template, - $replication_error_body_template, - $properties - ); - } + PVE::Notify::error( + $replication_error_subject_template, + $replication_error_body_template, + $template_data, + $metadata_fields + ); }; warn ": $@" if $@; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 18/27] test: fix vzdump notification test
The signature of the PVE::Notify functions have changed, this commit adapts the mocked functions so that the tests work again. Signed-off-by: Lukas Wagner --- test/vzdump_notification_test.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/vzdump_notification_test.pl b/test/vzdump_notification_test.pl index 21c31651..631606bb 100755 --- a/test/vzdump_notification_test.pl +++ b/test/vzdump_notification_test.pl @@ -38,14 +38,14 @@ my $result_properties; my $mock_notification_module = Test::MockModule->new('PVE::Notify'); my $mocked_notify = sub { -my ($channel, $severity, $title, $text, $properties) = @_; +my ($severity, $title, $text, $properties, $metadata) = @_; $result_text = $text; $result_properties = $properties; }; my $mocked_notify_short = sub { -my ($channel, @rest) = @_; -return $mocked_notify->($channel, '', @rest); +my (@params) = @_; +return $mocked_notify->('', @params); }; $mock_notification_module->mock( -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 20/27] ui: dc: config: show notification panel again
Rework should be done now. Signed-off-by: Lukas Wagner --- www/manager6/dc/Config.js | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index 0dea1c67..74a84e91 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -317,14 +317,9 @@ Ext.define('PVE.dc.Config', { ); } - // this is being reworked, but we need to release newer manager versions already.. - let notification_enabled = false; - if (notification_enabled && ( - caps.mapping['Mapping.Audit'] || - caps.mapping['Mapping.Use'] || - caps.mapping['Mapping.Modify'] - ) - ) { + if (caps.mapping['Mapping.Audit'] || + caps.mapping['Mapping.Use'] || + caps.mapping['Mapping.Modify']) { me.items.push( { xtype: 'pmxNotificationConfigView', -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 15/27] api: apt: adapt to matcher-based notifications
Signed-off-by: Lukas Wagner --- PVE/API2/APT.pm | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm index a213fc59..da75a4dc 100644 --- a/PVE/API2/APT.pm +++ b/PVE/API2/APT.pm @@ -286,8 +286,6 @@ __PACKAGE__->register_method({ description => "This is used to resynchronize the package index files from their sources (apt-get update).", permissions => { check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]], - description => "If 'notify: target-package-updates' is set, then the user must have the " - . "'Mapping.Use' permission on '/mapping/notification/'", }, protected => 1, proxyto => 'node', @@ -297,7 +295,7 @@ __PACKAGE__->register_method({ node => get_standard_option('pve-node'), notify => { type => 'boolean', - description => "Send notification mail about new packages (to email address specified for user 'root\@pam').", + description => "Send notification about new packages.", optional => 1, default => 0, }, @@ -317,16 +315,6 @@ __PACKAGE__->register_method({ my $rpcenv = PVE::RPCEnvironment::get(); my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg'); - my $target = $dcconf->{notify}->{'target-package-updates'} // - PVE::Notify::default_target(); - - if ($param->{notify} && $target ne PVE::Notify::default_target()) { - # If we notify via anything other than the default target (mail to root), - # then the user must have the proper permissions for the target. - # The mail-to-root target does not require these, as otherwise - # we would break compatibility. - PVE::Notify::check_may_use_target($target, $rpcenv); - } my $authuser = $rpcenv->get_user(); @@ -392,16 +380,23 @@ __PACKAGE__->register_method({ return if !$count; - my $properties = { + my $template_data = { updates => $updates_table, hostname => $hostname, }; + # Additional metadata fields that can be used in notification + # matchers. + my $metadata_fields = { + type => 'package-updates', + hostname => $hostname, + }; + PVE::Notify::info( - $target, $updates_available_subject_template, $updates_available_body_template, - $properties, + $template_data, + $metadata_fields, ); foreach my $pi (@$pkglist) { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 17/27] debian: postinst: create notifications.cfg if it does not exist
We only warn on failure so that the postinst script does not fail in case pmxcfs is not running. Signed-off-by: Lukas Wagner --- debian/postinst | 28 1 file changed, 28 insertions(+) diff --git a/debian/postinst b/debian/postinst index 4c9a1f25..7dad2b1a 100755 --- a/debian/postinst +++ b/debian/postinst @@ -93,6 +93,32 @@ migrate_apt_auth_conf() { fi } +write_notification_cfg() { +# Create default config: +# A sendmail-target that sends to root@pam, and a +# matcher that sends all notifications to this target +cat >> /etc/pve/notifications.cfg <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 14/27] vzdump: adapt to new matcher based notification system
To ease the migration from old-style mailto/mailnotification paramters for backup jobs, the code will add a ephemeral sendmail endpoint and a matcher. Signed-off-by: Lukas Wagner --- PVE/API2/VZDump.pm | 8 +--- PVE/VZDump.pm | 40 +++- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm index 3886772e..f66fc740 100644 --- a/PVE/API2/VZDump.pm +++ b/PVE/API2/VZDump.pm @@ -44,9 +44,7 @@ __PACKAGE__->register_method ({ ."'Datastore.AllocateSpace' on the backup storage. The 'tmpdir', 'dumpdir' and " ."'script' parameters are restricted to the 'root\@pam' user. The 'maxfiles' and " ."'prune-backups' settings require 'Datastore.Allocate' on the backup storage. The " - ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. " - ."If 'notification-target' is set, then the 'Mapping.Use' permission is needed on " - ."'/mapping/notification/'.", + ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. ", user => 'all', }, protected => 1, @@ -115,10 +113,6 @@ __PACKAGE__->register_method ({ $rpcenv->check($user, "/storage/$storeid", [ 'Datastore.AllocateSpace' ]); } - if (my $target = $param->{'notification-target'}) { - PVE::Notify::check_may_use_target($target, $rpcenv); - } - my $worker = sub { my $upid = shift; diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index 454ab494..b0574d41 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -452,20 +452,18 @@ sub send_notification { my $opts = $self->{opts}; my $mailto = $opts->{mailto}; my $cmdline = $self->{cmdline}; -my $target = $opts->{"notification-target"}; -# Fall back to 'mailnotification' if 'notification-policy' is not set. -# If both are set, 'notification-policy' takes precedence -my $policy = $opts->{"notification-policy"} // $opts->{mailnotification} // 'always'; +# Old-style notification policy. This parameter will influce +# if an ad-hoc notification target/matcher will be created. +my $policy = $opts->{"notification-policy"} // + $opts->{mailnotification} // + 'always'; -return if ($policy eq 'never'); sanitize_task_list($tasklist); my $error_count = count_failed_tasks($tasklist); my $failed = ($error_count || $err); -return if (!$failed && ($policy eq 'failure')); - my $status_text = $failed ? 'backup failed' : 'backup successful'; if ($err) { @@ -489,8 +487,10 @@ sub send_notification { "See Task History for details!\n"; }; +my $hostname = get_hostname(); + my $notification_props = { - "hostname" => get_hostname(), + "hostname" => $hostname, "error-message" => $err, "guest-table" => build_guest_table($tasklist), "logs" => $text_log_part, @@ -498,9 +498,16 @@ sub send_notification { "total-time"=> $total_time, }; +my $fields = { + type => "vzdump", + hostname => $hostname, +}; + my $notification_config = PVE::Notify::read_config(); -if ($mailto && scalar(@$mailto)) { +my $legacy_sendmail = $policy eq "always" || ($policy eq "failure" && $failed); + +if ($mailto && scalar(@$mailto) && $legacy_sendmail) { # <, >, @ are not allowed in endpoint names, but that is only # verified once the config is serialized. That means that # we can rely on that fact that no other endpoint with this name exists. @@ -514,29 +521,20 @@ sub send_notification { my $endpoints = [$endpoint_name]; - # Create an anonymous group containing the sendmail endpoint and the - # $target endpoint, if specified - if ($target) { - push @$endpoints, $target; - } - - $target = ""; - $notification_config->add_group( - $target, + $notification_config->add_matcher( + "", $endpoints, ); } -return if (!$target); - my $severity = $failed ? "error" : "info"; PVE::Notify::notify( - $target, $severity, $subject_template, $body_template, $notification_props, + $fields, $notification_config ); }; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 19/27] ui: vzdump: remove left-overs from target/policy based notifications
Signed-off-by: Lukas Wagner --- www/manager6/dc/Backup.js | 81 --- .../form/NotificationPolicySelector.js| 1 - www/manager6/window/Backup.js | 35 +--- 3 files changed, 15 insertions(+), 102 deletions(-) diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js index 0c8d2d4f..e1c76a1d 100644 --- a/www/manager6/dc/Backup.js +++ b/www/manager6/dc/Backup.js @@ -36,29 +36,11 @@ Ext.define('PVE.dc.BackupEdit', { delete values.node; } - if (!isCreate) { - // 'mailnotification' is deprecated in favor of 'notification-policy' - // -> Migration to the new parameter happens in init, so we are - //safe to remove the old parameter here. - Proxmox.Utils.assemble_field_data(values, { 'delete': 'mailnotification' }); - - // If sending notifications via mail, remove the current value of - // 'notification-target' - if (values['notification-mode'] === "mailto") { - Proxmox.Utils.assemble_field_data( - values, - { 'delete': 'notification-target' }, - ); - } else { - // and vice versa... - Proxmox.Utils.assemble_field_data( - values, - { 'delete': 'mailto' }, - ); - } - } - - delete values['notification-mode']; + // Get rid of new-old parameters for notification settings. + // These should only be set for those selected few who ran + // pve-manager from pvetest. + Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-policy' }); + Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-target' }); if (!values.id && isCreate) { values.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13); @@ -170,20 +152,14 @@ Ext.define('PVE.dc.BackupEdit', { success: function(response, _options) { let data = response.result.data; - // 'mailnotification' is deprecated. Let's automatically - // migrate to the compatible 'notification-policy' parameter - if (data.mailnotification) { - if (!data["notification-policy"]) { - data["notification-policy"] = data.mailnotification; - } - - delete data.mailnotification; - } - - if (data['notification-target']) { - data['notification-mode'] = 'notification-target'; - } else if (data.mailto) { - data['notification-mode'] = 'mailto'; + // Migrate 'new'-old notification-policy back to + // old-old mailnotification. Only should affect + // users who used pve-manager from pvetest. + // This was a remnant of notifications before the + // overhaul. + let policy = data['notification-policy']; + if (policy === 'always' || policy === 'failure') { + data.mailnotification = policy; } if (data.exclude) { @@ -228,7 +204,6 @@ Ext.define('PVE.dc.BackupEdit', { viewModel: { data: { selMode: 'include', - notificationMode: 'notification-target', }, formulas: { @@ -327,44 +302,16 @@ Ext.define('PVE.dc.BackupEdit', { { xtype: 'pveEmailNotificationSelector', fieldLabel: gettext('Notify'), - name: 'notification-policy', + name: 'mailnotification', cbind: { value: (get) => get('isCreate') ? 'always' : '', deleteEmpty: '{!isCreate}', }, }, - { - xtype:
[pve-devel] [PATCH proxmox-widget-toolkit 27/27] noficiation: matcher edit: make 'field' an editable combobox
For now with fixed options that are shared between most notification events - later, once we have a notification registry, this should be filled dynamically. Signed-off-by: Lukas Wagner --- src/window/NotificationMatcherEdit.js | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js index c6f0726..fb55e17 100644 --- a/src/window/NotificationMatcherEdit.js +++ b/src/window/NotificationMatcherEdit.js @@ -963,14 +963,23 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', { }, { fieldLabel: gettext('Field'), - xtype: 'textfield', + xtype: 'proxmoxKVComboBox', isFormField: false, submitValue: false, + allowBlank: false, + editable: true, + displayField: 'key', bind: { hidden: '{!typeIsMatchField}', disabled: '{!typeIsMatchField}', value: '{matchFieldField}', }, + // TODO: Once we have a 'notification registry', we should + // retrive those via an API call. + comboItems: [ + ['type', ''], + ['hostname', ''], + ], }, { fieldLabel: gettext('Value'), -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing
This modifies the old filter edit window in the following ways: - Split content into multiple panels - Name and comment in the first tab - Match rules in a tree-structure in the second tab - Targets to notify in the third tab Signed-off-by: Lukas Wagner --- Notes: The code binding the match rule tree structure to the editable fields could definitely be a bit cleaner. I think this is the first time that we have used such a pattern, so there there was much experimentation needed to get this working. I plan to revisit it and clean up a bit later, I wanted to get the notification system changes on the list ASAP. src/window/NotificationMatcherEdit.js | 867 -- 1 file changed, 820 insertions(+), 47 deletions(-) diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js index a014f3e..c6f0726 100644 --- a/src/window/NotificationMatcherEdit.js +++ b/src/window/NotificationMatcherEdit.js @@ -1,6 +1,6 @@ -Ext.define('Proxmox.panel.NotificationMatcherEditPanel', { +Ext.define('Proxmox.panel.NotificationMatcherGeneralPanel', { extend: 'Proxmox.panel.InputPanel', -xtype: 'pmxNotificationMatcherEditPanel', +xtype: 'pmxNotificationMatcherGeneralPanel', mixins: ['Proxmox.Mixin.CBind'], items: [ @@ -15,53 +15,27 @@ Ext.define('Proxmox.panel.NotificationMatcherEditPanel', { allowBlank: false, }, { - xtype: 'proxmoxKVComboBox', - name: 'min-severity', - fieldLabel: gettext('Minimum Severity'), - value: null, + xtype: 'proxmoxtextfield', + name: 'comment', + fieldLabel: gettext('Comment'), cbind: { deleteEmpty: '{!isCreate}', }, - comboItems: [ - ['info', 'info'], - ['notice', 'notice'], - ['warning', 'warning'], - ['error', 'error'], - ], - triggers: { - clear: { - cls: 'pmx-clear-trigger', - weight: -1, - hidden: false, - handler: function() { - this.setValue(''); - }, - }, - }, - }, - { - xtype: 'proxmoxcheckbox', - fieldLabel: gettext('Invert match'), - name: 'invert-match', - uncheckedValue: 0, - defaultValue: 0, - cbind: { - deleteDefaultValue: '{!isCreate}', - }, }, +], +}); + +Ext.define('Proxmox.panel.NotificationMatcherTargetPanel', { +extend: 'Proxmox.panel.InputPanel', +xtype: 'pmxNotificationMatcherTargetPanel', +mixins: ['Proxmox.Mixin.CBind'], + +items: [ { xtype: 'pmxNotificationTargetSelector', name: 'target', allowBlank: false, }, - { - xtype: 'proxmoxtextfield', - name: 'comment', - fieldLabel: gettext('Comment'), - cbind: { - deleteEmpty: '{!isCreate}', - }, - }, ], }); @@ -74,7 +48,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', { labelWidth: 120, }, -width: 500, +width: 700, initComponent: function() { let me = this; @@ -97,12 +71,38 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', { me.subject = gettext('Notification Matcher'); Ext.apply(me, { - items: [{ - name: me.name, - xtype: 'pmxNotificationMatcherEditPanel', - isCreate: me.isCreate, - baseUrl: me.baseUrl, - }], + bodyPadding: 0, + items: [ + { + xtype: 'tabpanel', + region: 'center', + layout: 'fit', + bodyPadding: 10, + items: [ + { + name: me.name, + title: gettext('General'), + xtype: 'pmxNotificationMatcherGeneralPanel', + isCreate: me.isCreate, + baseUrl: me.baseUrl, + }, + { + name: me.name, + title: gettext('Match Rules'), +
[pve-devel] [PATCH proxmox-widget-toolkit 24/27] notification ui: rename filter to matcher
Signed-off-by: Lukas Wagner --- src/Makefile | 2 +- src/data/model/NotificationConfig.js | 2 +- src/panel/NotificationConfigView.js | 26 +-- ...lterEdit.js => NotificationMatcherEdit.js} | 14 +- 4 files changed, 22 insertions(+), 22 deletions(-) rename src/window/{NotificationFilterEdit.js => NotificationMatcherEdit.js} (92%) diff --git a/src/Makefile b/src/Makefile index e07f17c..c6d31c3 100644 --- a/src/Makefile +++ b/src/Makefile @@ -88,7 +88,7 @@ JSSRC=\ window/ACMEPluginEdit.js\ window/ACMEDomains.js \ window/EndpointEditBase.js \ - window/NotificationFilterEdit.js \ + window/NotificationMatcherEdit.js \ window/FileBrowser.js \ window/AuthEditBase.js \ window/AuthEditOpenId.js\ diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js index bb4ef85..f447db4 100644 --- a/src/data/model/NotificationConfig.js +++ b/src/data/model/NotificationConfig.js @@ -7,7 +7,7 @@ Ext.define('proxmox-notification-endpoints', { idProperty: 'name', }); -Ext.define('proxmox-notification-filters', { +Ext.define('proxmox-notification-matchers', { extend: 'Ext.data.Model', fields: ['name', 'comment'], proxy: { diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js index ba98395..ecf764d 100644 --- a/src/panel/NotificationConfigView.js +++ b/src/panel/NotificationConfigView.js @@ -21,7 +21,7 @@ Ext.define('Proxmox.panel.NotificationConfigView', { border: false, collapsible: true, animCollapse: false, - xtype: 'pmxNotificationFilterView', + xtype: 'pmxNotificationMatcherView', cbind: { baseUrl: '{baseUrl}', }, @@ -209,21 +209,21 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { }, }); -Ext.define('Proxmox.panel.NotificationFilterView', { +Ext.define('Proxmox.panel.NotificationMatcherView', { extend: 'Ext.grid.Panel', -alias: 'widget.pmxNotificationFilterView', +alias: 'widget.pmxNotificationMatcherView', -title: gettext('Notification Filters'), +title: gettext('Notification Matchers'), controller: { xclass: 'Ext.app.ViewController', - openEditWindow: function(filter) { + openEditWindow: function(matcher) { let me = this; - Ext.create('Proxmox.window.NotificationFilterEdit', { + Ext.create('Proxmox.window.NotificationMatcherEdit', { baseUrl: me.getView().baseUrl, - name: filter, + name: matcher, autoShow: true, listeners: { destroy: () => me.reload(), @@ -253,12 +253,12 @@ Ext.define('Proxmox.panel.NotificationFilterView', { activate: 'reload', }, -emptyText: gettext('No notification filters configured'), +emptyText: gettext('No notification matchers configured'), columns: [ { dataIndex: 'name', - text: gettext('Filter Name'), + text: gettext('Matcher Name'), renderer: Ext.String.htmlEncode, flex: 1, }, @@ -276,8 +276,8 @@ Ext.define('Proxmox.panel.NotificationFilterView', { autoDestroyRstore: true, rstore: { type: 'update', - storeid: 'proxmox-notification-filters', - model: 'proxmox-notification-filters', + storeid: 'proxmox-notification-matchers', + model: 'proxmox-notification-matchers', autoStart: true, }, sorters: 'name', @@ -307,12 +307,12 @@ Ext.define('Proxmox.panel.NotificationFilterView', { { xtype: 'proxmoxStdRemoveButton', callback: 'reload', - baseurl: `${me.baseUrl}/filters`, + baseurl: `${me.baseUrl}/matchers`, }, ], }); me.callParent(); - me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/filters`); + me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/matchers`); }, }); diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationMatcherEdit.js similarity index 92% rename from src/window/NotificationFilterEdit.js rename to src/window/NotificationMatcherEdit.js index bcde4fa..a014f3e 100644 --- a/src/window/NotificationFilterEdit
[pve-devel] [PATCH pve-manager 13/27] ui: dc: remove unneeded notification events panel
The notification event settings are replaced by notification matchers, which will combine the notification routing and filtering into a single concept. Signed-off-by: Lukas Wagner --- www/manager6/Makefile | 4 - www/manager6/dc/Config.js | 17 +- www/manager6/dc/NotificationEvents.js | 276 -- 3 files changed, 2 insertions(+), 295 deletions(-) delete mode 100644 www/manager6/dc/NotificationEvents.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index 57e1b48f..18baa024 100644 --- a/www/manager6/Makefile +++ b/www/manager6/Makefile @@ -159,7 +159,6 @@ JSSRC= \ dc/Health.js\ dc/Log.js \ dc/NodeView.js \ - dc/NotificationEvents.js\ dc/OptionView.js\ dc/PermissionView.js\ dc/PoolEdit.js \ @@ -346,6 +345,3 @@ install: pvemanagerlib.js .PHONY: clean clean: rm -rf pvemanagerlib.js OnlineHelpInfo.js .lint-incremental - - - diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index 7d01da5f..0dea1c67 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -319,18 +319,6 @@ Ext.define('PVE.dc.Config', { // this is being reworked, but we need to release newer manager versions already.. let notification_enabled = false; - if (notification_enabled && caps.dc['Sys.Audit']) { - me.items.push( - { - xtype: 'pveNotificationEvents', - title: gettext('Notifications'), - onlineHelp: 'notification_events', - iconCls: 'fa fa-bell-o', - itemId: 'notifications', - }, - ); - } - if (notification_enabled && ( caps.mapping['Mapping.Audit'] || caps.mapping['Mapping.Use'] || @@ -340,12 +328,11 @@ Ext.define('PVE.dc.Config', { me.items.push( { xtype: 'pmxNotificationConfigView', - title: gettext('Notification Targets'), + title: gettext('Notifications'), onlineHelp: 'notification_targets', itemId: 'notification-targets', - iconCls: 'fa fa-dot-circle-o', + iconCls: 'fa fa-bell-o', baseUrl: '/cluster/notifications', - groups: ['notifications'], }, ); } diff --git a/www/manager6/dc/NotificationEvents.js b/www/manager6/dc/NotificationEvents.js deleted file mode 100644 index 18816290.. --- a/www/manager6/dc/NotificationEvents.js +++ /dev/null @@ -1,276 +0,0 @@ -Ext.define('PVE.dc.NotificationEventsPolicySelector', { -alias: ['widget.pveNotificationEventsPolicySelector'], -extend: 'Proxmox.form.KVComboBox', -deleteEmpty: false, -value: '__default__', - -config: { - warningRef: null, - warnIfValIs: null, -}, - -listeners: { - change: function(field, newValue) { - let me = this; - if (!me.warningRef && !me.warnIfValIs) { - return; - } - - let warningField = field.nextSibling( - `displayfield[reference=${me.warningRef}]`, - ); - warningField.setVisible(newValue === me.warnIfValIs); - }, -}, -}); - -Ext.define('PVE.dc.NotificationEventDisabledWarning', { -alias: ['widget.pveNotificationEventDisabledWarning'], -extend: 'Ext.form.field.Display', -userCls: 'pmx-hint', -hidden: true, -value: gettext('Disabling notifications is not recommended for production systems!'), -}); - -Ext.define('PVE.dc.NotificationEventsTargetSelector', { -alias: ['widget.pveNotificationEventsTargetSelector'], -extend: 'PVE.form.NotificationTargetSelector', -fieldLabel: gettext('Notification Target'), -allowBlank: true, -editable: true, -autoSelect: false, -deleteEmpty: false, -emptyText: `${Proxmox.Utils.defaultText} (mail-to-root)`, -}); - -Ext.define('PVE.dc.NotificationEvents', { -extend: 'Proxmox.grid.ObjectGrid', -alias: ['widget.pveNotificationEvents'], - -// Taken from OptionView.js, but adapted slightly. -// The modified version allows us to have multiple rows in the ObjectGrid -// for the same un
[pve-devel] [PATCH pve-manager 12/27] api: notification: add new matcher-based notification API
This renames filters -> matchers and adds new configuration options needed by matchers (e.g. match-field, match-calendar, etc.) Signed-off-by: Lukas Wagner --- PVE/API2/Cluster/Notifications.pm | 195 ++ 1 file changed, 88 insertions(+), 107 deletions(-) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index b34802c8..8f716f26 100644 --- a/PVE/API2/Cluster/Notifications.pm +++ b/PVE/API2/Cluster/Notifications.pm @@ -68,37 +68,12 @@ sub filter_entities_by_privs { "/mapping/notification/$_->{name}", $can_see_mapping_privs, 1 - ) || $_->{name} eq PVE::Notify::default_target(); + ); } @$entities]; return $filtered; } -sub target_used_by { -my ($target) = @_; - -my $used_by = []; - -# Check keys in datacenter.cfg -my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg'); -for my $key (qw(target-package-updates target-replication target-fencing)) { - if ($dc_conf->{notify} && $dc_conf->{notify}->{$key} eq $target) { - push @$used_by, $key; - } -} - -# Check backup jobs -my $jobs_conf = PVE::Cluster::cfs_read_file('jobs.cfg'); -for my $key (keys %{$jobs_conf->{ids}}) { - my $job = $jobs_conf->{ids}->{$key}; - if ($job->{'notification-target'} eq $target) { - push @$used_by, $key; - } -} - -return join(', ', @$used_by); -} - __PACKAGE__->register_method ({ name => 'index', path => '', @@ -120,7 +95,7 @@ __PACKAGE__->register_method ({ code => sub { my $result = [ { name => 'endpoints' }, - { name => 'filters' }, + { name => 'matchers' }, { name => 'targets' }, ]; @@ -259,15 +234,11 @@ __PACKAGE__->register_method ({ my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit']; - if ($name ne PVE::Notify::default_target()) { - # Due to backwards compatibility reasons the 'mail-to-root' - # target must be accessible for any user - $rpcenv->check_any( - $authuser, - "/mapping/notification/$name", - $privs, - ); - } + $rpcenv->check_any( + $authuser, + "/mapping/notification/$name", + $privs, + ); eval { my $config = PVE::Notify::read_config(); @@ -319,12 +290,6 @@ my $sendmail_properties = { type=> 'string', optional=> 1, }, -filter => { - description => 'Name of the filter that should be applied.', - type => 'string', - format => 'pve-configid', - optional => 1, -}, }; __PACKAGE__->register_method ({ @@ -431,7 +396,6 @@ __PACKAGE__->register_method ({ my $from_address = extract_param($param, 'from-address'); my $author = extract_param($param, 'author'); my $comment = extract_param($param, 'comment'); - my $filter = extract_param($param, 'filter'); eval { PVE::Notify::lock_config(sub { @@ -444,7 +408,6 @@ __PACKAGE__->register_method ({ $from_address, $author, $comment, - $filter ); PVE::Notify::write_config($config); @@ -492,7 +455,6 @@ __PACKAGE__->register_method ({ my $from_address = extract_param($param, 'from-address'); my $author = extract_param($param, 'author'); my $comment = extract_param($param, 'comment'); - my $filter = extract_param($param, 'filter'); my $delete = extract_param($param, 'delete'); my $digest = extract_param($param, 'digest'); @@ -508,7 +470,6 @@ __PACKAGE__->register_method ({ $from_address, $author, $comment, - $filter, $delete, $digest, ); @@ -545,11 +506,6 @@ __PACKAGE__->register_method ({ my ($param) = @_; my $name = extract_param($param, 'name'); - my $used_by = target_used_by($name); - if ($used_by) { - raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"}); - } - eval { PVE::Notify::lock_config(sub { my $config = PVE::Notify::read_config(); @@ -582,12 +538,6 @@ my $gotify_properties = { type=> 'string', option
[pve-devel] [PATCH proxmox 03/27] notify: replace filters and groups with matcher-based system
This shifts notification routing into the matcher-system. Every notification has associated metadata (key-value fields, severity - to be extended) that can be match with match directives in notification matchers. Right now, there are 2 matching directives, match-field and match-severity. The first one allows one to do a regex match/exact match on a metadata field, the other one allows one to match one or more severites. Every matcher also allows 'target' directives, these decide which target(s) will be notified if a matcher matches a notification. Since routing now happens in matchers, the API for sending is simplified, since we do not need to specify a target any more. The API routes for filters and groups have been removed completely. The parser for the configuration file will still accept filter/group entries, but will delete them once the config is saved again. This is needed to allow a smooth transition from the old system to the new system, since the old system was already available on pvetest. Signed-off-by: Lukas Wagner --- Notes: Sorry for the large commit, many of these changes interact with each other and it would have been significantly more effort to keep everything nice, tidy and compileable after splitting this apart. I wantend to get these changes out ASAP. proxmox-notify/Cargo.toml| 2 + proxmox-notify/src/api/common.rs | 6 +- proxmox-notify/src/api/filter.rs | 231 - proxmox-notify/src/api/gotify.rs | 16 - proxmox-notify/src/api/group.rs | 259 --- proxmox-notify/src/api/matcher.rs| 254 +++ proxmox-notify/src/api/mod.rs| 115 ++- proxmox-notify/src/api/sendmail.rs | 15 - proxmox-notify/src/config.rs | 34 +- proxmox-notify/src/endpoints/gotify.rs | 19 +- proxmox-notify/src/endpoints/sendmail.rs | 14 +- proxmox-notify/src/filter.rs | 195 +-- proxmox-notify/src/group.rs | 40 +-- proxmox-notify/src/lib.rs| 317 +++--- proxmox-notify/src/matcher.rs| 395 +++ proxmox-notify/src/schema.rs | 11 +- 16 files changed, 848 insertions(+), 1075 deletions(-) delete mode 100644 proxmox-notify/src/api/filter.rs delete mode 100644 proxmox-notify/src/api/group.rs create mode 100644 proxmox-notify/src/api/matcher.rs create mode 100644 proxmox-notify/src/matcher.rs diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index 1541b8b..4812896 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -8,6 +8,7 @@ repository.workspace = true exclude.workspace = true [dependencies] +anyhow.workspace = true handlebars = { workspace = true } lazy_static.workspace = true log.workspace = true @@ -16,6 +17,7 @@ openssl.workspace = true proxmox-http = { workspace = true, features = ["client-sync"], optional = true } proxmox-http-error.workspace = true proxmox-human-byte.workspace = true +proxmox-serde.workspace = true proxmox-schema = { workspace = true, features = ["api-macro", "api-types"]} proxmox-section-config = { workspace = true } proxmox-sys = { workspace = true, optional = true } diff --git a/proxmox-notify/src/api/common.rs b/proxmox-notify/src/api/common.rs index d17f4db..fa2356e 100644 --- a/proxmox-notify/src/api/common.rs +++ b/proxmox-notify/src/api/common.rs @@ -7,7 +7,7 @@ use crate::{Bus, Config, Notification}; /// /// The caller is responsible for any needed permission checks. /// Returns an `anyhow::Error` in case of an error. -pub fn send(config: &Config, channel: &str, notification: &Notification) -> Result<(), HttpError> { +pub fn send(config: &Config, notification: &Notification) -> Result<(), HttpError> { let bus = Bus::from_config(config).map_err(|err| { http_err!( INTERNAL_SERVER_ERROR, @@ -15,7 +15,7 @@ pub fn send(config: &Config, channel: &str, notification: &Notification) -> Resu ) })?; -bus.send(channel, notification); +bus.send(notification); Ok(()) } @@ -50,5 +50,5 @@ pub fn test_target(config: &Config, endpoint: &str) -> Result<(), HttpError> { /// If the entity does not exist, the result will only contain the entity. pub fn get_referenced_entities(config: &Config, entity: &str) -> Result, HttpError> { let entities = super::get_referenced_entities(config, entity); -Ok(Vec::from_iter(entities.into_iter())) +Ok(Vec::from_iter(entities)) } diff --git a/proxmox-notify/src/api/filter.rs b/proxmox-notify/src/api/filter.rs deleted file mode 100644 index b8682f4..000 --- a/proxmox-notify/src/api/filter.rs +++ /dev/null @@ -1,231 +0,0 @@ -use proxmox_http_error::HttpError; - -use crate::api::http_err; -use crate::filter::{DeleteableFilterProperty, FilterConfig, Fil
[pve-devel] [PATCH access-control 1/2] acl: allow more nesting for /mapping acl paths
This will be needed for ACL paths for the notification system, which will get separate namespaces for targets and matchers: /mapping/notification/targets/ as well as /mapping/notification/matchers/ Signed-off-by: Lukas Wagner --- src/PVE/AccessControl.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm index cc0f00b..22aef69 100644 --- a/src/PVE/AccessControl.pm +++ b/src/PVE/AccessControl.pm @@ -1277,6 +1277,7 @@ sub check_path { |/mapping |/mapping/[[:alnum:]\.\-\_]+ |/mapping/[[:alnum:]\.\-\_]+/[[:alnum:]\.\-\_]+ + |/mapping/[[:alnum:]\.\-\_]+/[[:alnum:]\.\-\_]+/[[:alnum:]\.\-\_]+ )$!xs; } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager 2/2] api: notifications: give targets and matchers their own ACL namespace
Right now, matchers and targets share a single namespace due to limitations of the section-config parser. This will probably be fixed some time in the future. As a preparation for that we need to ensure that the ACL tree has separate namespaces for both. Signed-off-by: Lukas Wagner --- This patch requires the pve-manager patches from my notification revamp patch series. PVE/API2/Cluster/Notifications.pm | 55 +++ 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index 8f716f26..6ff6d89e 100644 --- a/PVE/API2/Cluster/Notifications.pm +++ b/PVE/API2/Cluster/Notifications.pm @@ -57,7 +57,7 @@ sub raise_api_error { } sub filter_entities_by_privs { -my ($rpcenv, $entities) = @_; +my ($rpcenv, $prefix, $entities) = @_; my $authuser = $rpcenv->get_user(); my $can_see_mapping_privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit']; @@ -65,7 +65,7 @@ sub filter_entities_by_privs { my $filtered = [grep { $rpcenv->check_any( $authuser, - "/mapping/notification/$_->{name}", + "/mapping/notification/$prefix/$_->{name}", $can_see_mapping_privs, 1 ); @@ -138,8 +138,7 @@ __PACKAGE__->register_method ({ description => 'Returns a list of all entities that can be used as notification targets.', permissions => { description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" - . " 'Mapping.Audit' permissions on '/mapping/notification/'." - . " The special 'mail-to-root' target is available to all users.", + . " 'Mapping.Audit' permissions on '/mapping/notification/targets/'.", user => 'all', }, protected => 1, @@ -199,7 +198,7 @@ __PACKAGE__->register_method ({ raise_api_error($@) if $@; - return filter_entities_by_privs($rpcenv, $targets); + return filter_entities_by_privs($rpcenv, "targets", $targets); } }); @@ -211,7 +210,7 @@ __PACKAGE__->register_method ({ description => 'Send a test notification to a provided target.', permissions => { description => "The user requires 'Mapping.Modify', 'Mapping.Use' or" - . " 'Mapping.Audit' permissions on '/mapping/notification/'." + . " 'Mapping.Audit' permissions on '/mapping/notification/targets/'." . " The special 'mail-to-root' target can be accessed by all users.", user => 'all', }, @@ -236,7 +235,7 @@ __PACKAGE__->register_method ({ $rpcenv->check_any( $authuser, - "/mapping/notification/$name", + "/mapping/notification/targets/$name", $privs, ); @@ -299,7 +298,7 @@ __PACKAGE__->register_method ({ description => 'Returns a list of all sendmail endpoints', permissions => { description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" - . " 'Mapping.Audit' permissions on '/mapping/notification/'.", + . " 'Mapping.Audit' permissions on '/mapping/notification/targets/'.", user => 'all', }, protected => 1, @@ -324,7 +323,7 @@ __PACKAGE__->register_method ({ }; raise_api_error($@) if $@; - return filter_entities_by_privs($rpcenv, $entities); + return filter_entities_by_privs($rpcenv, "targets", $entities); } }); @@ -335,8 +334,8 @@ __PACKAGE__->register_method ({ description => 'Return a specific sendmail endpoint', permissions => { check => ['or', - ['perm', '/mapping/notification/{name}', ['Mapping.Modify']], - ['perm', '/mapping/notification/{name}', ['Mapping.Audit']], + ['perm', '/mapping/notification/targets/{name}', ['Mapping.Modify']], + ['perm', '/mapping/notification/targets/{name}', ['Mapping.Audit']], ], }, protected => 1, @@ -380,7 +379,7 @@ __PACKAGE__->register_method ({ method => 'POST', description => 'Create a new sendmail endpoint', permissions => { - check => ['perm', '/mapping/notification', ['Mapping.Modify']], + check => ['p
[pve-devel] [PATCH docs] notifications: update docs to for matcher-based notifications
Target groups and filters have been replaced by notification matchers. The matcher can match on certain notification properties and route the notification to a target in case of a match. This patch updates the docs to reflect these changes. Signed-off-by: Lukas Wagner --- notifications.adoc | 254 +++-- 1 file changed, 174 insertions(+), 80 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index c4d2931..764ec72 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -5,45 +5,40 @@ ifndef::manvolnum[] :pve-toplevel: endif::manvolnum[] -[[notification_events]] -Notification Events - -{pve} will attempt to notify system administrators in case of certain events, -such as: - -[width="100%",options="header"] -|=== -| Event name| Description | Severity -| `package-updates` | System updates are available| `info` -| `fencing` | The {pve} HA manager has fenced a node | `error` -| `replication` | A storage replication job has failed| `error` -| `vzdump` | vzdump backup finished | `info` (`error` on failure) -|=== - -In the 'Notification' panel of the datacenter view, the system's behavior can be -configured for all events except backup jobs. For backup jobs, -the settings can be found in the respective backup job configuration. -For every notification event there is an option to configure the notification -behavior (*when* to send a notification) and the notification target (*where* to -send the notification). - - -See also: - -* xref:datacenter_configuration_file[Datacenter Configuration] -* xref:datacenter_configuration_file[vzdump] +Overview + + +{pve} will send notifications if case of noteworthy events in the system. + +There are a number of different xref:notification_events[notification events], +each with their own set of metadata fields that can be used in +notification matchers. + +A xref:notification_matchers[notification matcher] determines +_which_ notifications shall be sent _where_. +A matcher has _match rules_, that can be used to +match on certain notification properties (e.g. timestamp, severity, +metadata fields). +If a matcher matches a notification, the notification will be routed +to a configurable set of notification targets. + +A xref:notification_targets[notification target] is an abstraction for a +destination where a notification should be sent to - for instance, +a Gotify server instance, or a set of email addresses. +There are multiple types of notification targets, including +`sendmail`, which uses the system's sendmail command to send emails, +or `gotify`, which sends a notification to a Gotify instance. + +The notification system can be configured in the GUI under +Datacenter -> Notifications. The configuration is stored in +`/etc/pve/notifications.cfg` and `/etc/pve/priv/notifications.cfg` - +the latter contains sensitive configuration options such as +passwords or authentication tokens for notification targets. [[notification_targets]] Notification Targets -Notification targets can be configured in the 'Notification Targets' panel. - -NOTE: The `mail-to-root` target is always available and cannot be modified or -removed. It sends a mail the `root@pam` user by using the `sendmail` command and -serves as a fallback target if no other target is configured for an event. - Sendmail The sendmail binary is a program commonly found on Unix-like operating systems @@ -73,7 +68,15 @@ set, the plugin will fall back to the `email_from` setting from `datacenter.cfg`. If that is also not set, the plugin will default to `root@$hostname`, where `$hostname` is the hostname of the node. -* `filter`: The name of the filter to use for this target. +Example configuration (`/etc/pve/notifications.cfg`): + +sendmail: example +mailto-user root@pam +mailto-user admin@pve +mailto m...@example.com +from-address p...@example.com +comment Send to multiple users/addresses + Gotify ~~ @@ -88,72 +91,163 @@ The configuration for Gotify target plugins has the following options: * `server`: The base URL of the Gotify server, e.g. `http://:` * `token`: The authentication token. Tokens can be generated within the Gotify web interface. -* `filter`: The name of the filter to use for this target. NOTE: The Gotify target plugin will respect the HTTP proxy settings from the xref:datacenter_configuration_file[datacenter configuration] -Group -~ +Example configuration (`/etc/pve/notifications.cfg`): + +gotify: example +server http://gotify.example.com: +comment Send to multiple users/addresses +
[pve-devel] [PATCH v4 debcargo-conf 01/11] cherry-pick chumsky 0.9.2 from debian unstable
Signed-off-by: Lukas Wagner --- src/chumsky/debian/changelog | 5 +++ src/chumsky/debian/copyright | 39 + src/chumsky/debian/copyright.debcargo.hint | 51 ++ src/chumsky/debian/debcargo.toml | 2 + 4 files changed, 97 insertions(+) create mode 100644 src/chumsky/debian/changelog create mode 100644 src/chumsky/debian/copyright create mode 100644 src/chumsky/debian/copyright.debcargo.hint create mode 100644 src/chumsky/debian/debcargo.toml diff --git a/src/chumsky/debian/changelog b/src/chumsky/debian/changelog new file mode 100644 index 0..ae6b5ff8f --- /dev/null +++ b/src/chumsky/debian/changelog @@ -0,0 +1,5 @@ +rust-chumsky (0.9.2-1) unstable; urgency=medium + + * Package chumsky 0.9.2 from crates.io using debcargo 2.6.0 + + -- Jelmer Vernooij Wed, 14 Jun 2023 23:38:48 +0100 diff --git a/src/chumsky/debian/copyright b/src/chumsky/debian/copyright new file mode 100644 index 0..eaa3e6768 --- /dev/null +++ b/src/chumsky/debian/copyright @@ -0,0 +1,39 @@ +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ +Upstream-Name: chumsky +Upstream-Contact: + Joshua Barretto + Elijah Hartvigsen +Source: https://github.com/zesterer/chumsky + +Files: * +Copyright: + 2021-2023 Joshua Barretto + 2021-2023 Elijah Hartvigsen +License: MIT + +Files: debian/* +Copyright: + 2023 Debian Rust Maintainers + 2023 Jelmer Vernooij +License: MIT + +License: MIT + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + . + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + . + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. diff --git a/src/chumsky/debian/copyright.debcargo.hint b/src/chumsky/debian/copyright.debcargo.hint new file mode 100644 index 0..e02a9ab0f --- /dev/null +++ b/src/chumsky/debian/copyright.debcargo.hint @@ -0,0 +1,51 @@ +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ +Upstream-Name: chumsky +Upstream-Contact: + Joshua Barretto + Elijah Hartvigsen +Source: https://github.com/zesterer/chumsky + +Files: * +Copyright: + FIXME (overlay) UNKNOWN-YEARS Joshua Barretto + FIXME (overlay) UNKNOWN-YEARS Elijah Hartvigsen +License: MIT +Comment: + FIXME (overlay): Since upstream copyright years are not available in + Cargo.toml, they were extracted from the upstream Git repository. This may not + be correct information so you should review and fix this before uploading to + the archive. + +Files: LICENSE +Copyright: 2021 Joshua Barretto +License: UNKNOWN-LICENSE; FIXME (overlay) +Comment: + FIXME (overlay): These notices are extracted from files. Please review them + before uploading to the archive. + +Files: debian/* +Copyright: + 2023 Debian Rust Maintainers + 2023 Jelmer Vernooij +License: MIT + +License: MIT + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + . + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + . + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. diff --git a/src/chumsky/debian/debcargo.toml b/src/chumsky/debian/debcargo.toml new file mode 100644 index 0..77e8151ed --- /dev/null +++ b/src/chumsky/debian/debcargo.toml @
[pve-devel] [PATCH v4 many 00/11] notifications: add SMTP endpoint
This patch series adds support for a new notification endpoint type, smtp. As the name suggests, this new endpoint allows PVE to talk to SMTP server directly, without using the system's MTA (postfix). On the Rust side, these patches add a new dependency to the `lettre` crate for SMTP communication. This crate was chosen as it is: - by far the most popular mailing crate for Rust - well maintained - has reasonable dependencies - has async support, enabling us to asyncify the proxmox-notify crate at some point, if needed Tested against: - the gmail SMTP server - the posteo SMTP server - our own webmail SMTP server These patches require a couple of other patches that have not been applied yet: series: "overhaul notification system, use matchers instead of filters" [3], pve-manager: "api: notifications: give targets and matchers their own ACL namespace" [4] pve-docs: "notifications: update docs to for matcher-based notifications" [5] The first two patches were cherry-picked and rebased from the 'system mail forwarding' patch series from [2]. I decided to pull them in so that I can already implement the mail forwarding part for SMTP targets. This series also required updating the 'lettre' crate since one of lettre's deps was bumped to a new version by us. Changes since v3: - Rebased on top of the matcher-based notification revamp - Removed 'filter' setting from target configuration - Pulled in required patches from 'system mail forwarding' patch series Changes since v2: - Rebased proxmox-widget-toolkit onto the latest master to avoid any conflicts. Changes since v1: - Rebased on top of [1] - Added a mechanism for mails forwarded by `proxmox-mail-forward` These are forwarded inline as "message/rfc822" to avoid having to rewrite mail headers (otherwise, some SMTP relays might reject the mail, because the `From` header of the forwarded mail does not match the mail account) [1] https://lists.proxmox.com/pipermail/pve-devel/2023-August/058956.html [2] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059299.html [3] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059818.html [4] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059843.html [5] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059872.html debcargo-conf: Lukas Wagner (2): cherry-pick chumsky 0.9.2 from debian unstable update lettre to 0.11.1 src/chumsky/debian/changelog | 5 ++ src/chumsky/debian/copyright | 39 +++ src/chumsky/debian/copyright.debcargo.hint| 51 ++ src/chumsky/debian/debcargo.toml | 2 + src/lettre/debian/changelog | 10 +++ .../debian/patches/downgrade_fastrand.patch | 13 .../debian/patches/downgrade_idna.patch | 13 src/lettre/debian/patches/downgrade_url.patch | 13 .../patches/remove_unused_features.patch | 69 ++- src/lettre/debian/patches/series | 4 +- .../patches/upgrade_quoted_printable.patch| 13 11 files changed, 185 insertions(+), 47 deletions(-) create mode 100644 src/chumsky/debian/changelog create mode 100644 src/chumsky/debian/copyright create mode 100644 src/chumsky/debian/copyright.debcargo.hint create mode 100644 src/chumsky/debian/debcargo.toml create mode 100644 src/lettre/debian/patches/downgrade_fastrand.patch create mode 100644 src/lettre/debian/patches/downgrade_idna.patch create mode 100644 src/lettre/debian/patches/downgrade_url.patch delete mode 100644 src/lettre/debian/patches/upgrade_quoted_printable.patch proxmox: Lukas Wagner (4): sys: email: add `forward` notify: add mechanisms for email message forwarding notify: add 'smtp' endpoint notify: add api for smtp endpoints Cargo.toml | 2 + proxmox-notify/Cargo.toml | 6 +- proxmox-notify/src/api/mod.rs | 33 ++ proxmox-notify/src/api/smtp.rs | 356 proxmox-notify/src/config.rs| 23 ++ proxmox-notify/src/endpoints/common/mail.rs | 24 ++ proxmox-notify/src/endpoints/common/mod.rs | 2 + proxmox-notify/src/endpoints/gotify.rs | 3 + proxmox-notify/src/endpoints/mod.rs | 4 + proxmox-notify/src/endpoints/sendmail.rs| 27 +- proxmox-notify/src/endpoints/smtp.rs| 250 ++ proxmox-notify/src/lib.rs | 57 proxmox-sys/src/email.rs| 52 ++- 13 files changed, 820 insertions(+), 19 deletions(-) create mode 100644 proxmox-notify/src/api/smtp.rs create mode 100644 proxmox-notify/src/endpoints/common/mail.rs create mode 100644 proxmox-notify/src/endpoints/common/mod.rs create mode 100644 proxmox-notify/src/endpoints/smtp.rs proxmo
[pve-devel] [PATCH v4 pve-docs 11/11] notifications: document 'comment' option for targets/matchers
Signed-off-by: Lukas Wagner --- notifications.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/notifications.adoc b/notifications.adoc index acbdfae..e8ed51b 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -67,6 +67,7 @@ accomodate multiple recipients. set, the plugin will fall back to the `email_from` setting from `datacenter.cfg`. If that is also not set, the plugin will default to `root@$hostname`, where `$hostname` is the hostname of the node. +* `comment`: Comment for this target The `From` header in the email will be set to `$author <$from-address>`. Example configuration (`/etc/pve/notifications.cfg`): @@ -138,6 +139,7 @@ The configuration for Gotify target plugins has the following options: * `server`: The base URL of the Gotify server, e.g. `http://:` * `token`: The authentication token. Tokens can be generated within the Gotify web interface. +* `comment`: Comment for this target NOTE: The Gotify target plugin will respect the HTTP proxy settings from the xref:datacenter_configuration_file[datacenter configuration] @@ -192,6 +194,7 @@ a matcher must be true. Defaults to `all`. * `match-calendar`: Match the notification's timestamp against a schedule * `match-field`: Match the notification's metadata fields * `match-severity`: Match the notification's severity +* `comment`: Comment for this matcher Calendar Matching Rules ~~~ -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 pve-manager 08/11] notify: add API routes for smtp endpoints
The Perl part of the API methods primarily defines the API schema, checks for any needed privileges and then calls the actual Rust implementation exposed via perlmod. Any errors returned by the Rust code are translated into PVE::Exception, so that the API call fails with the correct HTTP error code. Signed-off-by: Lukas Wagner --- PVE/API2/Cluster/Notifications.pm | 323 ++ 1 file changed, 323 insertions(+) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index 6ff6d89e..81a8c5af 100644 --- a/PVE/API2/Cluster/Notifications.pm +++ b/PVE/API2/Cluster/Notifications.pm @@ -193,6 +193,14 @@ __PACKAGE__->register_method ({ }; } + for my $target (@{$config->get_smtp_endpoints()}) { + push @$result, { + name => $target->{name}, + comment => $target->{comment}, + type => 'smtp', + }; + } + $result }; @@ -757,6 +765,321 @@ __PACKAGE__->register_method ({ } }); +my $smtp_properties= { +name => { + description => 'The name of the endpoint.', + type => 'string', + format => 'pve-configid', +}, +server => { + description => 'The address of the SMTP server.', + type => 'string', +}, +port => { + description => 'The port to be used. Defaults to 465 for TLS based connections,' + . ' 587 for STARTTLS based connections and port 25 for insecure plain-text' + . ' connections.', + type => 'integer', + optional => 1, +}, +mode => { + description => 'Determine which encryption method shall be used for the connection.', + type => 'string', + enum => [ qw(insecure starttls tls) ], + default => 'tls', + optional => 1, +}, +username => { + description => 'Username for SMTP authentication', + type => 'string', + optional => 1, +}, +password => { + description => 'Password for SMTP authentication', + type => 'string', + optional => 1, +}, +mailto => { + type => 'array', + items => { + type => 'string', + format => 'email-or-username', + }, + description => 'List of email recipients', + optional => 1, +}, +'mailto-user' => { + type => 'array', + items => { + type => 'string', + format => 'pve-userid', + }, + description => 'List of users', + optional => 1, +}, +'from-address' => { + description => '`From` address for the mail', + type => 'string', +}, +author => { + description => 'Author of the mail. Defaults to \'Proxmox VE\'.', + type => 'string', + optional => 1, +}, +'comment' => { + description => 'Comment', + type=> 'string', + optional=> 1, +}, +}; + +__PACKAGE__->register_method ({ +name => 'get_smtp_endpoints', +path => 'endpoints/smtp', +method => 'GET', +description => 'Returns a list of all smtp endpoints', +permissions => { + description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" + . " 'Mapping.Audit' permissions on '/mapping/notification/targets/'.", + user => 'all', +}, +protected => 1, +parameters => { + additionalProperties => 0, + properties => {}, +}, +returns => { + type => 'array', + items => { + type => 'object', + properties => $smtp_properties, + }, + links => [ { rel => 'child', href => '{name}' } ], +}, +code => sub { + my $config = PVE::Notify::read_config(); + my $rpcenv = PVE::RPCEnvironment::get(); + + my $entities = eval { + $config->get_smtp_endpoints(); + }; + raise_api_error($@) if $@; + + return filter_entities_by_privs($rpcenv, "targets", $entities); +} +}); + +__PACKAGE__->register_method ({ +name => 'get_smtp_endpoint', +path => 'endpoints/smtp/{name}', +method => 'GET', +description => 'Re
[pve-devel] [PATCH v4 proxmox-perl-rs 07/11] notify: add bindings for smtp API calls
Signed-off-by: Lukas Wagner --- common/src/notify.rs | 106 +++ 1 file changed, 106 insertions(+) diff --git a/common/src/notify.rs b/common/src/notify.rs index 4fbd705..8a6d76e 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@ -15,6 +15,10 @@ mod export { use proxmox_notify::endpoints::sendmail::{ DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater, }; +use proxmox_notify::endpoints::smtp::{ +DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig, +SmtpPrivateConfigUpdater, +}; use proxmox_notify::matcher::{ CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig, MatcherConfigUpdater, SeverityMatcher, @@ -271,6 +275,108 @@ mod export { api::gotify::delete_gotify_endpoint(&mut config, name) } +#[export(serialize_error)] +fn get_smtp_endpoints( +#[try_from_ref] this: &NotificationConfig, +) -> Result, HttpError> { +let config = this.config.lock().unwrap(); +api::smtp::get_endpoints(&config) +} + +#[export(serialize_error)] +fn get_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +id: &str, +) -> Result { +let config = this.config.lock().unwrap(); +api::smtp::get_endpoint(&config, id) +} + +#[export(serialize_error)] +#[allow(clippy::too_many_arguments)] +fn add_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +name: String, +server: String, +port: Option, +mode: Option, +username: Option, +password: Option, +mailto: Option>, +mailto_user: Option>, +from_address: String, +author: Option, +comment: Option, +) -> Result<(), HttpError> { +let mut config = this.config.lock().unwrap(); +api::smtp::add_endpoint( +&mut config, +&SmtpConfig { +name: name.clone(), +server, +port, +mode, +username, +mailto, +mailto_user, +from_address, +author, +comment, +}, +&SmtpPrivateConfig { name, password }, +) +} + +#[export(serialize_error)] +#[allow(clippy::too_many_arguments)] +fn update_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +name: &str, +server: Option, +port: Option, +mode: Option, +username: Option, +password: Option, +mailto: Option>, +mailto_user: Option>, +from_address: Option, +author: Option, +comment: Option, +delete: Option>, +digest: Option<&str>, +) -> Result<(), HttpError> { +let mut config = this.config.lock().unwrap(); +let digest = decode_digest(digest)?; + +api::smtp::update_endpoint( +&mut config, +name, +&SmtpConfigUpdater { +server, +port, +mode, +username, +mailto, +mailto_user, +from_address, +author, +comment, +}, +&SmtpPrivateConfigUpdater { password }, +delete.as_deref(), +digest.as_deref(), +) +} + +#[export(serialize_error)] +fn delete_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +name: &str, +) -> Result<(), HttpError> { +let mut config = this.config.lock().unwrap(); +api::smtp::delete_endpoint(&mut config, name) +} + #[export(serialize_error)] fn get_matchers( #[try_from_ref] this: &NotificationConfig, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 proxmox 06/11] notify: add api for smtp endpoints
Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/mod.rs| 33 +++ proxmox-notify/src/api/smtp.rs | 356 +++ proxmox-notify/src/endpoints/smtp.rs | 8 - 3 files changed, 389 insertions(+), 8 deletions(-) create mode 100644 proxmox-notify/src/api/smtp.rs diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs index 8042157..762d448 100644 --- a/proxmox-notify/src/api/mod.rs +++ b/proxmox-notify/src/api/mod.rs @@ -1,3 +1,4 @@ +use serde::Serialize; use std::collections::HashSet; use proxmox_http_error::HttpError; @@ -10,6 +11,8 @@ pub mod gotify; pub mod matcher; #[cfg(feature = "sendmail")] pub mod sendmail; +#[cfg(feature = "smtp")] +pub mod smtp; // We have our own, local versions of http_err and http_bail, because // we don't want to wrap the error in anyhow::Error. If we were to do that, @@ -60,6 +63,10 @@ fn ensure_endpoint_exists(#[allow(unused)] config: &Config, name: &str) -> Resul { exists = exists || gotify::get_endpoint(config, name).is_ok(); } +#[cfg(feature = "smtp")] +{ +exists = exists || smtp::get_endpoint(config, name).is_ok(); +} if !exists { http_bail!(NOT_FOUND, "endpoint '{name}' does not exist") @@ -100,6 +107,7 @@ fn get_referrers(config: &Config, entity: &str) -> Result, HttpE } } } + Ok(referrers) } @@ -148,6 +156,31 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet { expanded } +#[allow(unused)] +fn set_private_config_entry( +config: &mut Config, +private_config: &T, +typename: &str, +name: &str, +) -> Result<(), HttpError> { +config +.private_config +.set_data(name, typename, private_config) +.map_err(|e| { +http_err!( +INTERNAL_SERVER_ERROR, +"could not save private config for endpoint '{}': {e}", +name +) +}) +} + +#[allow(unused)] +fn remove_private_config_entry(config: &mut Config, name: &str) -> Result<(), HttpError> { +config.private_config.sections.remove(name); +Ok(()) +} + #[cfg(test)] mod test_helpers { use crate::Config; diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs new file mode 100644 index 000..bd9d7bb --- /dev/null +++ b/proxmox-notify/src/api/smtp.rs @@ -0,0 +1,356 @@ +use proxmox_http_error::HttpError; + +use crate::api::{http_bail, http_err}; +use crate::endpoints::smtp::{ +DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig, +SmtpPrivateConfigUpdater, SMTP_TYPENAME, +}; +use crate::Config; + +/// Get a list of all smtp endpoints. +/// +/// The caller is responsible for any needed permission checks. +/// Returns a list of all smtp endpoints or a `HttpError` if the config is +/// erroneous (`500 Internal server error`). +pub fn get_endpoints(config: &Config) -> Result, HttpError> { +config +.config +.convert_to_typed_array(SMTP_TYPENAME) +.map_err(|e| http_err!(NOT_FOUND, "Could not fetch endpoints: {e}")) +} + +/// Get smtp endpoint with given `name`. +/// +/// The caller is responsible for any needed permission checks. +/// Returns the endpoint or a `HttpError` if the endpoint was not found (`404 Not found`). +pub fn get_endpoint(config: &Config, name: &str) -> Result { +config +.config +.lookup(SMTP_TYPENAME, name) +.map_err(|_| http_err!(NOT_FOUND, "endpoint '{name}' not found")) +} + +/// Add a new smtp endpoint. +/// +/// The caller is responsible for any needed permission checks. +/// The caller also responsible for locking the configuration files. +/// Returns a `HttpError` if: +/// - an entity with the same name already exists (`400 Bad request`) +/// - the configuration could not be saved (`500 Internal server error`) +/// - mailto *and* mailto_user are both set to `None` +pub fn add_endpoint( +config: &mut Config, +endpoint_config: &SmtpConfig, +private_endpoint_config: &SmtpPrivateConfig, +) -> Result<(), HttpError> { +if endpoint_config.name != private_endpoint_config.name { +// Programming error by the user of the crate, thus we panic +panic!("name for endpoint config and private config must be identical"); +} + +super::ensure_unique(config, &endpoint_config.name)?; + +if endpoint_config.mailto.is_none() && endpoint_config.mailto_user.is_none() { +http_bail!( +BAD_REQUEST, +"must at least provide one recipient, either in mailto or in mailto-user" +); +} + +super::set_private_config_entry( +config, +private_endpoint_config, +
[pve-devel] [PATCH v4 proxmox 03/11] sys: email: add `forward`
This new function forwards an email to new recipients. Signed-off-by: Lukas Wagner --- proxmox-sys/src/email.rs | 52 +++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/proxmox-sys/src/email.rs b/proxmox-sys/src/email.rs index 8b3a1b6..c94f634 100644 --- a/proxmox-sys/src/email.rs +++ b/proxmox-sys/src/email.rs @@ -3,7 +3,7 @@ use std::io::Write; use std::process::{Command, Stdio}; -use anyhow::{bail, Error}; +use anyhow::{bail, format_err, Error}; /// Sends multi-part mail with text and/or html to a list of recipients /// @@ -110,6 +110,56 @@ pub fn sendmail( Ok(()) } +/// Forwards an email message to a given list of recipients. +/// +/// ``sendmail`` is used for sending the mail, thus `message` must be +/// compatible with that (the message is piped into stdin unmodified). +pub fn forward( +mailto: &[&str], +mailfrom: &str, +message: &[u8], +uid: Option, +) -> Result<(), Error> { +use std::os::unix::process::CommandExt; + +if mailto.is_empty() { +bail!("At least one recipient has to be specified!") +} + +let mut builder = Command::new("/usr/sbin/sendmail"); + +builder +.args([ +"-N", "never", // never send DSN (avoid mail loops) +"-f", mailfrom, "--", +]) +.args(mailto) +.stdin(Stdio::piped()) +.stdout(Stdio::null()) +.stderr(Stdio::null()); + +if let Some(uid) = uid { +builder.uid(uid); +} + +let mut process = builder +.spawn() +.map_err(|err| format_err!("could not spawn sendmail process: {err}"))?; + +process +.stdin +.take() +.unwrap() +.write_all(message) +.map_err(|err| format_err!("couldn't write to sendmail stdin: {err}"))?; + +process +.wait() +.map_err(|err| format_err!("sendmail did not exit successfully: {err}"))?; + +Ok(()) +} + #[cfg(test)] mod test { use crate::email::sendmail; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 proxmox 04/11] notify: add mechanisms for email message forwarding
As preparation for the integration of `proxmox-mail-foward` into the notification system, this commit makes a few changes that allow us to forward raw email messages (as passed from postfix). For mail-based notification targets, the email will be forwarded as-is, including all headers. The only thing that changes is the message envelope. For other notification targets, the mail is parsed using the `mail-parser` crate, which allows us to extract a subject and a body. As a body we use the plain-text version of the mail. If an email is HTML-only, the `mail-parser` crate will automatically attempt to transform the HTML into readable plain text. Signed-off-by: Lukas Wagner --- Cargo.toml | 1 + proxmox-notify/Cargo.toml| 2 ++ proxmox-notify/src/endpoints/gotify.rs | 3 ++ proxmox-notify/src/endpoints/sendmail.rs | 5 +++ proxmox-notify/src/lib.rs| 41 5 files changed, 52 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index f8bc181..3d81d85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ lazy_static = "1.4" ldap3 = { version = "0.11", default-features = false } libc = "0.2.107" log = "0.4.17" +mail-parser = "0.8.2" native-tls = "0.2" nix = "0.26.1" once_cell = "1.3.1" diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index 4812896..f2b4db5 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -12,6 +12,7 @@ anyhow.workspace = true handlebars = { workspace = true } lazy_static.workspace = true log.workspace = true +mail-parser = { workspace = true, optional = true } once_cell.workspace = true openssl.workspace = true proxmox-http = { workspace = true, features = ["client-sync"], optional = true } @@ -28,5 +29,6 @@ serde_json.workspace = true [features] default = ["sendmail", "gotify"] +mail-forwarder = ["dep:mail-parser"] sendmail = ["dep:proxmox-sys"] gotify = ["dep:proxmox-http"] diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs index 1c307a4..5713d99 100644 --- a/proxmox-notify/src/endpoints/gotify.rs +++ b/proxmox-notify/src/endpoints/gotify.rs @@ -19,6 +19,7 @@ fn severity_to_priority(level: Severity) -> u32 { Severity::Notice => 3, Severity::Warning => 5, Severity::Error => 9, +Severity::Unknown => 3, } } @@ -94,6 +95,8 @@ impl Endpoint for GotifyEndpoint { (rendered_title, rendered_message) } +#[cfg(feature = "mail-forwarder")] +Content::ForwardedMail { title, body, .. } => (title.clone(), body.clone()), }; // We don't have a TemplateRenderer::Markdown yet, so simply put everything diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs index a601744..3ef33b6 100644 --- a/proxmox-notify/src/endpoints/sendmail.rs +++ b/proxmox-notify/src/endpoints/sendmail.rs @@ -134,6 +134,11 @@ impl Endpoint for SendmailEndpoint { ) .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into())) } +#[cfg(feature = "mail-forwarder")] +Content::ForwardedMail { raw, uid, .. } => { +proxmox_sys::email::forward(&recipients_str, &mailfrom, raw, *uid) +.map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into())) +} } } diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 9997cef..ada1b0a 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -102,6 +102,8 @@ pub enum Severity { Warning, /// Error Error, +/// Unknown severity (e.g. forwarded system mails) +Unknown, } impl Display for Severity { @@ -111,6 +113,7 @@ impl Display for Severity { Severity::Notice => f.write_str("notice"), Severity::Warning => f.write_str("warning"), Severity::Error => f.write_str("error"), +Severity::Unknown => f.write_str("unknown"), } } } @@ -123,6 +126,7 @@ impl FromStr for Severity { "notice" => Ok(Self::Notice), "warning" => Ok(Self::Warning), "error" => Ok(Self::Error), +"unknown" => Ok(Self::Unknown), _ => Err(Error::Generic(format!("invalid severity {s}"))), } } @@ -148,6 +152,18 @@ pub enum Content { /// Data that can be used for template rendering. data: Value, }, +#[cfg(feature = "mail-forwarder")] +ForwardedMail { +/// Raw mail contents +
[pve-devel] [PATCH v4 debcargo-conf 02/11] update lettre to 0.11.1
Signed-off-by: Lukas Wagner --- src/lettre/debian/changelog | 10 +++ .../debian/patches/downgrade_fastrand.patch | 13 .../debian/patches/downgrade_idna.patch | 13 src/lettre/debian/patches/downgrade_url.patch | 13 .../patches/remove_unused_features.patch | 69 ++- src/lettre/debian/patches/series | 4 +- .../patches/upgrade_quoted_printable.patch| 13 7 files changed, 88 insertions(+), 47 deletions(-) create mode 100644 src/lettre/debian/patches/downgrade_fastrand.patch create mode 100644 src/lettre/debian/patches/downgrade_idna.patch create mode 100644 src/lettre/debian/patches/downgrade_url.patch delete mode 100644 src/lettre/debian/patches/upgrade_quoted_printable.patch diff --git a/src/lettre/debian/changelog b/src/lettre/debian/changelog index d49cbb042..e92c5c070 100644 --- a/src/lettre/debian/changelog +++ b/src/lettre/debian/changelog @@ -1,3 +1,13 @@ +rust-lettre (0.11.1-1) UNRELEASED-FIXME-AUTOGENERATED-DEBCARGO; urgency=medium + + * Package lettre 0.11.1 from crates.io using debcargo 2.6.0 + * Downgrade fastrand from 2.0 to 1.8 + * Downgrade idna from 0.4 to 0.3 + * Downgrade url from 2.4 to 2.3 + * Drop patch that upgrades quoted_printable + + -- Lukas Wagner Wed, 08 Nov 2023 13:32:49 +0100 + rust-lettre (0.10.4-1~bpo12+pve1) proxmox-rust; urgency=medium * Rebuild for Debian Bookworm / Proxmox diff --git a/src/lettre/debian/patches/downgrade_fastrand.patch b/src/lettre/debian/patches/downgrade_fastrand.patch new file mode 100644 index 0..975efeb1c --- /dev/null +++ b/src/lettre/debian/patches/downgrade_fastrand.patch @@ -0,0 +1,13 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 072ea3a..5decb37 100644 +--- a/Cargo.toml b/Cargo.toml +@@ -150,7 +150,7 @@ version = "0.2.1" + default-features = false + + [dependencies.fastrand] +-version = "2.0" ++version = "1.8" + optional = true + + [dependencies.futures-io] diff --git a/src/lettre/debian/patches/downgrade_idna.patch b/src/lettre/debian/patches/downgrade_idna.patch new file mode 100644 index 0..1cfaaa26c --- /dev/null +++ b/src/lettre/debian/patches/downgrade_idna.patch @@ -0,0 +1,13 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 5decb37..09d2b9b 100644 +--- a/Cargo.toml b/Cargo.toml +@@ -176,7 +176,7 @@ version = "1" + optional = true + + [dependencies.idna] +-version = "0.4" ++version = "0.3" + + [dependencies.mime] + version = "0.3.4" diff --git a/src/lettre/debian/patches/downgrade_url.patch b/src/lettre/debian/patches/downgrade_url.patch new file mode 100644 index 0..4da907540 --- /dev/null +++ b/src/lettre/debian/patches/downgrade_url.patch @@ -0,0 +1,13 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 09d2b9b..5004a3b 100644 +--- a/Cargo.toml b/Cargo.toml +@@ -237,7 +237,7 @@ optional = true + default-features = false + + [dependencies.url] +-version = "2.4" ++version = "2.3" + optional = true + + [dependencies.uuid] diff --git a/src/lettre/debian/patches/remove_unused_features.patch b/src/lettre/debian/patches/remove_unused_features.patch index 0229e41aa..7ce45be0f 100644 --- a/src/lettre/debian/patches/remove_unused_features.patch +++ b/src/lettre/debian/patches/remove_unused_features.patch @@ -1,8 +1,8 @@ diff --git a/Cargo.toml b/Cargo.toml -index 13c34b6..b4413b6 100644 +index 13e3b77..072ea3a 100644 --- a/Cargo.toml +++ b/Cargo.toml -@@ -114,32 +114,10 @@ required-features = [ +@@ -114,24 +114,6 @@ required-features = [ "builder", ] @@ -27,6 +27,9 @@ index 13c34b6..b4413b6 100644 [[bench]] name = "transport_smtp" harness = false +@@ -140,10 +122,6 @@ harness = false + name = "mailbox_parsing" + harness = false -[dependencies.async-std] -version = "1.8" @@ -35,8 +38,8 @@ index 13c34b6..b4413b6 100644 [dependencies.async-trait] version = "0.1" optional = true -@@ -217,19 +195,6 @@ optional = true - version = "0.8" +@@ -224,19 +202,6 @@ optional = true + version = "0.9" optional = true -[dependencies.rustls] @@ -55,19 +58,19 @@ index 13c34b6..b4413b6 100644 [dependencies.serde] version = "1" features = ["derive"] -@@ -248,11 +213,6 @@ optional = true - version = "0.4.4" +@@ -255,11 +220,6 @@ optional = true + version = "0.5.1" optional = true -[dependencies.tokio1_boring] --version = "2.1.4" +-version = "3" -optional = true -package = "tokio-boring" - [dependencies.tokio1_crate] version = "1" optional = true -@@ -263,11 +223,6 @@ version = "0.3" +@@ -270,11 +230,6 @@ version = "0.3" optional = true package = "tokio-native-tls" @@ -79,8 +82,8 @@ index 13c34b6..b4413b6 100644 [dependencies.tracing] version = "0.1.16" features = [&qu
[pve-devel] [PATCH v4 pve-docs 10/11] notifications: document SMTP endpoints
Signed-off-by: Lukas Wagner --- notifications.adoc | 47 ++ 1 file changed, 47 insertions(+) diff --git a/notifications.adoc b/notifications.adoc index 764ec72..acbdfae 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -67,6 +67,7 @@ accomodate multiple recipients. set, the plugin will fall back to the `email_from` setting from `datacenter.cfg`. If that is also not set, the plugin will default to `root@$hostname`, where `$hostname` is the hostname of the node. +The `From` header in the email will be set to `$author <$from-address>`. Example configuration (`/etc/pve/notifications.cfg`): @@ -78,6 +79,52 @@ sendmail: example comment Send to multiple users/addresses +SMTP + + +SMTP notification targets can send emails directly to an SMTP mail relay. + +The configuration for SMTP target plugins has the following options: + +* `mailto`: E-Mail address to which the notification shall be sent to. Can be +set multiple times to accomodate multiple recipients. +* `mailto-user`: Users to which emails shall be sent to. The user's email +address will be looked up in `users.cfg`. Can be set multiple times to +accomodate multiple recipients. +* `author`: Sets the author of the E-Mail. Defaults to `Proxmox VE`. +* `from-address`: Sets the From-addresss of the email. SMTP relays might require +that this address is owned by the user in order to avoid spoofing. +The `From` header in the email will be set to `$author <$from-address>`. +* `username`: Username to use during authentication. If no username is set, +no authentication will be performed. The PLAIN and LOGIN authentication methods +are supported. +* `password`: Password to use when authenticating. +* `mode`: Sets the encryption mode (`insecure`, `starttls` or `tls`). Defaults +to `tls`. +* `server`: Address/IP of the SMTP relay +* `port`: The port to connect to. If not set, the used port +defaults to 25 (`insecure`), 465 (`tls`) or 587 (`starttls`), depending on the +value of `mode`. +* `comment`: Comment for this target + +Example configuration (`/etc/pve/notifications.cfg`): + +smtp: example +mailto-user root@pam +mailto-user admin@pve +mailto m...@example.com +from-address p...@example.com +username pve1 +server mail.example.com +mode starttls + +The matching entry in `/etc/pve/priv/notifications.cfg`, containing the +secret token: + +smtp: example +password somepassword + + Gotify ~~ -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 proxmox-widget-toolkit 09/11] panel: notification: add gui for SMTP endpoints
This new endpoint configuration panel is embedded in the existing EndpointEditBase dialog window. This commit also factors out some of the non-trivial common form elements that are shared between the new panel and the already existing SendmailEditPanel into a separate panel EmailRecipientPanel. Signed-off-by: Lukas Wagner --- src/Makefile | 2 + src/Schema.js| 5 + src/panel/EmailRecipientPanel.js | 88 +++ src/panel/SendmailEditPanel.js | 58 +- src/panel/SmtpEditPanel.js | 183 +++ 5 files changed, 281 insertions(+), 55 deletions(-) create mode 100644 src/panel/EmailRecipientPanel.js create mode 100644 src/panel/SmtpEditPanel.js diff --git a/src/Makefile b/src/Makefile index c6d31c3..01145b1 100644 --- a/src/Makefile +++ b/src/Makefile @@ -71,7 +71,9 @@ JSSRC=\ panel/ACMEAccount.js\ panel/ACMEPlugin.js \ panel/ACMEDomains.js\ + panel/EmailRecipientPanel.js\ panel/SendmailEditPanel.js \ + panel/SmtpEditPanel.js \ panel/StatusView.js \ panel/TfaView.js\ panel/NotesView.js \ diff --git a/src/Schema.js b/src/Schema.js index 37ecd88..28e1037 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -43,6 +43,11 @@ Ext.define('Proxmox.Schema', { // a singleton ipanel: 'pmxSendmailEditPanel', iconCls: 'fa-envelope-o', }, + smtp: { + name: gettext('SMTP'), + ipanel: 'pmxSmtpEditPanel', + iconCls: 'fa-envelope-o', + }, gotify: { name: gettext('Gotify'), ipanel: 'pmxGotifyEditPanel', diff --git a/src/panel/EmailRecipientPanel.js b/src/panel/EmailRecipientPanel.js new file mode 100644 index 000..b2bc03c --- /dev/null +++ b/src/panel/EmailRecipientPanel.js @@ -0,0 +1,88 @@ +Ext.define('Proxmox.panel.EmailRecipientPanel', { +extend: 'Ext.panel.Panel', +xtype: 'pmxEmailRecipientPanel', +mixins: ['Proxmox.Mixin.CBind'], +border: false, + +mailValidator: function() { + let mailto_user = this.down(`[name=mailto-user]`); + let mailto = this.down(`[name=mailto]`); + + if (!mailto_user.getValue()?.length && !mailto.getValue()) { + return gettext('Either mailto or mailto-user must be set'); + } + + return true; +}, + +items: [ + { + layout: 'anchor', + border: false, + items: [ + { + xtype: 'pmxUserSelector', + name: 'mailto-user', + multiSelect: true, + allowBlank: true, + editable: false, + skipEmptyText: true, + fieldLabel: gettext('Recipient(s)'), + cbind: { + deleteEmpty: '{!isCreate}', + }, + validator: function() { + return this.up('pmxEmailRecipientPanel').mailValidator(); + }, + autoEl: { + tag: 'div', + 'data-qtip': gettext('The notification will be sent to the user\'s configured mail address'), + }, + listConfig: { + width: 600, + columns: [ + { + header: gettext('User'), + sortable: true, + dataIndex: 'userid', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + { + header: gettext('E-Mail'), + sortable: true, + dataIndex: 'email', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + { + header: gettext('Comment'), + sortable: false, + dataIndex: 'comment', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + ], + }, + }, + { + xtype: 'proxmoxtextfield', + fieldLabel: gettext('Additional Recip
[pve-devel] [PATCH v4 proxmox 05/11] notify: add 'smtp' endpoint
This commit adds a new endpoint type, namely 'smtp'. This endpoint uses the `lettre` crate to directly send emails to SMTP relays. The `lettre` crate was chosen since it is by far the most popular SMTP implementation for Rust that looks like it is well maintained. Also, it includes async support (for when we want to extend proxmox-notify to be async). For this new endpoint type, a new section-config type was introduced (smtp). It has the same fields as the type for `sendmail`, with the addition of some new options (smtp server, authentication, tls mode, etc.). Some of the behavior that is shared between sendmail and smtp endpoints has been moved to a new `endpoints::common::mail` module. Signed-off-by: Lukas Wagner --- Cargo.toml | 1 + proxmox-notify/Cargo.toml | 4 +- proxmox-notify/src/config.rs| 23 ++ proxmox-notify/src/endpoints/common/mail.rs | 24 ++ proxmox-notify/src/endpoints/common/mod.rs | 2 + proxmox-notify/src/endpoints/mod.rs | 4 + proxmox-notify/src/endpoints/sendmail.rs| 22 +- proxmox-notify/src/endpoints/smtp.rs| 258 proxmox-notify/src/lib.rs | 16 ++ 9 files changed, 336 insertions(+), 18 deletions(-) create mode 100644 proxmox-notify/src/endpoints/common/mail.rs create mode 100644 proxmox-notify/src/endpoints/common/mod.rs create mode 100644 proxmox-notify/src/endpoints/smtp.rs diff --git a/Cargo.toml b/Cargo.toml index 3d81d85..9f247be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ http = "0.2" hyper = "0.14.5" lazy_static = "1.4" ldap3 = { version = "0.11", default-features = false } +lettre = "0.11.1" libc = "0.2.107" log = "0.4.17" mail-parser = "0.8.2" diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index f2b4db5..8741b9b 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -11,6 +11,7 @@ exclude.workspace = true anyhow.workspace = true handlebars = { workspace = true } lazy_static.workspace = true +lettre = { workspace = true, optional = true } log.workspace = true mail-parser = { workspace = true, optional = true } once_cell.workspace = true @@ -28,7 +29,8 @@ serde = { workspace = true, features = ["derive"]} serde_json.workspace = true [features] -default = ["sendmail", "gotify"] +default = ["sendmail", "gotify", "smtp"] mail-forwarder = ["dep:mail-parser"] sendmail = ["dep:proxmox-sys"] gotify = ["dep:proxmox-http"] +smtp = ["dep:lettre"] diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs index a86995e..fe25ea7 100644 --- a/proxmox-notify/src/config.rs +++ b/proxmox-notify/src/config.rs @@ -28,6 +28,17 @@ fn config_init() -> SectionConfig { SENDMAIL_SCHEMA, )); } +#[cfg(feature = "smtp")] +{ +use crate::endpoints::smtp::{SmtpConfig, SMTP_TYPENAME}; + +const SMTP_SCHEMA: &ObjectSchema = SmtpConfig::API_SCHEMA.unwrap_object_schema(); +config.register_plugin(SectionConfigPlugin::new( +SMTP_TYPENAME.to_string(), +Some(String::from("name")), +SMTP_SCHEMA, +)); +} #[cfg(feature = "gotify")] { use crate::endpoints::gotify::{GotifyConfig, GOTIFY_TYPENAME}; @@ -80,6 +91,18 @@ fn private_config_init() -> SectionConfig { )); } +#[cfg(feature = "smtp")] +{ +use crate::endpoints::smtp::{SmtpPrivateConfig, SMTP_TYPENAME}; + +const SMTP_SCHEMA: &ObjectSchema = SmtpPrivateConfig::API_SCHEMA.unwrap_object_schema(); +config.register_plugin(SectionConfigPlugin::new( +SMTP_TYPENAME.to_string(), +Some(String::from("name")), +SMTP_SCHEMA, +)); +} + config } diff --git a/proxmox-notify/src/endpoints/common/mail.rs b/proxmox-notify/src/endpoints/common/mail.rs new file mode 100644 index 000..0929d7c --- /dev/null +++ b/proxmox-notify/src/endpoints/common/mail.rs @@ -0,0 +1,24 @@ +use std::collections::HashSet; + +use crate::context; + +pub(crate) fn get_recipients( +email_addrs: Option<&[String]>, +users: Option<&[String]>, +) -> HashSet { +let mut recipients = HashSet::new(); + +if let Some(mailto_addrs) = email_addrs { +for addr in mailto_addrs { +recipients.insert(addr.clone()); +} +} +if let Some(users) = users { +for user in users { +if let Some(addr) = context::context().lookup_email_for_user(user) { +recipients.insert(addr); +} +} +} +recipients +} diff --git a/proxmox-notify/src/endpoints/common/mod.rs b/proxmox-notify/src/endp
Re: [pve-devel] [PATCH v4 many 00/11] notifications: add SMTP endpoint
On 11/8/23 16:52, Dietmar Maurer wrote: This patch series adds support for a new notification endpoint type, smtp. As the name suggests, this new endpoint allows PVE to talk to SMTP server directly, without using the system's MTA (postfix). Isn't this totally unreliable? What if the server responds with a temporary error code? (An MTA retries several times). The notification system has no mechanism yet for queuing/retries, so yes, at the moment a SMTP endpoint would indeed be less reliable than a 'sendmail' endpoint. I'm not sure though if I would call it 'totally unreliable'. The same thing applies for gotify/webhook endpoints - if the network or Gotify server is down, a notification cannot be sent. A queuing/retry mechanism could be added at some point, but this would require some bigger changes, as the notification system is completely stateless right now. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v4 many 00/11] notifications: add SMTP endpoint
On 11/9/23 13:16, Dietmar Maurer wrote: On 11/8/23 16:52, Dietmar Maurer wrote: This patch series adds support for a new notification endpoint type, smtp. As the name suggests, this new endpoint allows PVE to talk to SMTP server directly, without using the system's MTA (postfix). Isn't this totally unreliable? What if the server responds with a temporary error code? (An MTA retries several times). The notification system has no mechanism yet for queuing/retries, so yes, at the moment a SMTP endpoint would indeed be less reliable than a 'sendmail' endpoint. I'm not sure though if I would call it 'totally unreliable'. This kind of notification system is quite popular for (PHP) web-sites contact form. I have seen many over-simplified implementation overs the years, and yes, it is totally unreliable. That is why we always used an MTA to deliver mails... I see. What would be your suggestion? To not have such a plugin at all? I implemented this because it was explicitly mentioned by Thomas in the tracking bugzilla issue for an overhauled notification system [1]. Not having to configure Postfix if one wants to use an external SMTP relay seems to be add quite a lot of value to the user (e.g. judging from [2] and [3]) As a compromise, maybe we could just add a note to the docs that discusses the reliability aspects of 'sendmail' vs 'smtp' endpoints? [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4156 [2] https://bugzilla.proxmox.com/show_bug.cgi?id=2965 [3] https://forum.proxmox.com/threads/get-postfix-to-send-notifications-email-externally.59940/ -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] debian: postinst: copy notifications.cfg from /usr/share/pve-manager
... instead of using a heredoc in postinst script. Signed-off-by: Lukas Wagner --- Requires "debian: postinst: create notifications.cfg if it does not exist "[1] from the "overhaul notification system, use matchers instead of filters" patch series [1] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059824.html configs/Makefile | 1 + configs/notifications.cfg | 7 +++ debian/postinst | 17 + 3 files changed, 9 insertions(+), 16 deletions(-) create mode 100644 configs/notifications.cfg diff --git a/configs/Makefile b/configs/Makefile index fd446b5b..575c48b9 100644 --- a/configs/Makefile +++ b/configs/Makefile @@ -13,6 +13,7 @@ install: country.dat vzdump.conf pve-sources.list pve-initramfs.conf pve-blackli install -D -m 0644 vzdump.conf $(DESTDIR)/etc/vzdump.conf install -D -m 0644 pve-initramfs.conf $(DESTDIR)/etc/initramfs-tools/conf.d/pve-initramfs.conf install -D -m 0644 country.dat $(DESTDIR)/usr/share/$(PACKAGE)/country.dat + install -D -m 0644 notifications.cfg $(DESTDIR)/usr/share/$(PACKAGE)/notifications.cfg clean: rm -f country.dat diff --git a/configs/notifications.cfg b/configs/notifications.cfg new file mode 100644 index ..57c496c5 --- /dev/null +++ b/configs/notifications.cfg @@ -0,0 +1,7 @@ +sendmail: default-target +mailto-user root@pam +comment Send mails to root@pam's email address + +matcher: default-matcher +target default-target +comment Send all notifications to 'default-target' diff --git a/debian/postinst b/debian/postinst index 7dad2b1a..3f941486 100755 --- a/debian/postinst +++ b/debian/postinst @@ -93,27 +93,12 @@ migrate_apt_auth_conf() { fi } -write_notification_cfg() { -# Create default config: -# A sendmail-target that sends to root@pam, and a -# matcher that sends all notifications to this target -cat >> /etc/pve/notifications.cfg <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH access-control 1/2] acl: allow more nesting for /mapping acl paths
On 11/10/23 09:18, Thomas Lamprecht wrote: Am 07/11/2023 um 13:46 schrieb Lukas Wagner: This will be needed for ACL paths for the notification system, which will get separate namespaces for targets and matchers: /mapping/notification/targets/ as well as /mapping/notification/matchers/ Not that it matters much to this supporting patch, but above should all use the singular, or? I.e., like "notification" also use "target" and "matcher". Yeah, I also was kind of unsure about that, but in the end I used the plural form because that's what I use for the API routes. e.g. /cluster/notifications/targets /cluster/notifications/matchers However, now I see another discrepancy I missed before, the API route also uses 'notifications' in its plural form. So maybe it would make sense to have the ACL tree nodes match that exactlty? E.g. /mapping/notifications/targets I don't have any strong preference for any form, I just think that some consistency with the API would be nice - and changing the API routes would be much more work ;) And regarding the granularity: Yes, maybe that's a bit overkill now. The per-target permissions were kind of important with the 'old' system where we would select a target at the notification call site (e.g. a backup job), but with the new 'pub-sub'-alike system it probably does not matter that much any more. But I don't really have any strong preference here as well. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-docs 5/5] notifications: add documentation for system mail forwarding
Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - Dropped paragraph about target/policy, since we now do routing in matchers notifications.adoc | 16 1 file changed, 16 insertions(+) diff --git a/notifications.adoc b/notifications.adoc index 764ec72..f0eee9d 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -231,6 +231,7 @@ Notification Events | Cluster node fenced |`fencing` | `error` | `hostname` | Storage replication failed |`replication` | `error` | - | Backup finished |`vzdump` | `info` (`error` on failure) | `hostname` +| Mail for root|`system-mail` | `unknown`| - |=== [width="100%",options="header"] @@ -240,6 +241,21 @@ Notification Events | `hostname` | Hostname, including domain (e.g. `pve1.example.com`) |=== +System Mail Forwarding +- + +Certain local system daemons, such as `smartd`, generate notification emails +that are initially directed to the local `root` user. {pve} will +feed these mails into the notification system as a notification of +type `system-mail` and with severity `unknown`. + +When the forwarding process involves an email-based target +(like `sendmail` or `smtp`), the email is forwarded exactly as received, with all +original mail headers remaining intact. For all other targets, +the system tries to extract both a subject line and the main text body +from the email content. In instances where emails solely consist of HTML +content, they will be transformed into plain text format during this process. + Permissions --- -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 proxmox-mail-forward 4/5] update d/control
proxmox-schema and proxmox-section config is not required anymore. add new dependency to proxmox-notify. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - new in v3 debian/control | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/debian/control b/debian/control index 43af70e..eb83a32 100644 --- a/debian/control +++ b/debian/control @@ -6,8 +6,10 @@ Build-Depends: cargo:native, librust-anyhow-1+default-dev, librust-log-0.4+default-dev (>= 0.4.17~~), librust-nix-0.26+default-dev, - librust-proxmox-schema-1+default-dev (>= 1.3~~), - librust-proxmox-section-config-1+default-dev (>= 1.0.2-~~), + librust-proxmox-notify-0.2+default-dev, + librust-proxmox-notify-0.2+mail-forwarder-dev, + librust-proxmox-notify-0.2+pbs-context-dev, + librust-proxmox-notify-0.2+pve-context-dev, librust-proxmox-sys-0.5+default-dev, librust-serde-1+default-dev, librust-serde-1+derive-dev, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 proxmox-mail-forward 3/5] feed forwarded mails into proxmox_notify
This allows us to send notifications for events from daemons that are not under our control, e.g. zed, smartd, cron. etc... For mail-based notification targets (sendmail, soon smtp) the mail is forwarded as is, including all headers. All other target types will try to parse the email to extra subject and text body. On PBS, where proxmox-notify is not yet fully integrated, we simply add a default target/matcher to an empty config. That way the behavior should be unchanged - mails will be forwarded to root@pam. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - Update to matcher-based system - no need to read datacenter.cfg/node.cfg any more Cargo.toml | 6 +- src/main.rs | 255 2 files changed, 121 insertions(+), 140 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c68e802..a562f3e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ name = "proxmox-mail-forward" version = "0.2.0" authors = [ "Fiona Ebner ", +"Lukas Wagner ", "Proxmox Support Team ", ] edition = "2021" @@ -16,10 +17,7 @@ exclude = [ "debian" ] anyhow = "1.0" log = "0.4.17" nix = "0.26" -serde = { version = "1.0", features = ["derive"] } -#serde_json = "1.0" syslog = "6.0" -proxmox-schema = "1.3" -proxmox-section-config = "1.0.2" proxmox-sys = "0.5" +proxmox-notify = {version = "0.2", features = ["mail-forwarder", "pve-context", "pbs-context"] } diff --git a/src/main.rs b/src/main.rs index f3d4193..e56bc1e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,132 +1,124 @@ +//! A helper binary that forwards any mail passed via stdin to +//! proxmox_notify. +//! +//! The binary's path is added to /root/.forward, which means that +//! postfix will invoke it when the local root user receives an email message. +//! The message is passed via stdin. +//! The binary is installed with setuid permissions and will thus run as +//! root (euid ~ root, ruid ~ nobody) +//! +//! The forwarding behavior is the following: +//! - PVE installed: Use PVE's notifications.cfg +//! - PBS installed: Use PBS's notifications.cfg if present. If not, +//! use an empty configuration and add a default sendmail target and +//! a matcher - this is needed because notifications are not yet +//! integrated in PBS. +//! - PVE/PBS co-installed: Use PVE's config *and* PBS's config, but if +//! PBS's config does not exist, a default sendmail target will *not* be +//! added. We assume that PVE's config contains the desired notification +//! behavior for system mails. +//! +use std::io::Read; use std::path::Path; -use std::process::Command; -use anyhow::{bail, format_err, Error}; -use serde::Deserialize; +use anyhow::Error; -use proxmox_schema::{ObjectSchema, Schema, StringSchema}; -use proxmox_section_config::{SectionConfig, SectionConfigPlugin}; +use proxmox_notify::context::pbs::PBS_CONTEXT; +use proxmox_notify::context::pve::PVE_CONTEXT; +use proxmox_notify::endpoints::sendmail::SendmailConfig; +use proxmox_notify::matcher::MatcherConfig; +use proxmox_notify::Config; use proxmox_sys::fs; -const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg"; -const PBS_ROOT_USER: &str = "root@pam"; - -// FIXME: Switch to the actual schema when possible in terms of dependency. -// It's safe to assume that the config was written with the actual schema restrictions, so parsing -// it with the less restrictive schema should be enough for the purpose of getting the mail address. -const DUMMY_ID_SCHEMA: Schema = StringSchema::new("dummy ID").min_length(3).schema(); -const DUMMY_EMAIL_SCHEMA: Schema = StringSchema::new("dummy email").schema(); -const DUMMY_USER_SCHEMA: ObjectSchema = ObjectSchema { -description: "minimal PBS user", -properties: &[ -("userid", false, &DUMMY_ID_SCHEMA), -("email", true, &DUMMY_EMAIL_SCHEMA), -], -additional_properties: true, -default_key: None, -}; - -#[derive(Deserialize)] -struct DummyPbsUser { -pub email: Option, -} - -const PVE_USER_CFG_FILENAME: &str = "/etc/pve/user.cfg"; -const PVE_DATACENTER_CFG_FILENAME: &str = "/etc/pve/datacenter.cfg"; -const PVE_ROOT_USER: &str = "root@pam"; +const PVE_CFG_PATH: &str = "/etc/pve"; +const PVE_PUB_NOTIFICATION_CFG_FILENAME: &str = "/etc/pve/notifications.cfg"; +const PVE_PRIV_NOTIFICATION_CFG_FILENAME: &str = "/etc/pve/priv/notifications.cfg"; -/// Convenience helper to get the trimmed contents of an optional &str, mapping blank ones to `None` -/// and creating a String from it fo
[pve-devel] [PATCH v3 proxmox 1/5] notify: add PVE/PBS context
This commit moves PVEContext from `proxmox-perl-rs` into the `proxmox-notify` crate, since we now also need to access it from `promxox-mail-forward`. The context is now hidden behind a feature flag `pve-context`, ensuring that we only compile it when needed. This commit adds PBSContext, since we now require it for `proxmox-mail-forward`. Some of the code for PBSContext comes from `proxmox-mail-forward`. This commit also changes the global context from being stored in a `once_cell` to a regular `Mutex`, since we now need to set/reset the context in `proxmox-mail-forward`. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - no changes proxmox-notify/Cargo.toml| 3 +- proxmox-notify/src/context.rs| 21 - proxmox-notify/src/context/common.rs | 27 ++ proxmox-notify/src/context/mod.rs| 36 proxmox-notify/src/context/pbs.rs| 130 +++ proxmox-notify/src/context/pve.rs| 82 + 6 files changed, 277 insertions(+), 22 deletions(-) delete mode 100644 proxmox-notify/src/context.rs create mode 100644 proxmox-notify/src/context/common.rs create mode 100644 proxmox-notify/src/context/mod.rs create mode 100644 proxmox-notify/src/context/pbs.rs create mode 100644 proxmox-notify/src/context/pve.rs diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index f2b4db5..7a3d434 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -13,7 +13,6 @@ handlebars = { workspace = true } lazy_static.workspace = true log.workspace = true mail-parser = { workspace = true, optional = true } -once_cell.workspace = true openssl.workspace = true proxmox-http = { workspace = true, features = ["client-sync"], optional = true } proxmox-http-error.workspace = true @@ -32,3 +31,5 @@ default = ["sendmail", "gotify"] mail-forwarder = ["dep:mail-parser"] sendmail = ["dep:proxmox-sys"] gotify = ["dep:proxmox-http"] +pve-context = ["dep:proxmox-sys"] +pbs-context = ["dep:proxmox-sys"] diff --git a/proxmox-notify/src/context.rs b/proxmox-notify/src/context.rs deleted file mode 100644 index 370c7ee..000 --- a/proxmox-notify/src/context.rs +++ /dev/null @@ -1,21 +0,0 @@ -use std::fmt::Debug; - -use once_cell::sync::OnceCell; - -pub trait Context: Send + Sync + Debug { -fn lookup_email_for_user(&self, user: &str) -> Option; -fn default_sendmail_author(&self) -> String; -fn default_sendmail_from(&self) -> String; -fn http_proxy_config(&self) -> Option; -} - -static CONTEXT: OnceCell<&'static dyn Context> = OnceCell::new(); - -pub fn set_context(context: &'static dyn Context) { -CONTEXT.set(context).expect("context has already been set"); -} - -#[allow(unused)] // context is not used if all endpoint features are disabled -pub(crate) fn context() -> &'static dyn Context { -*CONTEXT.get().expect("context has not been yet") -} diff --git a/proxmox-notify/src/context/common.rs b/proxmox-notify/src/context/common.rs new file mode 100644 index 000..7580bd1 --- /dev/null +++ b/proxmox-notify/src/context/common.rs @@ -0,0 +1,27 @@ +use std::path::Path; + +pub(crate) fn attempt_file_read>(path: P) -> Option { +match proxmox_sys::fs::file_read_optional_string(path) { +Ok(contents) => contents, +Err(err) => { +log::error!("{err}"); +None +} +} +} + +pub(crate) fn lookup_datacenter_config_key(content: &str, key: &str) -> Option { +let key_prefix = format!("{key}:"); +normalize_for_return( +content +.lines() +.find_map(|line| line.strip_prefix(&key_prefix)), +) +} + +pub(crate) fn normalize_for_return(s: Option<&str>) -> Option { +match s?.trim() { +"" => None, +s => Some(s.to_string()), +} +} diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs new file mode 100644 index 000..99d86de --- /dev/null +++ b/proxmox-notify/src/context/mod.rs @@ -0,0 +1,36 @@ +use std::fmt::Debug; +use std::sync::Mutex; + +#[cfg(any(feature = "pve-context", feature = "pbs-context"))] +pub mod common; +#[cfg(feature = "pbs-context")] +pub mod pbs; +#[cfg(feature = "pve-context")] +pub mod pve; + +/// Product-specific context +pub trait Context: Send + Sync + Debug { +/// Look up a user's email address from users.cfg +fn lookup_email_for_user(&self, user: &str) -> Option; +/// Default mail author for mail-based targets +fn default_sendmail_author(&self) -> String; +/// Default from address for sendmail-based targets +fn default_sendmail_from(&self) -> String; +/// Proxy configuration for the current
[pve-devel] [PATCH v3 proxmox-perl-rs 2/5] pve-rs: notify: remove notify_context for PVE
The context has now been moved to `proxmox-notify` due to the fact that we also need it in `proxmox-mail-forward` now. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - No changes pve-rs/Cargo.toml| 2 +- pve-rs/src/lib.rs| 7 ++- pve-rs/src/notify_context.rs | 117 --- 3 files changed, 5 insertions(+), 121 deletions(-) delete mode 100644 pve-rs/src/notify_context.rs diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml index e222d9d..2300c8d 100644 --- a/pve-rs/Cargo.toml +++ b/pve-rs/Cargo.toml @@ -36,7 +36,7 @@ perlmod = { version = "0.13", features = [ "exporter" ] } proxmox-apt = "0.10.6" proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] } proxmox-http-error = "0.1.0" -proxmox-notify = "0.2" +proxmox-notify = { version = "0.2", features = ["pve-context"] } proxmox-openid = "0.10" proxmox-resource-scheduling = "0.3.0" proxmox-subscription = "0.4" diff --git a/pve-rs/src/lib.rs b/pve-rs/src/lib.rs index d1915c9..42be39e 100644 --- a/pve-rs/src/lib.rs +++ b/pve-rs/src/lib.rs @@ -4,18 +4,19 @@ pub mod common; pub mod apt; -pub mod notify_context; pub mod openid; pub mod resource_scheduling; pub mod tfa; #[perlmod::package(name = "Proxmox::Lib::PVE", lib = "pve_rs")] mod export { -use crate::{common, notify_context}; +use proxmox_notify::context::pve::PVE_CONTEXT; + +use crate::common; #[export] pub fn init() { common::logger::init("PVE_LOG", "info"); -notify_context::init(); +proxmox_notify::context::set_context(&PVE_CONTEXT) } } diff --git a/pve-rs/src/notify_context.rs b/pve-rs/src/notify_context.rs deleted file mode 100644 index 3cf3e18..000 --- a/pve-rs/src/notify_context.rs +++ /dev/null @@ -1,117 +0,0 @@ -use log; -use std::path::Path; - -use proxmox_notify::context::Context; - -// Some helpers borrowed and slightly adapted from `proxmox-mail-forward` - -fn normalize_for_return(s: Option<&str>) -> Option { -match s?.trim() { -"" => None, -s => Some(s.to_string()), -} -} - -fn attempt_file_read>(path: P) -> Option { -match proxmox_sys::fs::file_read_optional_string(path) { -Ok(contents) => contents, -Err(err) => { -log::error!("{err}"); -None -} -} -} - -fn lookup_mail_address(content: &str, user: &str) -> Option { -normalize_for_return(content.lines().find_map(|line| { -let fields: Vec<&str> = line.split(':').collect(); -#[allow(clippy::get_first)] // to keep expression style consistent -match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == user { -true => fields.get(6).copied(), -false => None, -} -})) -} - -fn lookup_datacenter_config_key(content: &str, key: &str) -> Option { -let key_prefix = format!("{key}:"); -normalize_for_return( -content -.lines() -.find_map(|line| line.strip_prefix(&key_prefix)), -) -} - -#[derive(Debug)] -struct PVEContext; - -impl Context for PVEContext { -fn lookup_email_for_user(&self, user: &str) -> Option { -let content = attempt_file_read("/etc/pve/user.cfg"); -content.and_then(|content| lookup_mail_address(&content, user)) -} - -fn default_sendmail_author(&self) -> String { -"Proxmox VE".into() -} - -fn default_sendmail_from(&self) -> String { -let content = attempt_file_read("/etc/pve/datacenter.cfg"); -content -.and_then(|content| lookup_datacenter_config_key(&content, "email_from")) -.unwrap_or_else(|| String::from("root")) -} - -fn http_proxy_config(&self) -> Option { -let content = attempt_file_read("/etc/pve/datacenter.cfg"); -content.and_then(|content| lookup_datacenter_config_key(&content, "http_proxy")) -} -} - -#[cfg(test)] -mod tests { -use super::*; - -const USER_CONFIG: &str = " -user:root@pam:1:0:::r...@example.com::: -user:test@pve:1:0:::t...@example.com::: -user:no-mail@pve:1:0:: -"; - -#[test] -fn test_parse_mail() { -assert_eq!( -lookup_mail_address(USER_CONFIG, "root@pam"), -Some("r...@example.com".to_string()) -); -assert_eq!( -lookup_mail_address(USER_CONFIG, "test@pve"), -Some("t...@example.com".to_string()) -); -assert_eq!(lookup_mail_address(USER_CONFIG, "no-m
[pve-devel] [PATCH v3 many 0/5] notifications: feed system mails into proxmox_notify
The aim of this patch series is to adapt `proxmox-mail-forward` so that it forwards emails that were sent to the local root user through the `proxmox_notify` crate. A short summary of the status quo: Any mail that is sent to the local `root` user is forwarded by postfix to the `proxmox-mail-forward` binary, which receives the mail via STDIN. `proxmox-mail-forward` looks up the email address configured for the `root@pam` user in /etc/{proxmox-backup,pve}/user.cfg and then forwards the mail to this address by calling `sendmail` This patch series modifies `proxmox-mail-forward` in the following way: `proxmox-mail-forward` instantiates the configuration for `proxmox_notify` by reading `/etc/{proxmox-backup,pve}/notifications.cfg. The forwarding behavior is the following: - PVE installed: Use PVE's notifications.cfg - PBS installed: Use PBS's notifications.cfg if present. If not, use an empty configuration and add a default sendmail target and a matcher - this is needed because notifications are not yet integrated in PBS. In that way, the forwarding behavior is still the same as before on PBS (forward to root@pam via sendmail). - PVE/PBS co-installed: Use PVE's config *and* PBS's config. If PBS's notifications.cfg does not exist, a default sendmail target will *not* be added, to avoid forwarding the same mail twice. For co-installations we assume for now that PVE has a sensible matcher/target config for forwarded mails. Required patches: - series: 'overhaul notification system, use matchers instead of filters' [2] - pve-docs: 'notifications: update docs to for matcher-based notifications' [3] - Also, these two patches for 'proxmox' from the SMTP target series [4] are needed: - 'sys: email: add `forward`' - 'notify: add mechanisms for email message forwarding' Changelog: - v1 -> v2: - Rebased - Apply the same fix for the PVE context as in [1] - v2 -> v3: - Rebased on top of matcher-based notification system: This simplifies proxmox-mail-forward by a great deal, since notification routing is moved into the matcher. This means proxmox-mail-forward does not need to read /etc/pve/datacenter.cfg any more to determine the target for the notification. [1] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059294.html [2] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059818.html [3] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059872.html [4] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059894.html [5] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059899.html [6] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059900.html proxmox: Lukas Wagner (1): notify: add PVE/PBS context proxmox-notify/Cargo.toml| 3 +- proxmox-notify/src/context.rs| 21 - proxmox-notify/src/context/common.rs | 27 ++ proxmox-notify/src/context/mod.rs| 36 proxmox-notify/src/context/pbs.rs| 130 +++ proxmox-notify/src/context/pve.rs| 82 + 6 files changed, 277 insertions(+), 22 deletions(-) delete mode 100644 proxmox-notify/src/context.rs create mode 100644 proxmox-notify/src/context/common.rs create mode 100644 proxmox-notify/src/context/mod.rs create mode 100644 proxmox-notify/src/context/pbs.rs create mode 100644 proxmox-notify/src/context/pve.rs proxmox-perl-rs: Lukas Wagner (1): pve-rs: notify: remove notify_context for PVE pve-rs/Cargo.toml| 2 +- pve-rs/src/lib.rs| 7 ++- pve-rs/src/notify_context.rs | 117 --- 3 files changed, 5 insertions(+), 121 deletions(-) delete mode 100644 pve-rs/src/notify_context.rs proxmox-mail-forward: Lukas Wagner (2): feed forwarded mails into proxmox_notify update d/control Cargo.toml | 6 +- debian/control | 6 +- src/main.rs| 255 +++------ 3 files changed, 125 insertions(+), 142 deletions(-) pve-docs: Lukas Wagner (1): notifications: add documentation for system mail forwarding notifications.adoc | 16 1 file changed, 16 insertions(+) Summary over all repositories: 13 files changed, 423 insertions(+), 285 deletions(-) -- murpp v0.4.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters
Hi, thanks for your input. On 11/13/23 15:34, Dominik Csapak wrote: a few high level ui things (i did not look too deeply in the code, but i'll send probably some comments there too) Just as a warning, the tree code/data binding is definitely not as clean as it could be right now, a cleanup patch will follow. I just wanted to get these patches out ASAP ;) that probably was already there, but i find the all/any + invert combination confusing (i had to think about it for a bit before getting a grasp on it) i would propose we can write the four options out what they mean and internally convert them to all/any + invert, e.g. 'all rule match' 'any rule match' 'at least one rule does not match' (all + invert) 'no rule matches' (any + invert) I was considering this as well and discussed both approaches with Aaron. He was slightly in favor of the one I implemented, so I went with that. But now that I think about it again I think I like this version better, I'll send a followup for this (since this is is purely cosmetic). (also is the change from and/or to all/any not a breaking change?, did we expose this in the api yet ?) Well, the switch-over from filters to matchers is breaking as a whole, but it only ever hit pvetest, it is not a big problem. (config parsing was adapted so that it will clean out 'old' entries) The notification stuff was merged a bit too eagerly, this rework attempts to fix some of the issues with the first approach. second, we already have a very similar interface in the guest wizard for the disks, and there we have the remove button inline, i guess we should keep the style consistent Noted, i'll put it on my list for followups. third, do we really need the tree? we basically have the four options from above, and then a list of the matches wouldn't it be enough to seperate them? The tree was a conscious decision, because at some point I want to support 'sub-matchers' - where a matcher can reference another matcher. that way users can compose arbitrary boolean formulas, e.g.: All of match ... match ... any of match ... match ... For best UX, the sub-matchers shall be configurable within a single dialog, and this is the reason why I went with a tree widget. e.g. have a dropdown with the four options + a list instead of a tree? also currently the match options are not really verified before i can submit the form True, will fix this in a followup. Thx again! -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox 03/52] notify: introduce Error::Generic
... as leaf error-type for anything for which we do not necessarily want a separate enum variant. Signed-off-by: Lukas Wagner --- proxmox-notify/src/lib.rs | 11 +++ 1 file changed, 11 insertions(+) diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 7500778..f7d480c 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -25,13 +25,22 @@ mod config; #[derive(Debug)] pub enum Error { +/// There was an error serializing the config ConfigSerialization(Box), +/// There was an error deserializing the config ConfigDeserialization(Box), +/// An endpoint failed to send a notification NotifyFailed(String, Box), +/// A target does not exist TargetDoesNotExist(String), +/// Testing one or more notification targets failed TargetTestFailed(Vec>), +/// A filter could not be applied FilterFailed(String), +/// The notification's template string could not be rendered RenderError(Box), +/// Generic error for anything else +Generic(String), } impl Display for Error { @@ -60,6 +69,7 @@ impl Display for Error { write!(f, "could not apply filter: {message}") } Error::RenderError(err) => write!(f, "could not render notification template: {err}"), +Error::Generic(message) => f.write_str(message), } } } @@ -74,6 +84,7 @@ impl StdError for Error { Error::TargetTestFailed(errs) => Some(&*errs[0]), Error::FilterFailed(_) => None, Error::RenderError(err) => Some(&**err), +Error::Generic(_) => None, } } } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox 08/52] notify: let a matcher always match if it has no matching directives
This should be a bit more intuitive to users than the current behavior, which is 'always match' for mode==all and 'never match' for mode==any. The current behavior originates in the neutral element of the underlying logical operation (and, or). Signed-off-by: Lukas Wagner --- proxmox-notify/src/matcher.rs | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index e299fd0..553ca87 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -265,17 +265,22 @@ impl MatcherConfig { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); +// If there are no matching directives, the matcher will always match +let mut no_matchers = true; if let Some(severity_matchers) = self.match_severity.as_deref() { +no_matchers = false; is_match = mode.apply( is_match, self.check_matches(notification, severity_matchers)?, ); } if let Some(field_matchers) = self.match_field.as_deref() { +no_matchers = false; is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?); } if let Some(calendar_matchers) = self.match_calendar.as_deref() { +no_matchers = false; is_match = mode.apply( is_match, self.check_matches(notification, calendar_matchers)?, @@ -284,7 +289,7 @@ impl MatcherConfig { let invert_match = self.invert_match.unwrap_or_default(); -Ok(if is_match != invert_match { +Ok(if is_match != invert_match || no_matchers { Some(self.target.as_deref().unwrap_or_default()) } else { None @@ -455,4 +460,25 @@ mod tests { let matcher: SeverityMatcher = "info,notice,warning,error".parse().unwrap(); assert!(matcher.matches(¬ification).unwrap()); } + +#[test] +fn test_empty_matcher_matches_always() { +let notification = Notification::new_templated( +Severity::Notice, +"test", +"test", +Value::Null, +Default::default(), +); + +for mode in [MatchModeOperator::All, MatchModeOperator::Any] { +let config = MatcherConfig { +name: "matcher".to_string(), +mode: Some(mode), +..Default::default() +}; + +assert!(config.matches(¬ification).unwrap().is_some()) +} +} } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox 06/52] notify: add calendar matcher
This allows matching by a notification's timestamp: matcher: foo match-calendar mon..fri 8-12 Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/matcher.rs | 6 +++ proxmox-notify/src/lib.rs | 4 ++ proxmox-notify/src/matcher.rs | 65 +++ 3 files changed, 75 insertions(+) diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs index e37b74f..0592b14 100644 --- a/proxmox-notify/src/api/matcher.rs +++ b/proxmox-notify/src/api/matcher.rs @@ -80,6 +80,7 @@ pub fn update_matcher( match deleteable_property { DeleteableMatcherProperty::MatchSeverity => matcher.match_severity = None, DeleteableMatcherProperty::MatchField => matcher.match_field = None, +DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar = None, DeleteableMatcherProperty::Target => matcher.target = None, DeleteableMatcherProperty::Mode => matcher.mode = None, DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None, @@ -96,6 +97,10 @@ pub fn update_matcher( matcher.match_field = Some(match_field.clone()); } +if let Some(match_calendar) = &matcher_updater.match_calendar { +matcher.match_calendar = Some(match_calendar.clone()); +} + if let Some(mode) = matcher_updater.mode { matcher.mode = Some(mode); } @@ -200,6 +205,7 @@ matcher: matcher2 mode: Some(MatchModeOperator::Any), match_field: None, match_severity: None, +match_calendar: None, invert_match: Some(true), target: Some(vec!["foo".into()]), comment: Some("new comment".into()), diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 1f95ae0..9997cef 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -154,6 +154,8 @@ pub enum Content { pub struct Metadata { /// Notification severity severity: Severity, +/// Timestamp of the notification as a UNIX epoch +timestamp: i64, /// Additional fields for additional key-value metadata additional_fields: HashMap, } @@ -179,6 +181,7 @@ impl Notification { metadata: Metadata { severity, additional_fields: fields, +timestamp: proxmox_time::epoch_i64(), }, content: Content::Template { title_template: title.as_ref().to_string(), @@ -393,6 +396,7 @@ impl Bus { severity: Severity::Info, // TODO: what fields would make sense for test notifications? additional_fields: Default::default(), +timestamp: proxmox_time::epoch_i64(), }, content: Content::Template { title_template: "Test notification".into(), diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index c24726d..b03d11d 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -10,6 +10,7 @@ use proxmox_schema::api_types::COMMENT_SCHEMA; use proxmox_schema::{ api, const_regex, ApiStringFormat, Schema, StringSchema, Updater, SAFE_ID_REGEX_STR, }; +use proxmox_time::{parse_daily_duration, DailyDuration}; use crate::schema::ENTITY_NAME_SCHEMA; use crate::{Error, Notification, Severity}; @@ -88,6 +89,14 @@ pub const MATCH_FIELD_ENTRY_SCHEMA: Schema = StringSchema::new("Match metadata f }, optional: true, }, +"match-calendar": { +type: Array, +items: { +description: "Time stamps to match", +type: String +}, +optional: true, +}, "target": { type: Array, items: { @@ -112,6 +121,10 @@ pub struct MatcherConfig { #[serde(skip_serializing_if = "Option::is_none")] pub match_severity: Option>, +/// List of matched severity levels +#[serde(skip_serializing_if = "Option::is_none")] +pub match_calendar: Option>, + /// Decide if 'all' or 'any' match statements must match #[serde(skip_serializing_if = "Option::is_none")] pub mode: Option, @@ -249,6 +262,7 @@ impl MatcherConfig { let mut is_match = mode.neutral_element(); is_match = mode.apply(is_match, self.check_severity_match(notification)); is_match = mode.apply(is_match, self.check_field_match(notification)?); +is_match = mode.apply(is_match, self.check_calendar_match(notification)?); let invert_match = self.invert_match.unwrap_or_default(); @@ -285,6 +299,19 @@ impl MatcherConfig { is_match } + +fn check_calendar_match(
[pve-devel] [PATCH v2 proxmox 10/52] notify: add mechanisms for email message forwarding
As preparation for the integration of `proxmox-mail-foward` into the notification system, this commit makes a few changes that allow us to forward raw email messages (as passed from postfix). For mail-based notification targets, the email will be forwarded as-is, including all headers. The only thing that changes is the message envelope. For other notification targets, the mail is parsed using the `mail-parser` crate, which allows us to extract a subject and a body. As a body we use the plain-text version of the mail. If an email is HTML-only, the `mail-parser` crate will automatically attempt to transform the HTML into readable plain text. Signed-off-by: Lukas Wagner --- Cargo.toml | 1 + proxmox-notify/Cargo.toml| 2 ++ proxmox-notify/src/endpoints/gotify.rs | 3 ++ proxmox-notify/src/endpoints/sendmail.rs | 5 +++ proxmox-notify/src/lib.rs| 41 5 files changed, 52 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index f8bc181..3d81d85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ lazy_static = "1.4" ldap3 = { version = "0.11", default-features = false } libc = "0.2.107" log = "0.4.17" +mail-parser = "0.8.2" native-tls = "0.2" nix = "0.26.1" once_cell = "1.3.1" diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index 4812896..f2b4db5 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -12,6 +12,7 @@ anyhow.workspace = true handlebars = { workspace = true } lazy_static.workspace = true log.workspace = true +mail-parser = { workspace = true, optional = true } once_cell.workspace = true openssl.workspace = true proxmox-http = { workspace = true, features = ["client-sync"], optional = true } @@ -28,5 +29,6 @@ serde_json.workspace = true [features] default = ["sendmail", "gotify"] +mail-forwarder = ["dep:mail-parser"] sendmail = ["dep:proxmox-sys"] gotify = ["dep:proxmox-http"] diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs index 1c307a4..5713d99 100644 --- a/proxmox-notify/src/endpoints/gotify.rs +++ b/proxmox-notify/src/endpoints/gotify.rs @@ -19,6 +19,7 @@ fn severity_to_priority(level: Severity) -> u32 { Severity::Notice => 3, Severity::Warning => 5, Severity::Error => 9, +Severity::Unknown => 3, } } @@ -94,6 +95,8 @@ impl Endpoint for GotifyEndpoint { (rendered_title, rendered_message) } +#[cfg(feature = "mail-forwarder")] +Content::ForwardedMail { title, body, .. } => (title.clone(), body.clone()), }; // We don't have a TemplateRenderer::Markdown yet, so simply put everything diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs index a601744..3ef33b6 100644 --- a/proxmox-notify/src/endpoints/sendmail.rs +++ b/proxmox-notify/src/endpoints/sendmail.rs @@ -134,6 +134,11 @@ impl Endpoint for SendmailEndpoint { ) .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into())) } +#[cfg(feature = "mail-forwarder")] +Content::ForwardedMail { raw, uid, .. } => { +proxmox_sys::email::forward(&recipients_str, &mailfrom, raw, *uid) +.map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into())) +} } } diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 9997cef..ada1b0a 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -102,6 +102,8 @@ pub enum Severity { Warning, /// Error Error, +/// Unknown severity (e.g. forwarded system mails) +Unknown, } impl Display for Severity { @@ -111,6 +113,7 @@ impl Display for Severity { Severity::Notice => f.write_str("notice"), Severity::Warning => f.write_str("warning"), Severity::Error => f.write_str("error"), +Severity::Unknown => f.write_str("unknown"), } } } @@ -123,6 +126,7 @@ impl FromStr for Severity { "notice" => Ok(Self::Notice), "warning" => Ok(Self::Warning), "error" => Ok(Self::Error), +"unknown" => Ok(Self::Unknown), _ => Err(Error::Generic(format!("invalid severity {s}"))), } } @@ -148,6 +152,18 @@ pub enum Content { /// Data that can be used for template rendering. data: Value, }, +#[cfg(feature = "mail-forwarder")] +ForwardedMail { +/// Raw mail contents +
[pve-devel] [PATCH v2 proxmox 04/52] notify: factor out notification content into its own type
This will be useful later for system mail forwarding, where the content of the mail should be forwarded unchanged. This moves notification properties into this new type and calls them 'data'. They will exclusively used for template rendering. `Notification` will receive a separate field for metadata, which will be useful for notification filtering. This decouples template rendering and filtering, which enables us to be very precise about which metadata fields we allow to be used in filters. Signed-off-by: Lukas Wagner --- proxmox-notify/examples/render.rs| 4 +- proxmox-notify/src/endpoints/gotify.rs | 26 +--- proxmox-notify/src/endpoints/sendmail.rs | 62 +- proxmox-notify/src/filter.rs | 10 +-- proxmox-notify/src/lib.rs| 81 ++-- proxmox-notify/src/renderer/mod.rs | 15 ++--- 6 files changed, 109 insertions(+), 89 deletions(-) diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs index c0a6f27..d705fd0 100644 --- a/proxmox-notify/examples/render.rs +++ b/proxmox-notify/examples/render.rs @@ -53,10 +53,10 @@ fn main() -> Result<(), Error> { } }); -let output = render_template(TemplateRenderer::Html, TEMPLATE, Some(&properties))?; +let output = render_template(TemplateRenderer::Html, TEMPLATE, &properties)?; println!("{output}"); -let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, Some(&properties))?; +let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, &properties)?; println!("{output}"); Ok(()) diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs index 83df41f..af86f9c 100644 --- a/proxmox-notify/src/endpoints/gotify.rs +++ b/proxmox-notify/src/endpoints/gotify.rs @@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater}; use crate::context::context; use crate::renderer::TemplateRenderer; use crate::schema::ENTITY_NAME_SCHEMA; -use crate::{renderer, Endpoint, Error, Notification, Severity}; +use crate::{renderer, Content, Endpoint, Error, Notification, Severity}; fn severity_to_priority(level: Severity) -> u32 { match level { @@ -85,15 +85,21 @@ pub enum DeleteableGotifyProperty { impl Endpoint for GotifyEndpoint { fn send(&self, notification: &Notification) -> Result<(), Error> { -let properties = notification.properties.as_ref(); - -let title = renderer::render_template( -TemplateRenderer::Plaintext, -¬ification.title, -properties, -)?; -let message = -renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?; + +let (title, message) = match ¬ification.content { +Content::Template { +title_template, +body_template, +data +} => { +let rendered_title = +renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?; +let rendered_message = +renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?; + +(rendered_title, rendered_message) +} +}; // We don't have a TemplateRenderer::Markdown yet, so simply put everything // in code tags. Otherwise tables etc. are not formatted properly diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs index 26e2a17..c540925 100644 --- a/proxmox-notify/src/endpoints/sendmail.rs +++ b/proxmox-notify/src/endpoints/sendmail.rs @@ -8,7 +8,7 @@ use proxmox_schema::{api, Updater}; use crate::context::context; use crate::renderer::TemplateRenderer; use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA}; -use crate::{renderer, Endpoint, Error, Notification}; +use crate::{renderer, Content, Endpoint, Error, Notification}; pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail"; @@ -102,41 +102,43 @@ impl Endpoint for SendmailEndpoint { } } -let properties = notification.properties.as_ref(); - -let subject = renderer::render_template( -TemplateRenderer::Plaintext, -¬ification.title, -properties, -)?; -let html_part = -renderer::render_template(TemplateRenderer::Html, ¬ification.body, properties)?; -let text_part = -renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?; - -let author = self -.config -.author -.clone() -.unwrap_or_else(|| context().default_sendmail_author()); - +let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect(); let mailfrom = self
[pve-devel] [PATCH v2 pve-ha-manager 23/52] env: switch to matcher-based notification system
Signed-off-by: Lukas Wagner --- src/PVE/HA/Env/PVE2.pm | 10 ++ src/PVE/HA/NodeStatus.pm | 11 +-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm index ea9e6e4..fcb60a9 100644 --- a/src/PVE/HA/Env/PVE2.pm +++ b/src/PVE/HA/Env/PVE2.pm @@ -221,16 +221,10 @@ sub log { } sub send_notification { -my ($self, $subject, $text, $properties) = @_; +my ($self, $subject, $text, $template_data, $metadata_fields) = @_; eval { - my $dc_config = PVE::Cluster::cfs_read_file('datacenter.cfg'); - my $target = $dc_config->{notify}->{'target-fencing'} // PVE::Notify::default_target(); - my $notify = $dc_config->{notify}->{fencing} // 'always'; - - if ($notify eq 'always') { - PVE::Notify::error($target, $subject, $text, $properties); - } + PVE::Notify::error($subject, $text, $template_data, $metadata_fields); }; $self->log("warning", "could not notify: $@") if $@; diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm index b264a36..e053c55 100644 --- a/src/PVE/HA/NodeStatus.pm +++ b/src/PVE/HA/NodeStatus.pm @@ -212,7 +212,7 @@ my $send_fence_state_email = sub { my $haenv = $self->{haenv}; my $status = $haenv->read_manager_status(); -my $notification_properties = { +my $template_data = { "status-data"=> { manager_status => $status, node_status=> $self->{status} @@ -222,11 +222,18 @@ my $send_fence_state_email = sub { "subject"=> $subject, }; +my $metadata_fields = { + type => 'fencing', + hostname => $node, +}; + $haenv->send_notification( $subject_template, $body_template, - $notification_properties + $template_data, + $metadata_fields, ); + }; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 many 00/52] revamp notifications; smtp endpoints; system mail
ceives the mail via STDIN. `proxmox-mail-forward` looks up the email address configured for the `root@pam` user in /etc/{proxmox-backup,pve}/user.cfg and then forwards the mail to this address by calling `sendmail` This patch series modifies `proxmox-mail-forward` in the following way: `proxmox-mail-forward` instantiates the configuration for `proxmox_notify` by reading `/etc/{proxmox-backup,pve}/notifications.cfg. The forwarding behavior is the following: - PVE installed: Use PVE's notifications.cfg - PBS installed: Use PBS's notifications.cfg if present. If not, use an empty configuration and add a default sendmail target and a matcher - this is needed because notifications are not yet integrated in PBS. In that way, the forwarding behavior is still the same as before on PBS (forward to root@pam via sendmail). - PVE/PBS co-installed: Use PVE's config *and* PBS's config. If PBS's notifications.cfg does not exist, a default sendmail target will *not* be added, to avoid forwarding the same mail twice. For co-installations we assume for now that PVE has a sensible matcher/target config for forwarded mails. Changelog: - Merged series: no changes - v1 -> v2: - Rebased - Apply the same fix for the PVE context as in [1] - v2 -> v3: - Rebased on top of matcher-based notification system: This simplifies proxmox-mail-forward by a great deal, since notification routing is moved into the matcher. This means proxmox-mail-forward does not need to read /etc/pve/datacenter.cfg any more to determine the target for the notification. [1] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059294.html [2] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059818.html [3] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059872.html [4] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059894.html [5] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059899.html [6] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059900.html debcargo-conf: Lukas Wagner (2): cherry-pick chumsky 0.9.2 from debian unstable update lettre to 0.11.1 src/chumsky/debian/changelog | 5 ++ src/chumsky/debian/copyright | 39 +++ src/chumsky/debian/copyright.debcargo.hint| 51 ++ src/chumsky/debian/debcargo.toml | 2 + src/lettre/debian/changelog | 10 +++ .../debian/patches/downgrade_fastrand.patch | 13 .../debian/patches/downgrade_idna.patch | 13 src/lettre/debian/patches/downgrade_url.patch | 13 .../patches/remove_unused_features.patch | 69 ++- src/lettre/debian/patches/series | 4 +- .../patches/upgrade_quoted_printable.patch| 13 11 files changed, 185 insertions(+), 47 deletions(-) create mode 100644 src/chumsky/debian/changelog create mode 100644 src/chumsky/debian/copyright create mode 100644 src/chumsky/debian/copyright.debcargo.hint create mode 100644 src/chumsky/debian/debcargo.toml create mode 100644 src/lettre/debian/patches/downgrade_fastrand.patch create mode 100644 src/lettre/debian/patches/downgrade_idna.patch create mode 100644 src/lettre/debian/patches/downgrade_url.patch delete mode 100644 src/lettre/debian/patches/upgrade_quoted_printable.patch proxmox: Lukas Wagner (13): notify: introduce Error::Generic notify: factor out notification content into its own type notify: replace filters and groups with matcher-based system notify: add calendar matcher notify: matcher: introduce common trait for match directives notify: let a matcher always match if it has no matching directives sys: email: add `forward` notify: add mechanisms for email message forwarding notify: add PVE/PBS context notify: add 'smtp' endpoint notify: add api for smtp endpoints notify: add 'disable' parameter for matchers and targets. notify: add built-in config and 'origin' parameter Cargo.toml | 2 + proxmox-notify/Cargo.toml | 11 +- proxmox-notify/examples/render.rs | 4 +- proxmox-notify/src/api/common.rs| 6 +- proxmox-notify/src/api/filter.rs| 231 - proxmox-notify/src/api/gotify.rs| 22 +- proxmox-notify/src/api/group.rs | 259 -- proxmox-notify/src/api/matcher.rs | 265 ++ proxmox-notify/src/api/mod.rs | 146 ++ proxmox-notify/src/api/sendmail.rs | 24 +- proxmox-notify/src/api/smtp.rs | 362 ++ proxmox-notify/src/config.rs| 57 ++- proxmox-notify/src/context.rs | 21 - proxmox-notify/src/context/common.rs| 27 + proxmox-notify/src/context/mod.rs | 43 ++ proxmox-notify/src/context/pbs.rs
[pve-devel] [PATCH v2 pve-manager 30/52] test: fix vzdump notification test
The signature of the PVE::Notify functions have changed, this commit adapts the mocked functions so that the tests work again. Signed-off-by: Lukas Wagner --- test/vzdump_notification_test.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/vzdump_notification_test.pl b/test/vzdump_notification_test.pl index 21c31651..631606bb 100755 --- a/test/vzdump_notification_test.pl +++ b/test/vzdump_notification_test.pl @@ -38,14 +38,14 @@ my $result_properties; my $mock_notification_module = Test::MockModule->new('PVE::Notify'); my $mocked_notify = sub { -my ($channel, $severity, $title, $text, $properties) = @_; +my ($severity, $title, $text, $properties, $metadata) = @_; $result_text = $text; $result_properties = $properties; }; my $mocked_notify_short = sub { -my ($channel, @rest) = @_; -return $mocked_notify->($channel, '', @rest); +my (@params) = @_; +return $mocked_notify->('', @params); }; $mock_notification_module->mock( -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 pve-manager 29/52] api: replication: adapt to matcher-based notification system
Signed-off-by: Lukas Wagner --- PVE/API2/Replication.pm | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm index d61518ba..0dc944c9 100644 --- a/PVE/API2/Replication.pm +++ b/PVE/API2/Replication.pm @@ -129,7 +129,7 @@ my sub _handle_job_err { # The replication job is run every 15 mins if no schedule is set. my $schedule = $job->{schedule} // '*/15'; -my $properties = { +my $template_data = { "failure-count" => $fail_count, "last-sync" => $jobstate->{last_sync}, "next-sync" => $next_sync, @@ -139,19 +139,18 @@ my sub _handle_job_err { "error" => $err, }; +my $metadata_fields = { + # TODO: Add job-id? + type => "replication", +}; + eval { - my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg'); - my $target = $dcconf->{notify}->{'target-replication'} // PVE::Notify::default_target(); - my $notify = $dcconf->{notify}->{'replication'} // 'always'; - - if ($notify eq 'always') { - PVE::Notify::error( - $target, - $replication_error_subject_template, - $replication_error_body_template, - $properties - ); - } + PVE::Notify::error( + $replication_error_subject_template, + $replication_error_body_template, + $template_data, + $metadata_fields + ); }; warn ": $@" if $@; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 pve-manager 28/52] api: apt: adapt to matcher-based notifications
Signed-off-by: Lukas Wagner --- PVE/API2/APT.pm | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm index a213fc59..da75a4dc 100644 --- a/PVE/API2/APT.pm +++ b/PVE/API2/APT.pm @@ -286,8 +286,6 @@ __PACKAGE__->register_method({ description => "This is used to resynchronize the package index files from their sources (apt-get update).", permissions => { check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]], - description => "If 'notify: target-package-updates' is set, then the user must have the " - . "'Mapping.Use' permission on '/mapping/notification/'", }, protected => 1, proxyto => 'node', @@ -297,7 +295,7 @@ __PACKAGE__->register_method({ node => get_standard_option('pve-node'), notify => { type => 'boolean', - description => "Send notification mail about new packages (to email address specified for user 'root\@pam').", + description => "Send notification about new packages.", optional => 1, default => 0, }, @@ -317,16 +315,6 @@ __PACKAGE__->register_method({ my $rpcenv = PVE::RPCEnvironment::get(); my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg'); - my $target = $dcconf->{notify}->{'target-package-updates'} // - PVE::Notify::default_target(); - - if ($param->{notify} && $target ne PVE::Notify::default_target()) { - # If we notify via anything other than the default target (mail to root), - # then the user must have the proper permissions for the target. - # The mail-to-root target does not require these, as otherwise - # we would break compatibility. - PVE::Notify::check_may_use_target($target, $rpcenv); - } my $authuser = $rpcenv->get_user(); @@ -392,16 +380,23 @@ __PACKAGE__->register_method({ return if !$count; - my $properties = { + my $template_data = { updates => $updates_table, hostname => $hostname, }; + # Additional metadata fields that can be used in notification + # matchers. + my $metadata_fields = { + type => 'package-updates', + hostname => $hostname, + }; + PVE::Notify::info( - $target, $updates_available_subject_template, $updates_available_body_template, - $properties, + $template_data, + $metadata_fields, ); foreach my $pi (@$pkglist) { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 debcargo-conf 01/52] cherry-pick chumsky 0.9.2 from debian unstable
Signed-off-by: Lukas Wagner --- src/chumsky/debian/changelog | 5 +++ src/chumsky/debian/copyright | 39 + src/chumsky/debian/copyright.debcargo.hint | 51 ++ src/chumsky/debian/debcargo.toml | 2 + 4 files changed, 97 insertions(+) create mode 100644 src/chumsky/debian/changelog create mode 100644 src/chumsky/debian/copyright create mode 100644 src/chumsky/debian/copyright.debcargo.hint create mode 100644 src/chumsky/debian/debcargo.toml diff --git a/src/chumsky/debian/changelog b/src/chumsky/debian/changelog new file mode 100644 index 0..ae6b5ff8f --- /dev/null +++ b/src/chumsky/debian/changelog @@ -0,0 +1,5 @@ +rust-chumsky (0.9.2-1) unstable; urgency=medium + + * Package chumsky 0.9.2 from crates.io using debcargo 2.6.0 + + -- Jelmer Vernooij Wed, 14 Jun 2023 23:38:48 +0100 diff --git a/src/chumsky/debian/copyright b/src/chumsky/debian/copyright new file mode 100644 index 0..eaa3e6768 --- /dev/null +++ b/src/chumsky/debian/copyright @@ -0,0 +1,39 @@ +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ +Upstream-Name: chumsky +Upstream-Contact: + Joshua Barretto + Elijah Hartvigsen +Source: https://github.com/zesterer/chumsky + +Files: * +Copyright: + 2021-2023 Joshua Barretto + 2021-2023 Elijah Hartvigsen +License: MIT + +Files: debian/* +Copyright: + 2023 Debian Rust Maintainers + 2023 Jelmer Vernooij +License: MIT + +License: MIT + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + . + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + . + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. diff --git a/src/chumsky/debian/copyright.debcargo.hint b/src/chumsky/debian/copyright.debcargo.hint new file mode 100644 index 0..e02a9ab0f --- /dev/null +++ b/src/chumsky/debian/copyright.debcargo.hint @@ -0,0 +1,51 @@ +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ +Upstream-Name: chumsky +Upstream-Contact: + Joshua Barretto + Elijah Hartvigsen +Source: https://github.com/zesterer/chumsky + +Files: * +Copyright: + FIXME (overlay) UNKNOWN-YEARS Joshua Barretto + FIXME (overlay) UNKNOWN-YEARS Elijah Hartvigsen +License: MIT +Comment: + FIXME (overlay): Since upstream copyright years are not available in + Cargo.toml, they were extracted from the upstream Git repository. This may not + be correct information so you should review and fix this before uploading to + the archive. + +Files: LICENSE +Copyright: 2021 Joshua Barretto +License: UNKNOWN-LICENSE; FIXME (overlay) +Comment: + FIXME (overlay): These notices are extracted from files. Please review them + before uploading to the archive. + +Files: debian/* +Copyright: + 2023 Debian Rust Maintainers + 2023 Jelmer Vernooij +License: MIT + +License: MIT + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + . + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + . + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. diff --git a/src/chumsky/debian/debcargo.toml b/src/chumsky/debian/debcargo.toml new file mode 100644 index 0..77e8151ed --- /dev/null +++ b/src/chumsky/debian/debcargo.toml @
[pve-devel] [PATCH v2 proxmox-widget-toolkit 36/52] notification ui: add target selector for matcher
Signed-off-by: Lukas Wagner --- src/window/NotificationFilterEdit.js | 145 +++ 1 file changed, 145 insertions(+) diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationFilterEdit.js index 703a9e2..bcde4fa 100644 --- a/src/window/NotificationFilterEdit.js +++ b/src/window/NotificationFilterEdit.js @@ -49,6 +49,11 @@ Ext.define('Proxmox.panel.NotificationFilterEditPanel', { deleteDefaultValue: '{!isCreate}', }, }, + { + xtype: 'pmxNotificationTargetSelector', + name: 'target', + allowBlank: false, + }, { xtype: 'proxmoxtextfield', name: 'comment', @@ -107,3 +112,143 @@ Ext.define('Proxmox.window.NotificationFilterEdit', { } }, }); + +Ext.define('Proxmox.form.NotificationTargetSelector', { +extend: 'Ext.grid.Panel', +alias: 'widget.pmxNotificationTargetSelector', + +mixins: { + field: 'Ext.form.field.Field', +}, + +padding: '0 0 10 0', + +allowBlank: true, +selectAll: false, +isFormField: true, + +store: { + autoLoad: true, + model: 'proxmox-notification-endpoints', + sorters: 'name', +}, + +columns: [ + { + header: gettext('Target Name'), + dataIndex: 'name', + flex: 1, + }, + { + header: gettext('Type'), + dataIndex: 'type', + flex: 1, + }, + { + header: gettext('Comment'), + dataIndex: 'comment', + flex: 3, + }, +], + +selModel: { + selType: 'checkboxmodel', + mode: 'SIMPLE', +}, + +checkChangeEvents: [ + 'selectionchange', + 'change', +], + +listeners: { + selectionchange: function() { + // to trigger validity and error checks + this.checkChange(); + }, +}, + +getSubmitData: function() { + let me = this; + let res = {}; + res[me.name] = me.getValue(); + return res; +}, + +getValue: function() { + let me = this; + if (me.savedValue !== undefined) { + return me.savedValue; + } + let sm = me.getSelectionModel(); + return (sm.getSelection() ?? []).map(item => item.data.name); +}, + +setValueSelection: function(value) { + let me = this; + + let store = me.getStore(); + + let notFound = []; + let selection = value.map(item => { + let found = store.findRecord('name', item, 0, false, true, true); + if (!found) { + notFound.push(item); + } + return found; + }).filter(r => r); + + for (const name of notFound) { + let rec = store.add({ + name, + type: '-', + comment: gettext('Included target does not exist!'), + }); + selection.push(rec[0]); + } + + let sm = me.getSelectionModel(); + if (selection.length) { + sm.select(selection); + } else { + sm.deselectAll(); + } + // to correctly trigger invalid class + me.getErrors(); +}, + +setValue: function(value) { + let me = this; + + let store = me.getStore(); + if (!store.isLoaded()) { + me.savedValue = value; + store.on('load', function() { + me.setValueSelection(value); + delete me.savedValue; + }, { single: true }); + } else { + me.setValueSelection(value); + } + return me.mixins.field.setValue.call(me, value); +}, + +getErrors: function(value) { + let me = this; + if (!me.isDisabled() && me.allowBlank === false && + me.getSelectionModel().getCount() === 0) { + me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']); + return [gettext('No target selected')]; + } + + me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']); + return []; +}, + +initComponent: function() { + let me = this; + me.callParent(); + me.initField(); +}, + +}); -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 pve-manager 24/52] api: notification: remove notification groups
Signed-off-by: Lukas Wagner --- PVE/API2/Cluster/Notifications.pm | 267 +- 1 file changed, 4 insertions(+), 263 deletions(-) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index ec666903..b34802c8 100644 --- a/PVE/API2/Cluster/Notifications.pm +++ b/PVE/API2/Cluster/Notifications.pm @@ -121,7 +121,6 @@ __PACKAGE__->register_method ({ my $result = [ { name => 'endpoints' }, { name => 'filters' }, - { name => 'groups' }, { name => 'targets' }, ]; @@ -161,8 +160,7 @@ __PACKAGE__->register_method ({ name => 'get_all_targets', path => 'targets', method => 'GET', -description => 'Returns a list of all entities that can be used as notification targets' . - ' (endpoints and groups).', +description => 'Returns a list of all entities that can be used as notification targets.', permissions => { description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" . " 'Mapping.Audit' permissions on '/mapping/notification/'." @@ -180,14 +178,14 @@ __PACKAGE__->register_method ({ type => 'object', properties => { name => { - description => 'Name of the endpoint/group.', + description => 'Name of the target.', type => 'string', format => 'pve-configid', }, 'type' => { - description => 'Type of the endpoint or group.', + description => 'Type of the target.', type => 'string', - enum => [qw(sendmail gotify group)], + enum => [qw(sendmail gotify)], }, 'comment' => { description => 'Comment', @@ -221,14 +219,6 @@ __PACKAGE__->register_method ({ }; } - for my $target (@{$config->get_groups()}) { - push @$result, { - name => $target->{name}, - comment => $target->{comment}, - type => 'group', - }; - } - $result }; @@ -290,255 +280,6 @@ __PACKAGE__->register_method ({ } }); -my $group_properties = { -name => { - description => 'Name of the group.', - type => 'string', - format => 'pve-configid', -}, -'endpoint' => { - type => 'array', - items => { - type => 'string', - format => 'pve-configid', - }, - description => 'List of included endpoints', -}, -'comment' => { - description => 'Comment', - type => 'string', - optional => 1, -}, -filter => { - description => 'Name of the filter that should be applied.', - type => 'string', - format => 'pve-configid', - optional => 1, -}, -}; - -__PACKAGE__->register_method ({ -name => 'get_groups', -path => 'groups', -method => 'GET', -description => 'Returns a list of all groups', -protected => 1, -permissions => { - description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or" - . " 'Mapping.Audit' permissions on '/mapping/notification/'.", - user => 'all', -}, -parameters => { - additionalProperties => 0, - properties => {}, -}, -returns => { - type => 'array', - items => { - type => 'object', - properties => $group_properties, - }, - links => [ { rel => 'child', href => '{name}' } ], -}, -code => sub { - my $config = PVE::Notify::read_config(); - my $rpcenv = PVE::RPCEnvironment::get(); - - my $entities = eval { - $config->get_groups(); - }; - raise_api_error($@) if $@; - - return filter_entities_by_privs($rpcenv, $entities); -} -}); - -__PACKAGE__->register_method ({ -name => 'get_group', -path => 'groups/{name}', -method => 'GET', -description => 'Retu
[pve-devel] [PATCH v2 proxmox 14/52] notify: add 'disable' parameter for matchers and targets.
Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/gotify.rs | 8 +++- proxmox-notify/src/api/matcher.rs| 6 ++ proxmox-notify/src/api/sendmail.rs | 8 proxmox-notify/src/api/smtp.rs | 6 ++ proxmox-notify/src/endpoints/gotify.rs | 9 + proxmox-notify/src/endpoints/sendmail.rs | 11 ++- proxmox-notify/src/endpoints/smtp.rs | 9 + proxmox-notify/src/lib.rs| 13 + proxmox-notify/src/matcher.rs| 21 - 9 files changed, 84 insertions(+), 7 deletions(-) diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs index 22d3d2e..10f5d7d 100644 --- a/proxmox-notify/src/api/gotify.rs +++ b/proxmox-notify/src/api/gotify.rs @@ -88,6 +88,7 @@ pub fn update_endpoint( for deleteable_property in delete { match deleteable_property { DeleteableGotifyProperty::Comment => endpoint.comment = None, +DeleteableGotifyProperty::Disable => endpoint.disable = None, } } } @@ -110,6 +111,10 @@ pub fn update_endpoint( endpoint.comment = Some(comment.into()); } +if let Some(disable) = &endpoint_config_updater.disable { +endpoint.disable = Some(*disable); +} + config .config .set_data(name, GOTIFY_TYPENAME, &endpoint) @@ -172,7 +177,7 @@ mod tests { name: "gotify-endpoint".into(), server: "localhost".into(), comment: Some("comment".into()), -filter: None, +..Default::default() }, &GotifyPrivateConfig { name: "gotify-endpoint".into(), @@ -232,6 +237,7 @@ mod tests { &GotifyConfigUpdater { server: Some("newhost".into()), comment: Some("newcomment".into()), +..Default::default() }, &GotifyPrivateConfigUpdater { token: Some("changedtoken".into()), diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs index 0592b14..a69ca40 100644 --- a/proxmox-notify/src/api/matcher.rs +++ b/proxmox-notify/src/api/matcher.rs @@ -85,6 +85,7 @@ pub fn update_matcher( DeleteableMatcherProperty::Mode => matcher.mode = None, DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None, DeleteableMatcherProperty::Comment => matcher.comment = None, +DeleteableMatcherProperty::Disable => matcher.disable = None, } } } @@ -113,6 +114,10 @@ pub fn update_matcher( matcher.comment = Some(comment.into()); } +if let Some(disable) = &matcher_updater.disable { +matcher.disable = Some(*disable); +} + if let Some(target) = &matcher_updater.target { super::ensure_endpoints_exist(config, target.as_slice())?; matcher.target = Some(target.clone()); @@ -209,6 +214,7 @@ matcher: matcher2 invert_match: Some(true), target: Some(vec!["foo".into()]), comment: Some("new comment".into()), +..Default::default() }, None, Some(&digest), diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs index dbd9559..1f6e9ae 100644 --- a/proxmox-notify/src/api/sendmail.rs +++ b/proxmox-notify/src/api/sendmail.rs @@ -85,6 +85,7 @@ pub fn update_endpoint( DeleteableSendmailProperty::Comment => endpoint.comment = None, DeleteableSendmailProperty::Mailto => endpoint.mailto = None, DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user = None, +DeleteableSendmailProperty::Disable => endpoint.disable = None, } } } @@ -109,6 +110,10 @@ pub fn update_endpoint( endpoint.comment = Some(comment.into()); } +if let Some(disable) = &updater.disable { +endpoint.disable = Some(*disable); +} + if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() { http_bail!( BAD_REQUEST, @@ -165,6 +170,7 @@ pub mod tests { author: Some("root".into()), comment: Some("Comment".into()), filter: None, +..Default::default() }, )?; @@ -208,6 +214,7 @@ pub mod tests { from_address: Some("r...@example.com".into()), author: Some("newauthor".into()), comment: Some("new comment".into()), +..Default::default() }, None,
[pve-devel] [PATCH v2 proxmox-widget-toolkit 39/52] notification ui: rename filter to matcher
Signed-off-by: Lukas Wagner --- src/Makefile | 2 +- src/data/model/NotificationConfig.js | 2 +- src/panel/NotificationConfigView.js | 26 +-- ...lterEdit.js => NotificationMatcherEdit.js} | 14 +- 4 files changed, 22 insertions(+), 22 deletions(-) rename src/window/{NotificationFilterEdit.js => NotificationMatcherEdit.js} (92%) diff --git a/src/Makefile b/src/Makefile index e07f17c..c6d31c3 100644 --- a/src/Makefile +++ b/src/Makefile @@ -88,7 +88,7 @@ JSSRC=\ window/ACMEPluginEdit.js\ window/ACMEDomains.js \ window/EndpointEditBase.js \ - window/NotificationFilterEdit.js \ + window/NotificationMatcherEdit.js \ window/FileBrowser.js \ window/AuthEditBase.js \ window/AuthEditOpenId.js\ diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js index bb4ef85..f447db4 100644 --- a/src/data/model/NotificationConfig.js +++ b/src/data/model/NotificationConfig.js @@ -7,7 +7,7 @@ Ext.define('proxmox-notification-endpoints', { idProperty: 'name', }); -Ext.define('proxmox-notification-filters', { +Ext.define('proxmox-notification-matchers', { extend: 'Ext.data.Model', fields: ['name', 'comment'], proxy: { diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js index ba98395..ecf764d 100644 --- a/src/panel/NotificationConfigView.js +++ b/src/panel/NotificationConfigView.js @@ -21,7 +21,7 @@ Ext.define('Proxmox.panel.NotificationConfigView', { border: false, collapsible: true, animCollapse: false, - xtype: 'pmxNotificationFilterView', + xtype: 'pmxNotificationMatcherView', cbind: { baseUrl: '{baseUrl}', }, @@ -209,21 +209,21 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { }, }); -Ext.define('Proxmox.panel.NotificationFilterView', { +Ext.define('Proxmox.panel.NotificationMatcherView', { extend: 'Ext.grid.Panel', -alias: 'widget.pmxNotificationFilterView', +alias: 'widget.pmxNotificationMatcherView', -title: gettext('Notification Filters'), +title: gettext('Notification Matchers'), controller: { xclass: 'Ext.app.ViewController', - openEditWindow: function(filter) { + openEditWindow: function(matcher) { let me = this; - Ext.create('Proxmox.window.NotificationFilterEdit', { + Ext.create('Proxmox.window.NotificationMatcherEdit', { baseUrl: me.getView().baseUrl, - name: filter, + name: matcher, autoShow: true, listeners: { destroy: () => me.reload(), @@ -253,12 +253,12 @@ Ext.define('Proxmox.panel.NotificationFilterView', { activate: 'reload', }, -emptyText: gettext('No notification filters configured'), +emptyText: gettext('No notification matchers configured'), columns: [ { dataIndex: 'name', - text: gettext('Filter Name'), + text: gettext('Matcher Name'), renderer: Ext.String.htmlEncode, flex: 1, }, @@ -276,8 +276,8 @@ Ext.define('Proxmox.panel.NotificationFilterView', { autoDestroyRstore: true, rstore: { type: 'update', - storeid: 'proxmox-notification-filters', - model: 'proxmox-notification-filters', + storeid: 'proxmox-notification-matchers', + model: 'proxmox-notification-matchers', autoStart: true, }, sorters: 'name', @@ -307,12 +307,12 @@ Ext.define('Proxmox.panel.NotificationFilterView', { { xtype: 'proxmoxStdRemoveButton', callback: 'reload', - baseurl: `${me.baseUrl}/filters`, + baseurl: `${me.baseUrl}/matchers`, }, ], }); me.callParent(); - me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/filters`); + me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/matchers`); }, }); diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationMatcherEdit.js similarity index 92% rename from src/window/NotificationFilterEdit.js rename to src/window/NotificationMatcherEdit.js index bcde4fa..a014f3e 100644 --- a/src/window/NotificationFilterEdit
[pve-devel] [PATCH v2 pve-manager 31/52] ui: vzdump: remove left-overs from target/policy based notifications
Signed-off-by: Lukas Wagner --- www/manager6/dc/Backup.js | 81 --- .../form/NotificationPolicySelector.js| 1 - www/manager6/window/Backup.js | 35 +--- 3 files changed, 15 insertions(+), 102 deletions(-) diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js index 0c8d2d4f..e1c76a1d 100644 --- a/www/manager6/dc/Backup.js +++ b/www/manager6/dc/Backup.js @@ -36,29 +36,11 @@ Ext.define('PVE.dc.BackupEdit', { delete values.node; } - if (!isCreate) { - // 'mailnotification' is deprecated in favor of 'notification-policy' - // -> Migration to the new parameter happens in init, so we are - //safe to remove the old parameter here. - Proxmox.Utils.assemble_field_data(values, { 'delete': 'mailnotification' }); - - // If sending notifications via mail, remove the current value of - // 'notification-target' - if (values['notification-mode'] === "mailto") { - Proxmox.Utils.assemble_field_data( - values, - { 'delete': 'notification-target' }, - ); - } else { - // and vice versa... - Proxmox.Utils.assemble_field_data( - values, - { 'delete': 'mailto' }, - ); - } - } - - delete values['notification-mode']; + // Get rid of new-old parameters for notification settings. + // These should only be set for those selected few who ran + // pve-manager from pvetest. + Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-policy' }); + Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-target' }); if (!values.id && isCreate) { values.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13); @@ -170,20 +152,14 @@ Ext.define('PVE.dc.BackupEdit', { success: function(response, _options) { let data = response.result.data; - // 'mailnotification' is deprecated. Let's automatically - // migrate to the compatible 'notification-policy' parameter - if (data.mailnotification) { - if (!data["notification-policy"]) { - data["notification-policy"] = data.mailnotification; - } - - delete data.mailnotification; - } - - if (data['notification-target']) { - data['notification-mode'] = 'notification-target'; - } else if (data.mailto) { - data['notification-mode'] = 'mailto'; + // Migrate 'new'-old notification-policy back to + // old-old mailnotification. Only should affect + // users who used pve-manager from pvetest. + // This was a remnant of notifications before the + // overhaul. + let policy = data['notification-policy']; + if (policy === 'always' || policy === 'failure') { + data.mailnotification = policy; } if (data.exclude) { @@ -228,7 +204,6 @@ Ext.define('PVE.dc.BackupEdit', { viewModel: { data: { selMode: 'include', - notificationMode: 'notification-target', }, formulas: { @@ -327,44 +302,16 @@ Ext.define('PVE.dc.BackupEdit', { { xtype: 'pveEmailNotificationSelector', fieldLabel: gettext('Notify'), - name: 'notification-policy', + name: 'mailnotification', cbind: { value: (get) => get('isCreate') ? 'always' : '', deleteEmpty: '{!isCreate}', }, }, - { - xtype:
[pve-devel] [PATCH v2 pve-manager 34/52] api: notification: add disable and origin params
'disable' can be set to disable a matcher/target. 'origin' signals whether the configuration entry was created by the user or whether it was built-in/ built-in-and-modified. Signed-off-by: Lukas Wagner --- PVE/API2/Cluster/Notifications.pm | 113 ++ 1 file changed, 99 insertions(+), 14 deletions(-) diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm index 42207aaa..27e3a66d 100644 --- a/PVE/API2/Cluster/Notifications.pm +++ b/PVE/API2/Cluster/Notifications.pm @@ -164,8 +164,19 @@ __PACKAGE__->register_method ({ }, 'comment' => { description => 'Comment', - type=> 'string', - optional=> 1, + type => 'string', + optional => 1, + }, + 'disable' => { + description => 'Show if this target is disabled', + type => 'boolean', + optional => 1, + default => 0, + }, + 'origin' => { + description => 'Show if this entry was created by a user or was built-in', + type => 'string', + enum => [qw(user-created builtin modified-builtin)], }, }, }, @@ -183,6 +194,8 @@ __PACKAGE__->register_method ({ name => $target->{name}, comment => $target->{comment}, type => 'sendmail', + disable => $target->{disable}, + origin => $target->{origin}, }; } @@ -191,6 +204,8 @@ __PACKAGE__->register_method ({ name => $target->{name}, comment => $target->{comment}, type => 'gotify', + disable => $target->{disable}, + origin => $target->{origin}, }; } @@ -199,6 +214,8 @@ __PACKAGE__->register_method ({ name => $target->{name}, comment => $target->{comment}, type => 'smtp', + disable => $target->{disable}, + origin => $target->{origin}, }; } @@ -295,8 +312,14 @@ my $sendmail_properties = { }, 'comment' => { description => 'Comment', - type=> 'string', - optional=> 1, + type => 'string', + optional => 1, +}, +'disable' => { + description => 'Disable this target', + type => 'boolean', + optional => 1, + default => 0, }, }; @@ -319,7 +342,14 @@ __PACKAGE__->register_method ({ type => 'array', items => { type => 'object', - properties => $sendmail_properties, + properties => { + %$sendmail_properties, + 'origin' => { + description => 'Show if this entry was created by a user or was built-in', + type => 'string', + enum => [qw(user-created builtin modified-builtin)], + }, + }, }, links => [ { rel => 'child', href => '{name}' } ], }, @@ -404,6 +434,7 @@ __PACKAGE__->register_method ({ my $from_address = extract_param($param, 'from-address'); my $author = extract_param($param, 'author'); my $comment = extract_param($param, 'comment'); + my $disable = extract_param($param, 'disable'); eval { PVE::Notify::lock_config(sub { @@ -416,6 +447,7 @@ __PACKAGE__->register_method ({ $from_address, $author, $comment, + $disable, ); PVE::Notify::write_config($config); @@ -463,6 +495,7 @@ __PACKAGE__->register_method ({ my $from_address = extract_param($param, 'from-address'); my $author = extract_param($param, 'author'); my $comment = extract_param($param, 'comment'); + my $disable = extract_param($param, 'disable'); my $delete = extract_param($param, 'delete'); my $digest = extract_param($param, 'digest'); @@ -478,6 +511,7 @@ __PACKAGE__->register_method ({ $from_address,
[pve-devel] [PATCH v2 proxmox 13/52] notify: add api for smtp endpoints
Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/mod.rs| 33 +++ proxmox-notify/src/api/smtp.rs | 356 +++ proxmox-notify/src/endpoints/smtp.rs | 8 - 3 files changed, 389 insertions(+), 8 deletions(-) create mode 100644 proxmox-notify/src/api/smtp.rs diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs index 8042157..762d448 100644 --- a/proxmox-notify/src/api/mod.rs +++ b/proxmox-notify/src/api/mod.rs @@ -1,3 +1,4 @@ +use serde::Serialize; use std::collections::HashSet; use proxmox_http_error::HttpError; @@ -10,6 +11,8 @@ pub mod gotify; pub mod matcher; #[cfg(feature = "sendmail")] pub mod sendmail; +#[cfg(feature = "smtp")] +pub mod smtp; // We have our own, local versions of http_err and http_bail, because // we don't want to wrap the error in anyhow::Error. If we were to do that, @@ -60,6 +63,10 @@ fn ensure_endpoint_exists(#[allow(unused)] config: &Config, name: &str) -> Resul { exists = exists || gotify::get_endpoint(config, name).is_ok(); } +#[cfg(feature = "smtp")] +{ +exists = exists || smtp::get_endpoint(config, name).is_ok(); +} if !exists { http_bail!(NOT_FOUND, "endpoint '{name}' does not exist") @@ -100,6 +107,7 @@ fn get_referrers(config: &Config, entity: &str) -> Result, HttpE } } } + Ok(referrers) } @@ -148,6 +156,31 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet { expanded } +#[allow(unused)] +fn set_private_config_entry( +config: &mut Config, +private_config: &T, +typename: &str, +name: &str, +) -> Result<(), HttpError> { +config +.private_config +.set_data(name, typename, private_config) +.map_err(|e| { +http_err!( +INTERNAL_SERVER_ERROR, +"could not save private config for endpoint '{}': {e}", +name +) +}) +} + +#[allow(unused)] +fn remove_private_config_entry(config: &mut Config, name: &str) -> Result<(), HttpError> { +config.private_config.sections.remove(name); +Ok(()) +} + #[cfg(test)] mod test_helpers { use crate::Config; diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs new file mode 100644 index 000..bd9d7bb --- /dev/null +++ b/proxmox-notify/src/api/smtp.rs @@ -0,0 +1,356 @@ +use proxmox_http_error::HttpError; + +use crate::api::{http_bail, http_err}; +use crate::endpoints::smtp::{ +DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig, +SmtpPrivateConfigUpdater, SMTP_TYPENAME, +}; +use crate::Config; + +/// Get a list of all smtp endpoints. +/// +/// The caller is responsible for any needed permission checks. +/// Returns a list of all smtp endpoints or a `HttpError` if the config is +/// erroneous (`500 Internal server error`). +pub fn get_endpoints(config: &Config) -> Result, HttpError> { +config +.config +.convert_to_typed_array(SMTP_TYPENAME) +.map_err(|e| http_err!(NOT_FOUND, "Could not fetch endpoints: {e}")) +} + +/// Get smtp endpoint with given `name`. +/// +/// The caller is responsible for any needed permission checks. +/// Returns the endpoint or a `HttpError` if the endpoint was not found (`404 Not found`). +pub fn get_endpoint(config: &Config, name: &str) -> Result { +config +.config +.lookup(SMTP_TYPENAME, name) +.map_err(|_| http_err!(NOT_FOUND, "endpoint '{name}' not found")) +} + +/// Add a new smtp endpoint. +/// +/// The caller is responsible for any needed permission checks. +/// The caller also responsible for locking the configuration files. +/// Returns a `HttpError` if: +/// - an entity with the same name already exists (`400 Bad request`) +/// - the configuration could not be saved (`500 Internal server error`) +/// - mailto *and* mailto_user are both set to `None` +pub fn add_endpoint( +config: &mut Config, +endpoint_config: &SmtpConfig, +private_endpoint_config: &SmtpPrivateConfig, +) -> Result<(), HttpError> { +if endpoint_config.name != private_endpoint_config.name { +// Programming error by the user of the crate, thus we panic +panic!("name for endpoint config and private config must be identical"); +} + +super::ensure_unique(config, &endpoint_config.name)?; + +if endpoint_config.mailto.is_none() && endpoint_config.mailto_user.is_none() { +http_bail!( +BAD_REQUEST, +"must at least provide one recipient, either in mailto or in mailto-user" +); +} + +super::set_private_config_entry( +config, +private_endpoint_config, +
[pve-devel] [PATCH v2 debcargo-conf 02/52] update lettre to 0.11.1
Signed-off-by: Lukas Wagner --- src/lettre/debian/changelog | 10 +++ .../debian/patches/downgrade_fastrand.patch | 13 .../debian/patches/downgrade_idna.patch | 13 src/lettre/debian/patches/downgrade_url.patch | 13 .../patches/remove_unused_features.patch | 69 ++- src/lettre/debian/patches/series | 4 +- .../patches/upgrade_quoted_printable.patch| 13 7 files changed, 88 insertions(+), 47 deletions(-) create mode 100644 src/lettre/debian/patches/downgrade_fastrand.patch create mode 100644 src/lettre/debian/patches/downgrade_idna.patch create mode 100644 src/lettre/debian/patches/downgrade_url.patch delete mode 100644 src/lettre/debian/patches/upgrade_quoted_printable.patch diff --git a/src/lettre/debian/changelog b/src/lettre/debian/changelog index d49cbb042..e92c5c070 100644 --- a/src/lettre/debian/changelog +++ b/src/lettre/debian/changelog @@ -1,3 +1,13 @@ +rust-lettre (0.11.1-1) UNRELEASED-FIXME-AUTOGENERATED-DEBCARGO; urgency=medium + + * Package lettre 0.11.1 from crates.io using debcargo 2.6.0 + * Downgrade fastrand from 2.0 to 1.8 + * Downgrade idna from 0.4 to 0.3 + * Downgrade url from 2.4 to 2.3 + * Drop patch that upgrades quoted_printable + + -- Lukas Wagner Wed, 08 Nov 2023 13:32:49 +0100 + rust-lettre (0.10.4-1~bpo12+pve1) proxmox-rust; urgency=medium * Rebuild for Debian Bookworm / Proxmox diff --git a/src/lettre/debian/patches/downgrade_fastrand.patch b/src/lettre/debian/patches/downgrade_fastrand.patch new file mode 100644 index 0..975efeb1c --- /dev/null +++ b/src/lettre/debian/patches/downgrade_fastrand.patch @@ -0,0 +1,13 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 072ea3a..5decb37 100644 +--- a/Cargo.toml b/Cargo.toml +@@ -150,7 +150,7 @@ version = "0.2.1" + default-features = false + + [dependencies.fastrand] +-version = "2.0" ++version = "1.8" + optional = true + + [dependencies.futures-io] diff --git a/src/lettre/debian/patches/downgrade_idna.patch b/src/lettre/debian/patches/downgrade_idna.patch new file mode 100644 index 0..1cfaaa26c --- /dev/null +++ b/src/lettre/debian/patches/downgrade_idna.patch @@ -0,0 +1,13 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 5decb37..09d2b9b 100644 +--- a/Cargo.toml b/Cargo.toml +@@ -176,7 +176,7 @@ version = "1" + optional = true + + [dependencies.idna] +-version = "0.4" ++version = "0.3" + + [dependencies.mime] + version = "0.3.4" diff --git a/src/lettre/debian/patches/downgrade_url.patch b/src/lettre/debian/patches/downgrade_url.patch new file mode 100644 index 0..4da907540 --- /dev/null +++ b/src/lettre/debian/patches/downgrade_url.patch @@ -0,0 +1,13 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 09d2b9b..5004a3b 100644 +--- a/Cargo.toml b/Cargo.toml +@@ -237,7 +237,7 @@ optional = true + default-features = false + + [dependencies.url] +-version = "2.4" ++version = "2.3" + optional = true + + [dependencies.uuid] diff --git a/src/lettre/debian/patches/remove_unused_features.patch b/src/lettre/debian/patches/remove_unused_features.patch index 0229e41aa..7ce45be0f 100644 --- a/src/lettre/debian/patches/remove_unused_features.patch +++ b/src/lettre/debian/patches/remove_unused_features.patch @@ -1,8 +1,8 @@ diff --git a/Cargo.toml b/Cargo.toml -index 13c34b6..b4413b6 100644 +index 13e3b77..072ea3a 100644 --- a/Cargo.toml +++ b/Cargo.toml -@@ -114,32 +114,10 @@ required-features = [ +@@ -114,24 +114,6 @@ required-features = [ "builder", ] @@ -27,6 +27,9 @@ index 13c34b6..b4413b6 100644 [[bench]] name = "transport_smtp" harness = false +@@ -140,10 +122,6 @@ harness = false + name = "mailbox_parsing" + harness = false -[dependencies.async-std] -version = "1.8" @@ -35,8 +38,8 @@ index 13c34b6..b4413b6 100644 [dependencies.async-trait] version = "0.1" optional = true -@@ -217,19 +195,6 @@ optional = true - version = "0.8" +@@ -224,19 +202,6 @@ optional = true + version = "0.9" optional = true -[dependencies.rustls] @@ -55,19 +58,19 @@ index 13c34b6..b4413b6 100644 [dependencies.serde] version = "1" features = ["derive"] -@@ -248,11 +213,6 @@ optional = true - version = "0.4.4" +@@ -255,11 +220,6 @@ optional = true + version = "0.5.1" optional = true -[dependencies.tokio1_boring] --version = "2.1.4" +-version = "3" -optional = true -package = "tokio-boring" - [dependencies.tokio1_crate] version = "1" optional = true -@@ -263,11 +223,6 @@ version = "0.3" +@@ -270,11 +230,6 @@ version = "0.3" optional = true package = "tokio-native-tls" @@ -79,8 +82,8 @@ index 13c34b6..b4413b6 100644 [dependencies.tracing] version = "0.1.16" features = [&qu
[pve-devel] [PATCH v2 proxmox 09/52] sys: email: add `forward`
This new function forwards an email to new recipients. Signed-off-by: Lukas Wagner --- proxmox-sys/src/email.rs | 52 +++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/proxmox-sys/src/email.rs b/proxmox-sys/src/email.rs index 8b3a1b6..c94f634 100644 --- a/proxmox-sys/src/email.rs +++ b/proxmox-sys/src/email.rs @@ -3,7 +3,7 @@ use std::io::Write; use std::process::{Command, Stdio}; -use anyhow::{bail, Error}; +use anyhow::{bail, format_err, Error}; /// Sends multi-part mail with text and/or html to a list of recipients /// @@ -110,6 +110,56 @@ pub fn sendmail( Ok(()) } +/// Forwards an email message to a given list of recipients. +/// +/// ``sendmail`` is used for sending the mail, thus `message` must be +/// compatible with that (the message is piped into stdin unmodified). +pub fn forward( +mailto: &[&str], +mailfrom: &str, +message: &[u8], +uid: Option, +) -> Result<(), Error> { +use std::os::unix::process::CommandExt; + +if mailto.is_empty() { +bail!("At least one recipient has to be specified!") +} + +let mut builder = Command::new("/usr/sbin/sendmail"); + +builder +.args([ +"-N", "never", // never send DSN (avoid mail loops) +"-f", mailfrom, "--", +]) +.args(mailto) +.stdin(Stdio::piped()) +.stdout(Stdio::null()) +.stderr(Stdio::null()); + +if let Some(uid) = uid { +builder.uid(uid); +} + +let mut process = builder +.spawn() +.map_err(|err| format_err!("could not spawn sendmail process: {err}"))?; + +process +.stdin +.take() +.unwrap() +.write_all(message) +.map_err(|err| format_err!("couldn't write to sendmail stdin: {err}"))?; + +process +.wait() +.map_err(|err| format_err!("sendmail did not exit successfully: {err}"))?; + +Ok(()) +} + #[cfg(test)] mod test { use crate::email::sendmail; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-perl-rs 20/52] notify: support 'origin' paramter
This parameter shows the origin of a config entry (builtin, user-created, modified-builtin) Signed-off-by: Lukas Wagner --- common/src/notify.rs | 5 + 1 file changed, 5 insertions(+) diff --git a/common/src/notify.rs b/common/src/notify.rs index a5ab754..8f9f38f 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@ -161,6 +161,8 @@ mod export { author, comment, disable, +filter: None, +origin: None, }, ) } @@ -242,6 +244,7 @@ mod export { comment, disable, filter: None, +origin: None, }, &GotifyPrivateConfig { name, token }, ) @@ -334,6 +337,7 @@ mod export { author, comment, disable, +origin: None, }, &SmtpPrivateConfig { name, password }, ) @@ -435,6 +439,7 @@ mod export { invert_match, comment, disable, +origin: None, }, ) } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox 07/52] notify: matcher: introduce common trait for match directives
This allows us to make the match-checking code a bit shorter. Signed-off-by: Lukas Wagner --- proxmox-notify/src/matcher.rs | 92 +-- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index b03d11d..e299fd0 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -142,6 +142,11 @@ pub struct MatcherConfig { pub comment: Option, } +trait MatchDirective { +fn matches(&self, notification: &Notification) -> Result; +} + +/// Check if the notification metadata fields match #[derive(Clone, Debug)] pub enum FieldMatcher { Exact { @@ -157,9 +162,9 @@ pub enum FieldMatcher { proxmox_serde::forward_deserialize_to_from_str!(FieldMatcher); proxmox_serde::forward_serialize_to_display!(FieldMatcher); -impl FieldMatcher { -fn matches(&self, notification: &Notification) -> bool { -match self { +impl MatchDirective for FieldMatcher { +fn matches(&self, notification: &Notification) -> Result { +Ok(match self { FieldMatcher::Exact { field, matched_value, @@ -186,7 +191,7 @@ impl FieldMatcher { false } } -} +}) } } @@ -260,9 +265,22 @@ impl MatcherConfig { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); -is_match = mode.apply(is_match, self.check_severity_match(notification)); -is_match = mode.apply(is_match, self.check_field_match(notification)?); -is_match = mode.apply(is_match, self.check_calendar_match(notification)?); + +if let Some(severity_matchers) = self.match_severity.as_deref() { +is_match = mode.apply( +is_match, +self.check_matches(notification, severity_matchers)?, +); +} +if let Some(field_matchers) = self.match_field.as_deref() { +is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?); +} +if let Some(calendar_matchers) = self.match_calendar.as_deref() { +is_match = mode.apply( +is_match, +self.check_matches(notification, calendar_matchers)?, +); +} let invert_match = self.invert_match.unwrap_or_default(); @@ -273,46 +291,24 @@ impl MatcherConfig { }) } -fn check_field_match(&self, notification: &Notification) -> Result { -let mode = self.mode.unwrap_or_default(); -let mut is_match = mode.neutral_element(); - -if let Some(match_field) = self.match_field.as_deref() { -for field_matcher in match_field { -// let field_matcher: FieldMatcher = match_stmt.parse()?; -is_match = mode.apply(is_match, field_matcher.matches(notification)); -} -} - -Ok(is_match) -} - -fn check_severity_match(&self, notification: &Notification) -> bool { +/// Check if given `MatchDirectives` match a notification. +fn check_matches( +&self, +notification: &Notification, +matchers: &[impl MatchDirective], +) -> Result { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); -if let Some(matchers) = self.match_severity.as_ref() { -for severity_matcher in matchers { -is_match = mode.apply(is_match, severity_matcher.matches(notification)); -} -} - -is_match -} - -fn check_calendar_match(&self, notification: &Notification) -> Result { -let mode = self.mode.unwrap_or_default(); -let mut is_match = mode.neutral_element(); - -if let Some(matchers) = self.match_calendar.as_ref() { -for matcher in matchers { -is_match = mode.apply(is_match, matcher.matches(notification)?); -} +for field_matcher in matchers { +is_match = mode.apply(is_match, field_matcher.matches(notification)?); } Ok(is_match) } } + +/// Match severity of the notification. #[derive(Clone, Debug)] pub struct SeverityMatcher { severities: Vec, @@ -321,9 +317,11 @@ pub struct SeverityMatcher { proxmox_serde::forward_deserialize_to_from_str!(SeverityMatcher); proxmox_serde::forward_serialize_to_display!(SeverityMatcher); -impl SeverityMatcher { -fn matches(&self, notification: &Notification) -> bool { -self.severities.contains(¬ification.metadata.severity) +/// Common trait implemented by all matching directives +impl MatchDirective for SeverityMatcher { +/// Check if this directive matches a given notification +fn matches(&self, notification: &Notification) -> Result {
[pve-devel] [PATCH v2 proxmox-widget-toolkit 43/52] panel: notification: add gui for SMTP endpoints
This new endpoint configuration panel is embedded in the existing EndpointEditBase dialog window. This commit also factors out some of the non-trivial common form elements that are shared between the new panel and the already existing SendmailEditPanel into a separate panel EmailRecipientPanel. Signed-off-by: Lukas Wagner --- src/Makefile | 2 + src/Schema.js| 5 + src/panel/EmailRecipientPanel.js | 88 +++ src/panel/SendmailEditPanel.js | 58 +- src/panel/SmtpEditPanel.js | 183 +++ 5 files changed, 281 insertions(+), 55 deletions(-) create mode 100644 src/panel/EmailRecipientPanel.js create mode 100644 src/panel/SmtpEditPanel.js diff --git a/src/Makefile b/src/Makefile index c6d31c3..01145b1 100644 --- a/src/Makefile +++ b/src/Makefile @@ -71,7 +71,9 @@ JSSRC=\ panel/ACMEAccount.js\ panel/ACMEPlugin.js \ panel/ACMEDomains.js\ + panel/EmailRecipientPanel.js\ panel/SendmailEditPanel.js \ + panel/SmtpEditPanel.js \ panel/StatusView.js \ panel/TfaView.js\ panel/NotesView.js \ diff --git a/src/Schema.js b/src/Schema.js index 37ecd88..28e1037 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -43,6 +43,11 @@ Ext.define('Proxmox.Schema', { // a singleton ipanel: 'pmxSendmailEditPanel', iconCls: 'fa-envelope-o', }, + smtp: { + name: gettext('SMTP'), + ipanel: 'pmxSmtpEditPanel', + iconCls: 'fa-envelope-o', + }, gotify: { name: gettext('Gotify'), ipanel: 'pmxGotifyEditPanel', diff --git a/src/panel/EmailRecipientPanel.js b/src/panel/EmailRecipientPanel.js new file mode 100644 index 000..b2bc03c --- /dev/null +++ b/src/panel/EmailRecipientPanel.js @@ -0,0 +1,88 @@ +Ext.define('Proxmox.panel.EmailRecipientPanel', { +extend: 'Ext.panel.Panel', +xtype: 'pmxEmailRecipientPanel', +mixins: ['Proxmox.Mixin.CBind'], +border: false, + +mailValidator: function() { + let mailto_user = this.down(`[name=mailto-user]`); + let mailto = this.down(`[name=mailto]`); + + if (!mailto_user.getValue()?.length && !mailto.getValue()) { + return gettext('Either mailto or mailto-user must be set'); + } + + return true; +}, + +items: [ + { + layout: 'anchor', + border: false, + items: [ + { + xtype: 'pmxUserSelector', + name: 'mailto-user', + multiSelect: true, + allowBlank: true, + editable: false, + skipEmptyText: true, + fieldLabel: gettext('Recipient(s)'), + cbind: { + deleteEmpty: '{!isCreate}', + }, + validator: function() { + return this.up('pmxEmailRecipientPanel').mailValidator(); + }, + autoEl: { + tag: 'div', + 'data-qtip': gettext('The notification will be sent to the user\'s configured mail address'), + }, + listConfig: { + width: 600, + columns: [ + { + header: gettext('User'), + sortable: true, + dataIndex: 'userid', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + { + header: gettext('E-Mail'), + sortable: true, + dataIndex: 'email', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + { + header: gettext('Comment'), + sortable: false, + dataIndex: 'comment', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + ], + }, + }, + { + xtype: 'proxmoxtextfield', + fieldLabel: gettext('Additional Recip
[pve-devel] [PATCH v2 pve-guest-common 22/52] vzdump: deprecate mailto/mailnotification/notification-{target, policy}
The first two will be migrated to the notification system, the second were part for the first attempt for the new notification system. The first attempt only ever hit pvetest, so we simply tell the user to not use the two params. Signed-off-by: Lukas Wagner --- src/PVE/VZDump/Common.pm | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm index be605af..b93ad86 100644 --- a/src/PVE/VZDump/Common.pm +++ b/src/PVE/VZDump/Common.pm @@ -175,21 +175,22 @@ my $confdesc = { mailto => { type => 'string', format => 'email-or-username-list', - description => "Comma-separated list of email addresses or users that should" . - " receive email notifications. Has no effect if the 'notification-target' option " . - " is set at the same time.", + description => "Deprecated: Use notification targets/matchers instead." . + " Comma-separated list of email addresses or users that should" . + " receive email notifications.", optional => 1, }, mailnotification => { type => 'string', - description => "Deprecated: use 'notification-policy' instead.", + description => "Deprecated: use notification targets/matchers instead." . + " Specify when to send a notification mail", optional => 1, enum => [ 'always', 'failure' ], default => 'always', }, 'notification-policy' => { type => 'string', - description => "Specify when to send a notification", + description => "Deprecated: Do not use", optional => 1, enum => [ 'always', 'failure', 'never'], default => 'always', @@ -197,10 +198,7 @@ my $confdesc = { 'notification-target' => { type => 'string', format => 'pve-configid', - description => "Determine the target to which notifications should be sent." . - " Can either be a notification endpoint or a notification group." . - " This option takes precedence over 'mailto', meaning that if both are " . - " set, the 'mailto' option will be ignored.", + description => "Deprecated: Do not use", optional => 1, }, tmpdir => { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-perl-rs 19/52] notify: add 'disable' parameter
This parameter disables a matcher/a target. Signed-off-by: Lukas Wagner --- common/src/notify.rs | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/common/src/notify.rs b/common/src/notify.rs index 8a6d76e..a5ab754 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@ -147,6 +147,7 @@ mod export { from_address: Option, author: Option, comment: Option, +disable: Option, ) -> Result<(), HttpError> { let mut config = this.config.lock().unwrap(); @@ -159,7 +160,7 @@ mod export { from_address, author, comment, -filter: None, +disable, }, ) } @@ -174,6 +175,7 @@ mod export { from_address: Option, author: Option, comment: Option, +disable: Option, delete: Option>, digest: Option<&str>, ) -> Result<(), HttpError> { @@ -189,6 +191,7 @@ mod export { from_address, author, comment, +disable, }, delete.as_deref(), digest.as_deref(), @@ -228,6 +231,7 @@ mod export { server: String, token: String, comment: Option, +disable: Option, ) -> Result<(), HttpError> { let mut config = this.config.lock().unwrap(); api::gotify::add_endpoint( @@ -236,6 +240,7 @@ mod export { name: name.clone(), server, comment, +disable, filter: None, }, &GotifyPrivateConfig { name, token }, @@ -250,6 +255,7 @@ mod export { server: Option, token: Option, comment: Option, +disable: Option, delete: Option>, digest: Option<&str>, ) -> Result<(), HttpError> { @@ -259,7 +265,11 @@ mod export { api::gotify::update_endpoint( &mut config, name, -&GotifyConfigUpdater { server, comment }, +&GotifyConfigUpdater { +server, +comment, +disable, +}, &GotifyPrivateConfigUpdater { token }, delete.as_deref(), digest.as_deref(), @@ -307,6 +317,7 @@ mod export { from_address: String, author: Option, comment: Option, +disable: Option, ) -> Result<(), HttpError> { let mut config = this.config.lock().unwrap(); api::smtp::add_endpoint( @@ -322,6 +333,7 @@ mod export { from_address, author, comment, +disable, }, &SmtpPrivateConfig { name, password }, ) @@ -342,6 +354,7 @@ mod export { from_address: Option, author: Option, comment: Option, +disable: Option, delete: Option>, digest: Option<&str>, ) -> Result<(), HttpError> { @@ -361,6 +374,7 @@ mod export { from_address, author, comment, +disable, }, &SmtpPrivateConfigUpdater { password }, delete.as_deref(), @@ -406,6 +420,7 @@ mod export { mode: Option, invert_match: Option, comment: Option, +disable: Option, ) -> Result<(), HttpError> { let mut config = this.config.lock().unwrap(); api::matcher::add_matcher( @@ -419,6 +434,7 @@ mod export { mode, invert_match, comment, +disable, }, ) } @@ -435,6 +451,7 @@ mod export { mode: Option, invert_match: Option, comment: Option, +disable: Option, delete: Option>, digest: Option<&str>, ) -> Result<(), HttpError> { @@ -452,6 +469,7 @@ mod export { mode, invert_match, comment, +disable, }, delete.as_deref(), digest.as_deref(), -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-perl-rs 17/52] notify: add bindings for smtp API calls
Signed-off-by: Lukas Wagner --- common/src/notify.rs | 106 +++ 1 file changed, 106 insertions(+) diff --git a/common/src/notify.rs b/common/src/notify.rs index 4fbd705..8a6d76e 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@ -15,6 +15,10 @@ mod export { use proxmox_notify::endpoints::sendmail::{ DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater, }; +use proxmox_notify::endpoints::smtp::{ +DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig, +SmtpPrivateConfigUpdater, +}; use proxmox_notify::matcher::{ CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig, MatcherConfigUpdater, SeverityMatcher, @@ -271,6 +275,108 @@ mod export { api::gotify::delete_gotify_endpoint(&mut config, name) } +#[export(serialize_error)] +fn get_smtp_endpoints( +#[try_from_ref] this: &NotificationConfig, +) -> Result, HttpError> { +let config = this.config.lock().unwrap(); +api::smtp::get_endpoints(&config) +} + +#[export(serialize_error)] +fn get_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +id: &str, +) -> Result { +let config = this.config.lock().unwrap(); +api::smtp::get_endpoint(&config, id) +} + +#[export(serialize_error)] +#[allow(clippy::too_many_arguments)] +fn add_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +name: String, +server: String, +port: Option, +mode: Option, +username: Option, +password: Option, +mailto: Option>, +mailto_user: Option>, +from_address: String, +author: Option, +comment: Option, +) -> Result<(), HttpError> { +let mut config = this.config.lock().unwrap(); +api::smtp::add_endpoint( +&mut config, +&SmtpConfig { +name: name.clone(), +server, +port, +mode, +username, +mailto, +mailto_user, +from_address, +author, +comment, +}, +&SmtpPrivateConfig { name, password }, +) +} + +#[export(serialize_error)] +#[allow(clippy::too_many_arguments)] +fn update_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +name: &str, +server: Option, +port: Option, +mode: Option, +username: Option, +password: Option, +mailto: Option>, +mailto_user: Option>, +from_address: Option, +author: Option, +comment: Option, +delete: Option>, +digest: Option<&str>, +) -> Result<(), HttpError> { +let mut config = this.config.lock().unwrap(); +let digest = decode_digest(digest)?; + +api::smtp::update_endpoint( +&mut config, +name, +&SmtpConfigUpdater { +server, +port, +mode, +username, +mailto, +mailto_user, +from_address, +author, +comment, +}, +&SmtpPrivateConfigUpdater { password }, +delete.as_deref(), +digest.as_deref(), +) +} + +#[export(serialize_error)] +fn delete_smtp_endpoint( +#[try_from_ref] this: &NotificationConfig, +name: &str, +) -> Result<(), HttpError> { +let mut config = this.config.lock().unwrap(); +api::smtp::delete_endpoint(&mut config, name) +} + #[export(serialize_error)] fn get_matchers( #[try_from_ref] this: &NotificationConfig, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 pve-cluster 21/52] notify: adapt to matcher based notification system
This commit removes the target paramters from all notify calls. Also, the default 'mail-to-root' target is not added automatically any more - this target will be added by an dpkg hook in the future. Signed-off-by: Lukas Wagner --- src/PVE/Notify.pm | 101 +- 1 file changed, 47 insertions(+), 54 deletions(-) diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm index 419bf6d..872eb25 100644 --- a/src/PVE/Notify.pm +++ b/src/PVE/Notify.pm @@ -18,8 +18,6 @@ cfs_register_file( \&write_notification_config, ); -my $mail_to_root_target = 'mail-to-root'; - sub parse_notification_config { my ($filename, $raw) = @_; @@ -48,86 +46,81 @@ sub read_config { my $notification_config = Proxmox::RS::Notify->parse_config($config, $priv_config); -eval { - # This target should always be available... - $notification_config->add_sendmail_endpoint( - $mail_to_root_target, - undef, - ['root@pam'], - undef, - undef, - 'Send mail to root@pam\'s email address' - ); -}; - return $notification_config; } sub write_config { my ($notification_config) = @_; -eval { - # ... but don't persist it to the config. - # Rationale: If it is in the config, the user might think - # that it can be changed by editing the configuration there. - # However, since we always add it in `read_config`, any changes - # will be implicitly overridden by the default. - - # If users want's to change the configuration, they are supposed to - # create a new sendmail endpoint. - $notification_config->delete_sendmail_endpoint($mail_to_root_target); -}; - my ($config, $priv_config) = $notification_config->write_config(); cfs_write_file('notifications.cfg', $config, 1); cfs_write_file('priv/notifications.cfg', $priv_config, 1); } -sub default_target { -return $mail_to_root_target; -} - my $send_notification = sub { -my ($target, $severity, $title, $message, $properties, $config) = @_; +my ($severity, $title, $message, $template_data, $fields, $config) = @_; $config = read_config() if !defined($config); -my ($module, $file, $line) = caller(1); - -# Augment properties with the source code location of the notify call -my $props_with_source = { - %$properties, - source => { - module => $module, - file => $file, - line => $line, - } -}; - -$config->send($target, $severity, $title, $message, $props_with_source); +$config->send($severity, $title, $message, $template_data, $fields); }; sub notify { -my ($target, $severity, $title, $message, $properties, $config) = @_; -$send_notification->($target, $severity, $title, $message, $properties, $config); +my ($severity, $title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +$severity, +$title, +$message, +$template_data, +$fields, +$config +); } sub info { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'info', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'info', +$title, +$message, +$template_data, +$fields, +$config +); } sub notice { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'notice', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'notice', +$title, +$message, +$template_data, +$fields, +$config +); } sub warning { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'warning', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'warning', +$title, +$message, +$template_data, +$fields, +$config +); } sub error { -my ($target, $title, $message, $properties, $config) = @_; -$send_notification->($target, 'error', $title, $message, $properties, $config); +my ($title, $message, $template_data, $fields, $config) = @_; +$send_notification->( +'error', +$title, +$message, +$template_data, +$fields, +$config +); } sub check_may_use_target { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-perl-rs 18/52] pve-rs: notify: remove notify_context for PVE
The context has now been moved to `proxmox-notify` due to the fact that we also need it in `proxmox-mail-forward` now. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - No changes pve-rs/Cargo.toml| 2 +- pve-rs/src/lib.rs| 7 ++- pve-rs/src/notify_context.rs | 117 --- 3 files changed, 5 insertions(+), 121 deletions(-) delete mode 100644 pve-rs/src/notify_context.rs diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml index e222d9d..2300c8d 100644 --- a/pve-rs/Cargo.toml +++ b/pve-rs/Cargo.toml @@ -36,7 +36,7 @@ perlmod = { version = "0.13", features = [ "exporter" ] } proxmox-apt = "0.10.6" proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] } proxmox-http-error = "0.1.0" -proxmox-notify = "0.2" +proxmox-notify = { version = "0.2", features = ["pve-context"] } proxmox-openid = "0.10" proxmox-resource-scheduling = "0.3.0" proxmox-subscription = "0.4" diff --git a/pve-rs/src/lib.rs b/pve-rs/src/lib.rs index d1915c9..42be39e 100644 --- a/pve-rs/src/lib.rs +++ b/pve-rs/src/lib.rs @@ -4,18 +4,19 @@ pub mod common; pub mod apt; -pub mod notify_context; pub mod openid; pub mod resource_scheduling; pub mod tfa; #[perlmod::package(name = "Proxmox::Lib::PVE", lib = "pve_rs")] mod export { -use crate::{common, notify_context}; +use proxmox_notify::context::pve::PVE_CONTEXT; + +use crate::common; #[export] pub fn init() { common::logger::init("PVE_LOG", "info"); -notify_context::init(); +proxmox_notify::context::set_context(&PVE_CONTEXT) } } diff --git a/pve-rs/src/notify_context.rs b/pve-rs/src/notify_context.rs deleted file mode 100644 index 3cf3e18..000 --- a/pve-rs/src/notify_context.rs +++ /dev/null @@ -1,117 +0,0 @@ -use log; -use std::path::Path; - -use proxmox_notify::context::Context; - -// Some helpers borrowed and slightly adapted from `proxmox-mail-forward` - -fn normalize_for_return(s: Option<&str>) -> Option { -match s?.trim() { -"" => None, -s => Some(s.to_string()), -} -} - -fn attempt_file_read>(path: P) -> Option { -match proxmox_sys::fs::file_read_optional_string(path) { -Ok(contents) => contents, -Err(err) => { -log::error!("{err}"); -None -} -} -} - -fn lookup_mail_address(content: &str, user: &str) -> Option { -normalize_for_return(content.lines().find_map(|line| { -let fields: Vec<&str> = line.split(':').collect(); -#[allow(clippy::get_first)] // to keep expression style consistent -match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == user { -true => fields.get(6).copied(), -false => None, -} -})) -} - -fn lookup_datacenter_config_key(content: &str, key: &str) -> Option { -let key_prefix = format!("{key}:"); -normalize_for_return( -content -.lines() -.find_map(|line| line.strip_prefix(&key_prefix)), -) -} - -#[derive(Debug)] -struct PVEContext; - -impl Context for PVEContext { -fn lookup_email_for_user(&self, user: &str) -> Option { -let content = attempt_file_read("/etc/pve/user.cfg"); -content.and_then(|content| lookup_mail_address(&content, user)) -} - -fn default_sendmail_author(&self) -> String { -"Proxmox VE".into() -} - -fn default_sendmail_from(&self) -> String { -let content = attempt_file_read("/etc/pve/datacenter.cfg"); -content -.and_then(|content| lookup_datacenter_config_key(&content, "email_from")) -.unwrap_or_else(|| String::from("root")) -} - -fn http_proxy_config(&self) -> Option { -let content = attempt_file_read("/etc/pve/datacenter.cfg"); -content.and_then(|content| lookup_datacenter_config_key(&content, "http_proxy")) -} -} - -#[cfg(test)] -mod tests { -use super::*; - -const USER_CONFIG: &str = " -user:root@pam:1:0:::r...@example.com::: -user:test@pve:1:0:::t...@example.com::: -user:no-mail@pve:1:0:: -"; - -#[test] -fn test_parse_mail() { -assert_eq!( -lookup_mail_address(USER_CONFIG, "root@pam"), -Some("r...@example.com".to_string()) -); -assert_eq!( -lookup_mail_address(USER_CONFIG, "test@pve"), -Some("t...@example.com".to_string()) -); -assert_eq!(lookup_mail_address(USER_CONFIG, "no-m
[pve-devel] [PATCH v2 pve-manager 32/52] ui: dc: config: show notification panel again
Rework should be done now. Signed-off-by: Lukas Wagner --- www/manager6/dc/Config.js | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index 0dea1c67..74a84e91 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -317,14 +317,9 @@ Ext.define('PVE.dc.Config', { ); } - // this is being reworked, but we need to release newer manager versions already.. - let notification_enabled = false; - if (notification_enabled && ( - caps.mapping['Mapping.Audit'] || - caps.mapping['Mapping.Use'] || - caps.mapping['Mapping.Modify'] - ) - ) { + if (caps.mapping['Mapping.Audit'] || + caps.mapping['Mapping.Use'] || + caps.mapping['Mapping.Modify']) { me.items.push( { xtype: 'pmxNotificationConfigView', -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 pve-manager 27/52] vzdump: adapt to new matcher based notification system
To ease the migration from old-style mailto/mailnotification paramters for backup jobs, the code will add a ephemeral sendmail endpoint and a matcher. Signed-off-by: Lukas Wagner --- PVE/API2/VZDump.pm | 8 +--- PVE/VZDump.pm | 40 +++- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm index 3886772e..f66fc740 100644 --- a/PVE/API2/VZDump.pm +++ b/PVE/API2/VZDump.pm @@ -44,9 +44,7 @@ __PACKAGE__->register_method ({ ."'Datastore.AllocateSpace' on the backup storage. The 'tmpdir', 'dumpdir' and " ."'script' parameters are restricted to the 'root\@pam' user. The 'maxfiles' and " ."'prune-backups' settings require 'Datastore.Allocate' on the backup storage. The " - ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. " - ."If 'notification-target' is set, then the 'Mapping.Use' permission is needed on " - ."'/mapping/notification/'.", + ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. ", user => 'all', }, protected => 1, @@ -115,10 +113,6 @@ __PACKAGE__->register_method ({ $rpcenv->check($user, "/storage/$storeid", [ 'Datastore.AllocateSpace' ]); } - if (my $target = $param->{'notification-target'}) { - PVE::Notify::check_may_use_target($target, $rpcenv); - } - my $worker = sub { my $upid = shift; diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm index 454ab494..b0574d41 100644 --- a/PVE/VZDump.pm +++ b/PVE/VZDump.pm @@ -452,20 +452,18 @@ sub send_notification { my $opts = $self->{opts}; my $mailto = $opts->{mailto}; my $cmdline = $self->{cmdline}; -my $target = $opts->{"notification-target"}; -# Fall back to 'mailnotification' if 'notification-policy' is not set. -# If both are set, 'notification-policy' takes precedence -my $policy = $opts->{"notification-policy"} // $opts->{mailnotification} // 'always'; +# Old-style notification policy. This parameter will influce +# if an ad-hoc notification target/matcher will be created. +my $policy = $opts->{"notification-policy"} // + $opts->{mailnotification} // + 'always'; -return if ($policy eq 'never'); sanitize_task_list($tasklist); my $error_count = count_failed_tasks($tasklist); my $failed = ($error_count || $err); -return if (!$failed && ($policy eq 'failure')); - my $status_text = $failed ? 'backup failed' : 'backup successful'; if ($err) { @@ -489,8 +487,10 @@ sub send_notification { "See Task History for details!\n"; }; +my $hostname = get_hostname(); + my $notification_props = { - "hostname" => get_hostname(), + "hostname" => $hostname, "error-message" => $err, "guest-table" => build_guest_table($tasklist), "logs" => $text_log_part, @@ -498,9 +498,16 @@ sub send_notification { "total-time"=> $total_time, }; +my $fields = { + type => "vzdump", + hostname => $hostname, +}; + my $notification_config = PVE::Notify::read_config(); -if ($mailto && scalar(@$mailto)) { +my $legacy_sendmail = $policy eq "always" || ($policy eq "failure" && $failed); + +if ($mailto && scalar(@$mailto) && $legacy_sendmail) { # <, >, @ are not allowed in endpoint names, but that is only # verified once the config is serialized. That means that # we can rely on that fact that no other endpoint with this name exists. @@ -514,29 +521,20 @@ sub send_notification { my $endpoints = [$endpoint_name]; - # Create an anonymous group containing the sendmail endpoint and the - # $target endpoint, if specified - if ($target) { - push @$endpoints, $target; - } - - $target = ""; - $notification_config->add_group( - $target, + $notification_config->add_matcher( + "", $endpoints, ); } -return if (!$target); - my $severity = $failed ? "error" : "info"; PVE::Notify::notify( - $target, $severity, $subject_template, $body_template, $notification_props, + $fields, $notification_config ); }; -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-perl-rs 16/52] notify: adapt to new matcher-based notification routing
Signed-off-by: Lukas Wagner --- common/src/notify.rs | 167 +-- 1 file changed, 50 insertions(+), 117 deletions(-) diff --git a/common/src/notify.rs b/common/src/notify.rs index 9f44225..4fbd705 100644 --- a/common/src/notify.rs +++ b/common/src/notify.rs @@ -1,10 +1,12 @@ #[perlmod::package(name = "Proxmox::RS::Notify")] mod export { +use std::collections::HashMap; +use std::sync::Mutex; + use anyhow::{bail, Error}; -use perlmod::Value; use serde_json::Value as JSONValue; -use std::sync::Mutex; +use perlmod::Value; use proxmox_http_error::HttpError; use proxmox_notify::endpoints::gotify::{ DeleteableGotifyProperty, GotifyConfig, GotifyConfigUpdater, GotifyPrivateConfig, @@ -13,10 +15,10 @@ mod export { use proxmox_notify::endpoints::sendmail::{ DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater, }; -use proxmox_notify::filter::{ -DeleteableFilterProperty, FilterConfig, FilterConfigUpdater, FilterModeOperator, +use proxmox_notify::matcher::{ +CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig, +MatcherConfigUpdater, SeverityMatcher, }; -use proxmox_notify::group::{DeleteableGroupProperty, GroupConfig, GroupConfigUpdater}; use proxmox_notify::{api, Config, Notification, Severity}; pub struct NotificationConfig { @@ -87,22 +89,22 @@ mod export { #[export(serialize_error)] fn send( #[try_from_ref] this: &NotificationConfig, -channel: &str, severity: Severity, title: String, body: String, -properties: Option, +template_data: Option, +fields: Option>, ) -> Result<(), HttpError> { let config = this.config.lock().unwrap(); - -let notification = Notification { +let notification = Notification::new_templated( severity, title, body, -properties, -}; +template_data.unwrap_or_default(), +fields.unwrap_or_default(), +); -api::common::send(&config, channel, ¬ification) +api::common::send(&config, ¬ification) } #[export(serialize_error)] @@ -114,78 +116,6 @@ mod export { api::common::test_target(&config, target) } -#[export(serialize_error)] -fn get_groups( -#[try_from_ref] this: &NotificationConfig, -) -> Result, HttpError> { -let config = this.config.lock().unwrap(); -api::group::get_groups(&config) -} - -#[export(serialize_error)] -fn get_group( -#[try_from_ref] this: &NotificationConfig, -id: &str, -) -> Result { -let config = this.config.lock().unwrap(); -api::group::get_group(&config, id) -} - -#[export(serialize_error)] -fn add_group( -#[try_from_ref] this: &NotificationConfig, -name: String, -endpoints: Vec, -comment: Option, -filter: Option, -) -> Result<(), HttpError> { -let mut config = this.config.lock().unwrap(); -api::group::add_group( -&mut config, -&GroupConfig { -name, -endpoint: endpoints, -comment, -filter, -}, -) -} - -#[export(serialize_error)] -fn update_group( -#[try_from_ref] this: &NotificationConfig, -name: &str, -endpoints: Option>, -comment: Option, -filter: Option, -delete: Option>, -digest: Option<&str>, -) -> Result<(), HttpError> { -let mut config = this.config.lock().unwrap(); -let digest = decode_digest(digest)?; - -api::group::update_group( -&mut config, -name, -&GroupConfigUpdater { -endpoint: endpoints, -comment, -filter, -}, -delete.as_deref(), -digest.as_deref(), -) -} - -#[export(serialize_error)] -fn delete_group( -#[try_from_ref] this: &NotificationConfig, -name: &str, -) -> Result<(), HttpError> { -let mut config = this.config.lock().unwrap(); -api::group::delete_group(&mut config, name) -} - #[export(serialize_error)] fn get_sendmail_endpoints( #[try_from_ref] this: &NotificationConfig, @@ -213,7 +143,6 @@ mod export { from_address: Option, author: Option, comment: Option, -filter: Option, ) -> Result<(), HttpError> { let mut config = this.config.lock().unwrap(); @@ -226,7 +155,7 @@ mod export { from_address,
[pve-devel] [PATCH v2 proxmox-widget-toolkit 41/52] notification ui: unprotected mailto-root target
A default notification config will now be created in pve-manager's postinst hook - which is not magic in any way and can be modified and deleted as desired. Signed-off-by: Lukas Wagner --- src/panel/NotificationConfigView.js | 6 -- 1 file changed, 6 deletions(-) diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js index ecf764d..6a9bc20 100644 --- a/src/panel/NotificationConfigView.js +++ b/src/panel/NotificationConfigView.js @@ -41,10 +41,6 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { openEditWindow: function(endpointType, endpoint) { let me = this; - if (endpoint === 'mail-to-root') { - return; - } - Ext.create('Proxmox.window.EndpointEditBase', { baseUrl: me.getView().baseUrl, type: endpointType, @@ -183,13 +179,11 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { xtype: 'proxmoxButton', text: gettext('Modify'), handler: 'openEditForSelectedItem', - enableFn: rec => rec.data.name !== 'mail-to-root', disabled: true, }, { xtype: 'proxmoxStdRemoveButton', callback: 'reload', - enableFn: rec => rec.data.name !== 'mail-to-root', getUrl: function(rec) { return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`; }, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-widget-toolkit 42/52] noficiation: matcher edit: make 'field' an editable combobox
For now with fixed options that are shared between most notification events - later, once we have a notification registry, this should be filled dynamically. Signed-off-by: Lukas Wagner --- src/window/NotificationMatcherEdit.js | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js index c6f0726..fb55e17 100644 --- a/src/window/NotificationMatcherEdit.js +++ b/src/window/NotificationMatcherEdit.js @@ -963,14 +963,23 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', { }, { fieldLabel: gettext('Field'), - xtype: 'textfield', + xtype: 'proxmoxKVComboBox', isFormField: false, submitValue: false, + allowBlank: false, + editable: true, + displayField: 'key', bind: { hidden: '{!typeIsMatchField}', disabled: '{!typeIsMatchField}', value: '{matchFieldField}', }, + // TODO: Once we have a 'notification registry', we should + // retrive those via an API call. + comboItems: [ + ['type', ''], + ['hostname', ''], + ], }, { fieldLabel: gettext('Value'), -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-mail-forward 52/52] update d/control
proxmox-schema and proxmox-section config is not required anymore. add new dependency to proxmox-notify. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - new in v3 debian/control | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/debian/control b/debian/control index 43af70e..eb83a32 100644 --- a/debian/control +++ b/debian/control @@ -6,8 +6,10 @@ Build-Depends: cargo:native, librust-anyhow-1+default-dev, librust-log-0.4+default-dev (>= 0.4.17~~), librust-nix-0.26+default-dev, - librust-proxmox-schema-1+default-dev (>= 1.3~~), - librust-proxmox-section-config-1+default-dev (>= 1.0.2-~~), + librust-proxmox-notify-0.2+default-dev, + librust-proxmox-notify-0.2+mail-forwarder-dev, + librust-proxmox-notify-0.2+pbs-context-dev, + librust-proxmox-notify-0.2+pve-context-dev, librust-proxmox-sys-0.5+default-dev, librust-serde-1+default-dev, librust-serde-1+derive-dev, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox-widget-toolkit 45/52] notification ui: add column for 'origin'
This column shows whether a matcher/target was provided as a built-in default config or if it was created by the user. For built-ins, it also shows whether the built-in settings have been changed. To reset a built-in entry to its defaults, one can simply delete it. For best UX, the 'delete' button should change its text to 'reset defaults' when a built-in target/matcher is selected. This will be added in another patch. Signed-off-by: Lukas Wagner --- src/data/model/NotificationConfig.js | 4 ++-- src/panel/NotificationConfigView.js | 32 ++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js index 9171515..e8ebf28 100644 --- a/src/data/model/NotificationConfig.js +++ b/src/data/model/NotificationConfig.js @@ -1,6 +1,6 @@ Ext.define('proxmox-notification-endpoints', { extend: 'Ext.data.Model', -fields: ['name', 'type', 'comment', 'disable'], +fields: ['name', 'type', 'comment', 'disable', 'origin'], proxy: { type: 'proxmox', }, @@ -9,7 +9,7 @@ Ext.define('proxmox-notification-endpoints', { Ext.define('proxmox-notification-matchers', { extend: 'Ext.data.Model', -fields: ['name', 'comment', 'disable'], +fields: ['name', 'comment', 'disable', 'origin'], proxy: { type: 'proxmox', }, diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js index ba69298..4695da5 100644 --- a/src/panel/NotificationConfigView.js +++ b/src/panel/NotificationConfigView.js @@ -129,7 +129,7 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { dataIndex: 'name', text: gettext('Target Name'), renderer: Ext.String.htmlEncode, - flex: 1, + flex: 2, }, { dataIndex: 'type', @@ -141,7 +141,21 @@ Ext.define('Proxmox.panel.NotificationEndpointView', { dataIndex: 'comment', text: gettext('Comment'), renderer: Ext.String.htmlEncode, - flex: 1, + flex: 3, + }, + { + dataIndex: 'origin', + text: gettext('Origin'), + renderer: (origin) => { + switch (origin) { + case 'user-created': return gettext('Custom'); + case 'modified-builtin': return gettext('Built-In (modified)'); + case 'builtin': return gettext('Built-In'); + } + + // Should not happen... + return 'unknown'; + }, }, ], @@ -274,6 +288,20 @@ Ext.define('Proxmox.panel.NotificationMatcherView', { renderer: Ext.String.htmlEncode, flex: 2, }, + { + dataIndex: 'origin', + text: gettext('Origin'), + renderer: (origin) => { + switch (origin) { + case 'user-created': return gettext('Custom'); + case 'modified-builtin': return gettext('Built-In (modified)'); + case 'builtin': return gettext('Built-In'); + } + + // Should not happen... + return 'unknown'; + }, + }, ], store: { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 proxmox 11/52] notify: add PVE/PBS context
This commit moves PVEContext from `proxmox-perl-rs` into the `proxmox-notify` crate, since we now also need to access it from `promxox-mail-forward`. The context is now hidden behind a feature flag `pve-context`, ensuring that we only compile it when needed. This commit adds PBSContext, since we now require it for `proxmox-mail-forward`. Some of the code for PBSContext comes from `proxmox-mail-forward`. This commit also changes the global context from being stored in a `once_cell` to a regular `Mutex`, since we now need to set/reset the context in `proxmox-mail-forward`. Signed-off-by: Lukas Wagner --- Notes: Changes v2 -> v3: - no changes proxmox-notify/Cargo.toml| 3 +- proxmox-notify/src/context.rs| 21 - proxmox-notify/src/context/common.rs | 27 ++ proxmox-notify/src/context/mod.rs| 36 proxmox-notify/src/context/pbs.rs| 130 +++ proxmox-notify/src/context/pve.rs| 82 + 6 files changed, 277 insertions(+), 22 deletions(-) delete mode 100644 proxmox-notify/src/context.rs create mode 100644 proxmox-notify/src/context/common.rs create mode 100644 proxmox-notify/src/context/mod.rs create mode 100644 proxmox-notify/src/context/pbs.rs create mode 100644 proxmox-notify/src/context/pve.rs diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index f2b4db5..7a3d434 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -13,7 +13,6 @@ handlebars = { workspace = true } lazy_static.workspace = true log.workspace = true mail-parser = { workspace = true, optional = true } -once_cell.workspace = true openssl.workspace = true proxmox-http = { workspace = true, features = ["client-sync"], optional = true } proxmox-http-error.workspace = true @@ -32,3 +31,5 @@ default = ["sendmail", "gotify"] mail-forwarder = ["dep:mail-parser"] sendmail = ["dep:proxmox-sys"] gotify = ["dep:proxmox-http"] +pve-context = ["dep:proxmox-sys"] +pbs-context = ["dep:proxmox-sys"] diff --git a/proxmox-notify/src/context.rs b/proxmox-notify/src/context.rs deleted file mode 100644 index 370c7ee..000 --- a/proxmox-notify/src/context.rs +++ /dev/null @@ -1,21 +0,0 @@ -use std::fmt::Debug; - -use once_cell::sync::OnceCell; - -pub trait Context: Send + Sync + Debug { -fn lookup_email_for_user(&self, user: &str) -> Option; -fn default_sendmail_author(&self) -> String; -fn default_sendmail_from(&self) -> String; -fn http_proxy_config(&self) -> Option; -} - -static CONTEXT: OnceCell<&'static dyn Context> = OnceCell::new(); - -pub fn set_context(context: &'static dyn Context) { -CONTEXT.set(context).expect("context has already been set"); -} - -#[allow(unused)] // context is not used if all endpoint features are disabled -pub(crate) fn context() -> &'static dyn Context { -*CONTEXT.get().expect("context has not been yet") -} diff --git a/proxmox-notify/src/context/common.rs b/proxmox-notify/src/context/common.rs new file mode 100644 index 000..7580bd1 --- /dev/null +++ b/proxmox-notify/src/context/common.rs @@ -0,0 +1,27 @@ +use std::path::Path; + +pub(crate) fn attempt_file_read>(path: P) -> Option { +match proxmox_sys::fs::file_read_optional_string(path) { +Ok(contents) => contents, +Err(err) => { +log::error!("{err}"); +None +} +} +} + +pub(crate) fn lookup_datacenter_config_key(content: &str, key: &str) -> Option { +let key_prefix = format!("{key}:"); +normalize_for_return( +content +.lines() +.find_map(|line| line.strip_prefix(&key_prefix)), +) +} + +pub(crate) fn normalize_for_return(s: Option<&str>) -> Option { +match s?.trim() { +"" => None, +s => Some(s.to_string()), +} +} diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs new file mode 100644 index 000..99d86de --- /dev/null +++ b/proxmox-notify/src/context/mod.rs @@ -0,0 +1,36 @@ +use std::fmt::Debug; +use std::sync::Mutex; + +#[cfg(any(feature = "pve-context", feature = "pbs-context"))] +pub mod common; +#[cfg(feature = "pbs-context")] +pub mod pbs; +#[cfg(feature = "pve-context")] +pub mod pve; + +/// Product-specific context +pub trait Context: Send + Sync + Debug { +/// Look up a user's email address from users.cfg +fn lookup_email_for_user(&self, user: &str) -> Option; +/// Default mail author for mail-based targets +fn default_sendmail_author(&self) -> String; +/// Default from address for sendmail-based targets +fn default_sendmail_from(&self) -> String; +/// Proxy configuration for the current
[pve-devel] [PATCH v2 pve-docs 50/52] notifications: change to simplified ACL structure.
For now, we use a less deeply nested structure. We can always extend it if we need to. Signed-off-by: Lukas Wagner --- notifications.adoc | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index c7bdc5a..74447e5 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -309,11 +309,9 @@ content, they will be transformed into plain text format during this process. Permissions --- -For every target, there exists a corresponding ACL path -`/mapping/notification/targets/`. Matchers use -a seperate namespace in the ACL tree: `/mapping/notification/matchers/`. - -To test a target, a user must have the `Mapping.Use` permission on the corresponding -node in the ACL tree. -`Mapping.Modify` and `Mapping.Audit` are needed to read/modify the -configuration of a target or matcher. +In order to modify/view the configuration for notification targets, +the `Mapping.Modify/Mapping.Audit` permissions are required for the +`/mapping/notifications` ACL node. + +Testing a target requires `Mapping.Use`, `Mapping.Audit` or `Mapping.Modify` +permissions on `/mapping/notifications` -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel