all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pmg-devel] [PATCH pmg-api v3] fix #4410: Remove non-null host bits from CIDR when writing postfix config
@ 2022-12-28 11:52 Christoph Heiss
  2022-12-28 17:08 ` Stoiko Ivanov
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Heiss @ 2022-12-28 11:52 UTC (permalink / raw)
  To: pmg-devel

This will drop non-null host bits from `mynetworks` CIDRs when writing
the `main.cf` postfix template.
Backwards-compatibility with old entries in `/etc/pmg/mynetworks` is
thus also preserved.

Add an additional comment to the mynetworks API, indicating that unused
fields can/should be dropped with the next PMG version.

No GUI changes. The entries are written to `/etc/pmg/mynetworks` as the
user enters them. Suggested by Stoiko, see discussion in v2 thread [0].

[0] https://lists.proxmox.com/pipermail/pmg-devel/2022-December/002247.html

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---

 Changes v2 -> v3:
 * Dropped validation of host-bits of new entries on creation
 * Entries are now again written verbatim to `/etc/pmg/mynetworks`
 * Host bits are now dropped when writing the postfix template

 Changes v1 -> v2:
 * Reverted unneeded loop iterator change
 * Display CIDRs in GUI as the user entered them

 src/PMG/Config.pm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index 9ba5c76..c702394 100755
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -8,6 +8,7 @@ use Data::Dumper;
 use PVE::Tools;
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::SectionConfig;
+use PVE::Network;

 use base qw(PVE::SectionConfig);

@@ -1011,6 +1012,7 @@ sub read_pmg_mynetworks {
 	    if ($line =~ m!^((?:$IPV4RE|$IPV6RE))/(\d+)\s*(?:#(.*)\s*)?$!) {
 		my ($network, $prefix_size, $comment) = ($1, $2, $3);
 		my $cidr = "$network/${prefix_size}";
+		# FIXME: Drop unused `network_address` and `prefix_size` with PMG 8.0
 		$mynetworks->{$cidr} = {
 		    cidr => $cidr,
 		    network_address => $network,
@@ -1337,10 +1339,12 @@ sub get_template_vars {

     my $netlist = PVE::INotify::read_file('mynetworks');
     foreach my $cidr (keys %$netlist) {
-	if ($cidr =~ m/^($IPV6RE)\/(\d+)$/) {
-	    $mynetworks->{"[$1]/$2"} = 1;
+	my $ip = PVE::Network::IP_from_cidr($cidr);
+	if ($ip->version() == 4) {
+	    $mynetworks->{$ip->prefix()} = 1;
 	} else {
-	    $mynetworks->{$cidr} = 1;
+	    my $address = '[' . $ip->short() . ']/' . $ip->prefixlen();
+	    $mynetworks->{$address} = 1;
 	}
     }

--
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v3] fix #4410: Remove non-null host bits from CIDR when writing postfix config
  2022-12-28 11:52 [pmg-devel] [PATCH pmg-api v3] fix #4410: Remove non-null host bits from CIDR when writing postfix config Christoph Heiss
@ 2022-12-28 17:08 ` Stoiko Ivanov
  2022-12-29  9:21   ` Christoph Heiss
  0 siblings, 1 reply; 3+ messages in thread
From: Stoiko Ivanov @ 2022-12-28 17:08 UTC (permalink / raw)
  To: Christoph Heiss; +Cc: pmg-devel

Looks good and minimal - one tiny nit/improvement:

On Wed, 28 Dec 2022 12:52:59 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> This will drop non-null host bits from `mynetworks` CIDRs when writing
> the `main.cf` postfix template.
> Backwards-compatibility with old entries in `/etc/pmg/mynetworks` is
> thus also preserved.
> 
> Add an additional comment to the mynetworks API, indicating that unused
> fields can/should be dropped with the next PMG version.
> 
> No GUI changes. The entries are written to `/etc/pmg/mynetworks` as the
> user enters them. Suggested by Stoiko, see discussion in v2 thread [0].
> 
> [0] https://lists.proxmox.com/pipermail/pmg-devel/2022-December/002247.html
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> 
>  Changes v2 -> v3:
>  * Dropped validation of host-bits of new entries on creation
>  * Entries are now again written verbatim to `/etc/pmg/mynetworks`
>  * Host bits are now dropped when writing the postfix template
> 
>  Changes v1 -> v2:
>  * Reverted unneeded loop iterator change
>  * Display CIDRs in GUI as the user entered them
> 
>  src/PMG/Config.pm | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
> index 9ba5c76..c702394 100755
> --- a/src/PMG/Config.pm
> +++ b/src/PMG/Config.pm
> @@ -8,6 +8,7 @@ use Data::Dumper;
>  use PVE::Tools;
>  use PVE::JSONSchema qw(get_standard_option);
>  use PVE::SectionConfig;
> +use PVE::Network;
> 
>  use base qw(PVE::SectionConfig);
> 
> @@ -1011,6 +1012,7 @@ sub read_pmg_mynetworks {
>  	    if ($line =~ m!^((?:$IPV4RE|$IPV6RE))/(\d+)\s*(?:#(.*)\s*)?$!) {
>  		my ($network, $prefix_size, $comment) = ($1, $2, $3);
>  		my $cidr = "$network/${prefix_size}";
> +		# FIXME: Drop unused `network_address` and `prefix_size` with PMG 8.0
>  		$mynetworks->{$cidr} = {
>  		    cidr => $cidr,
>  		    network_address => $network,
> @@ -1337,10 +1339,12 @@ sub get_template_vars {
> 
>      my $netlist = PVE::INotify::read_file('mynetworks');
>      foreach my $cidr (keys %$netlist) {
> -	if ($cidr =~ m/^($IPV6RE)\/(\d+)$/) {
> -	    $mynetworks->{"[$1]/$2"} = 1;
> +	my $ip = PVE::Network::IP_from_cidr($cidr);
this can return undef and we should check for it
while our config-parser takes care of many edge-cases and broken cidrs I
managed to get passed it with a mask of 148 (max would be 128 for ipv6)

I would expect that at least part of our users do edit the config files
manually and end up with invalid data there.

Probably a `warn` and ignoring the entry might be appropriate here (afaict
this is what happens when the mynetworks parser runs into a broken line as
well)


> +	if ($ip->version() == 4) {
> +	    $mynetworks->{$ip->prefix()} = 1;
>  	} else {
> -	    $mynetworks->{$cidr} = 1;
> +	    my $address = '[' . $ip->short() . ']/' . $ip->prefixlen();
> +	    $mynetworks->{$address} = 1;
>  	}
>      }
> 
> --
> 2.30.2
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
> 





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pmg-devel] [PATCH pmg-api v3] fix #4410: Remove non-null host bits from CIDR when writing postfix config
  2022-12-28 17:08 ` Stoiko Ivanov
@ 2022-12-29  9:21   ` Christoph Heiss
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Heiss @ 2022-12-29  9:21 UTC (permalink / raw)
  To: Stoiko Ivanov; +Cc: pmg-devel

On Wed, Dec 28, 2022 at 06:08:43PM +0100, Stoiko Ivanov wrote:
> Looks good and minimal - one tiny nit/improvement:
>
> On Wed, 28 Dec 2022 12:52:59 +0100
> Christoph Heiss <c.heiss@proxmox.com> wrote:
>
> > [..]
> > @@ -1337,10 +1339,12 @@ sub get_template_vars {
> >
> >      my $netlist = PVE::INotify::read_file('mynetworks');
> >      foreach my $cidr (keys %$netlist) {
> > -	if ($cidr =~ m/^($IPV6RE)\/(\d+)$/) {
> > -	    $mynetworks->{"[$1]/$2"} = 1;
> > +	my $ip = PVE::Network::IP_from_cidr($cidr);
> this can return undef and we should check for it
> while our config-parser takes care of many edge-cases and broken cidrs I
> managed to get passed it with a mask of 148 (max would be 128 for ipv6)
>
> I would expect that at least part of our users do edit the config files
> manually and end up with invalid data there.
>
> Probably a `warn` and ignoring the entry might be appropriate here (afaict
> this is what happens when the mynetworks parser runs into a broken line as
> well)
Good point, really did not think of that.
Will send a v4 shortly!

>
>
> > +	if ($ip->version() == 4) {
> > +	    $mynetworks->{$ip->prefix()} = 1;
> >  	} else {
> > -	    $mynetworks->{$cidr} = 1;
> > +	    my $address = '[' . $ip->short() . ']/' . $ip->prefixlen();
> > +	    $mynetworks->{$address} = 1;
> >  	}
> >      }
> >
> > --
> > 2.30.2
> >
> >
> >
> > _______________________________________________
> > pmg-devel mailing list
> > pmg-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> >
> >
>




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-29  9:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-28 11:52 [pmg-devel] [PATCH pmg-api v3] fix #4410: Remove non-null host bits from CIDR when writing postfix config Christoph Heiss
2022-12-28 17:08 ` Stoiko Ivanov
2022-12-29  9:21   ` Christoph Heiss

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal