[pmg-devel] [PATCH pmg-api 0/4] add notification about unmodified template-overrides

[pmg-devel] [PATCH pmg-api 0/4] add notification about unmodified template-overrides

[Stoiko Ivanov]

This patch series adds functionality to notify users and our support about
templates copied to /etc/pmg/templates, without any changes.

While I thought initially that this could lead to actual issues (as the
unmodified templates from an old version mask the changes we provide in
the package) - this is luckily handled gracefully by ucf (which simply
overwrites the unmodified version in /etc/pmg/templates, by the new one in
/var/lib/pmg/templates.)

The merit of the series is (at least) that it should now be easier to see
actually relevant changes in the templates in our support-channels
(through the updated report).

if applied - it would make sense to also apply this to stable-7

Stoiko Ivanov (4):
  utils: add helper for unmodified templates
  pmg7to8: notify about unmodified templates
  report: skip irrelevant files in /etc/pmg/templates
  report: group use statements

 src/PMG/CLI/pmg7to8.pm |  6 ++++++
 src/PMG/Report.pm      | 28 ++++++++++++++++++++++++++--
 src/PMG/Utils.pm       | 20 ++++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

-- 
2.39.2

[pmg-devel] [PATCH pmg-api 1/4] utils: add helper for unmodified templates

[Stoiko Ivanov]

find_unmodified_templates yields a list of overridden templates in
/etc/pmg/templates, which have no change to the one they replace from
/var/lib/pmg/templates.

In practice this should only be a cosmetic problem, since the ones
in /etc/pmg/templates would get updated to the new version in
/var/lib/pmg/templates, by ucf if they have no changes.
(There would be a dialog asking what to do if there was a change, as
with sshd_config). However it can cause confusion also when providing
support, and experience shows that there are quite a few installs,
which have all templates copied (only to make a small modification to
one).

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Utils.pm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 6405934..f0d31c1 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -1573,4 +1573,24 @@ sub try_decode_utf8 {
     return eval { decode('UTF-8', $data, 1) } // $data;
 }
 
+sub find_unmodified_templates {
+    my $res = [];
+
+    my $template_dir = '/var/lib/pmg/templates';
+    my $override_dir = '/etc/pmg/templates';
+
+    PVE::Tools::dir_glob_foreach('/var/lib/pmg/templates', '.*\.(?:tt|in).*', sub {
+	my ($filename) = @_;
+	if ( -e "$override_dir/$filename") {
+	    my $shipped = PVE::Tools::file_get_contents("$template_dir/$filename", 1024*1024);
+	    my $override = PVE::Tools::file_get_contents("$override_dir/$filename", 1024*1024);
+	    if ( $shipped eq $override ) {
+		push (@$res, $filename);
+	    }
+	}
+    });
+
+    return $res;
+}
+
 1;
-- 
2.39.2

Re: [pmg-devel] [PATCH pmg-api 1/4] utils: add helper for unmodified templates

[Thomas Lamprecht]

Am 07/07/2023 um 18:54 schrieb Stoiko Ivanov:
> find_unmodified_templates yields a list of overridden templates in
> /etc/pmg/templates, which have no change to the one they replace from
> /var/lib/pmg/templates.
> 

are we planning in reusing this for other things? as otherwise it might be
better to have it in pmg7to8 directly.

> In practice this should only be a cosmetic problem, since the ones
> in /etc/pmg/templates would get updated to the new version in
> /var/lib/pmg/templates, by ucf if they have no changes.
> (There would be a dialog asking what to do if there was a change, as
> with sshd_config). However it can cause confusion also when providing
> support, and experience shows that there are quite a few installs,
> which have all templates copied (only to make a small modification to
> one).
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/Utils.pm | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 6405934..f0d31c1 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -1573,4 +1573,24 @@ sub try_decode_utf8 {
>      return eval { decode('UTF-8', $data, 1) } // $data;
>  }
>  
> +sub find_unmodified_templates {
> +    my $res = [];
> +
> +    my $template_dir = '/var/lib/pmg/templates';
> +    my $override_dir = '/etc/pmg/templates';
> +
> +    PVE::Tools::dir_glob_foreach('/var/lib/pmg/templates', '.*\.(?:tt|in).*', sub {
> +	my ($filename) = @_;
> +	if ( -e "$override_dir/$filename") {

style issue: extra leading whitespace in if-test expression.

also, we often use an early "return" like `next if !-e "...";` to avoid an indentation for the
rest of the block for such must-have conditions for the whole block.


> +	    my $shipped = PVE::Tools::file_get_contents("$template_dir/$filename", 1024*1024);
> +	    my $override = PVE::Tools::file_get_contents("$override_dir/$filename", 1024*1024);
> +	    if ( $shipped eq $override ) {

style issue: extra leading and trailing whitespace in if-test expression

> +		push (@$res, $filename);
> +	    }
> +	}
> +    });
> +
> +    return $res;
> +}
> +
>  1;

[pmg-devel] [PATCH pmg-api 2/4] pmg7to8: notify about unmodified templates

[Stoiko Ivanov]

I considered making this a warning, but since unmodified files get
updated to the new versions in /var/lib/pmg/templates by ucf a notice
seems more appropriate.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/CLI/pmg7to8.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PMG/CLI/pmg7to8.pm b/src/PMG/CLI/pmg7to8.pm
index ea29056..1cb4e7f 100644
--- a/src/PMG/CLI/pmg7to8.pm
+++ b/src/PMG/CLI/pmg7to8.pm
@@ -552,6 +552,12 @@ sub check_misc {
 
     check_apt_repos();
     check_bootloader();
+
+    if (my $unmodified_templates = PMG::Utils::find_unmodified_templates()) {
+	my $msg = "Found templates in /etc/pmg/templates, without modification. Consider simply removing them: \n    ";
+	$msg .= join("\n    ", $unmodified_templates->@*);
+	log_notice($msg);
+    }
 }
 
 my sub colored_if {
-- 
2.39.2

[pmg-devel] applied: [PATCH pmg-api 2/4] pmg7to8: notify about unmodified templates

[Thomas Lamprecht]

Am 07/07/2023 um 18:54 schrieb Stoiko Ivanov:
> I considered making this a warning, but since unmodified files get
> updated to the new versions in /var/lib/pmg/templates by ucf a notice
> seems more appropriate.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/CLI/pmg7to8.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
>

applied, but merged in the relevant part from patch 1 directly, thanks!

[pmg-devel] [PATCH pmg-api 3/4] report: skip irrelevant files in /etc/pmg/templates

[Stoiko Ivanov]

This patch removes:
* templates which have no changes to the ones in
  /var/lib/pmg/templates
* files generated by ucf

from the report. Unmodified files are reported, so that the user can
remove them.

This should make providing support a bit easier - as currenlty I'd
copy each template from the report to `diff` it with the version in
the package, for finding out if there is something relevant.

the new dump_template sub was copied from dir_to_text, in order to
explicitly write which files are skipped.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Report.pm | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/PMG/Report.pm b/src/PMG/Report.pm
index 027c882..b3a554c 100644
--- a/src/PMG/Report.pm
+++ b/src/PMG/Report.pm
@@ -5,6 +5,8 @@ use warnings;
 use PVE::Tools;
 use Mail::SpamAssassin::DnsResolver;
 
+use PMG::Utils;
+
 $ENV{'PATH'} = '/sbin:/bin:/usr/sbin:/usr/bin';
 
 my $cmd_timeout = 10; # generous timeout
@@ -46,7 +48,7 @@ my $report_def = {
 	'pmgconfig dump',
 	sub { dir2text('/etc/pmg/','(?:domains|mynetworks|tls_policy|transport)' ) },
 	sub { dir2text('/etc/postfix/','(?:clientaccess|senderaccess|rcptaccess)' ) },
-	sub { dir2text('/etc/pmg/templates/', '[^.].*' ) },
+	sub { dump_templates() },
 	'pmgdb dump',
     ],
 };
@@ -135,4 +137,26 @@ sub check_dns_resolution {
     $report .= $answertext . "\n";
 }
 
+sub dump_templates {
+
+    my $unmodified = PMG::Utils::find_unmodified_templates();
+    my $unmodified_re;
+    if ($unmodified) {
+	$report .= "\n unmodified templates:\n    ";
+	$report .= join("\n    ", $unmodified->@*);
+	$unmodified_re = '(?:'.join('|', $unmodified->@*) .')';
+    }
+
+    my $template_dir = '/etc/pmg/templates/';
+    PVE::Tools::dir_glob_foreach($template_dir, '[^.].*', sub {
+	my ($file) = @_;
+	if ($file =~ /\.ucf-(?:dist|new|old)/) {
+	    $report .= "\n skipping $file";
+	} elsif (!defined($unmodified_re) || $file !~ qr/$unmodified_re$/) {
+	    $report .=  "\n# cat $template_dir$file\n";
+	    $report .= PVE::Tools::file_get_contents($template_dir.$file)."\n";
+	}
+    });
+}
+
 1;
-- 
2.39.2

[pmg-devel] applied: [PATCH pmg-api 3/4] report: skip irrelevant files in /etc/pmg/templates

[Thomas Lamprecht]

Am 07/07/2023 um 18:54 schrieb Stoiko Ivanov:
> This patch removes:
> * templates which have no changes to the ones in
>   /var/lib/pmg/templates
> * files generated by ucf
> 
> from the report. Unmodified files are reported, so that the user can
> remove them.
> 
> This should make providing support a bit easier - as currenlty I'd
> copy each template from the report to `diff` it with the version in
> the package, for finding out if there is something relevant.
> 
> the new dump_template sub was copied from dir_to_text, in order to
> explicitly write which files are skipped.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/Report.pm | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
>

applied, but merged in the relevant part from patch 1 directly, thanks!

[pmg-devel] [PATCH pmg-api 4/4] report: group use statements

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Report.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PMG/Report.pm b/src/PMG/Report.pm
index b3a554c..f55f062 100644
--- a/src/PMG/Report.pm
+++ b/src/PMG/Report.pm
@@ -2,9 +2,9 @@ package PMG::Report;
 
 use strict;
 use warnings;
-use PVE::Tools;
 use Mail::SpamAssassin::DnsResolver;
 
+use PVE::Tools;
 use PMG::Utils;
 
 $ENV{'PATH'} = '/sbin:/bin:/usr/sbin:/usr/bin';
-- 
2.39.2

[pmg-devel] applied: [PATCH pmg-api 4/4] report: group use statements

[Thomas Lamprecht]

Am 07/07/2023 um 18:54 schrieb Stoiko Ivanov:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/Report.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!