[PATCH pve-mamager v3 0/3] fix #8031: Write APT proxy config on http_proxy change

[PATCH pve-mamager v3 0/3] fix #8031: Write APT proxy config on http_proxy change

[Jonas Theisen]

Goal of this patch series is to update the http_proxy config
across the whole cluster immideately after a config change
and independently of the use of PVE tooling to trigger
a database update.

To allow reuse of the same code this patch series factors out
the relevant function inside the APT part of the API and reuses
the function on change of the http_proxy variable in the Cluster API.

This version also moves from trying to update the local file across
all nodes after a change to updating one file in the pmxcfs and
pointing all nodes to that file via a symlink.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8031

Thanks for the reviews of v2 @e.huhsovitz and @m.sandoval

Changes from v2:
 * added symlink logic to update call
 * adapted update function to only write on change
 * moved update into datacenter.cfg lock phase

Jonas Theisen (3):
  api: apt: factor out writing the APT proxy config
  api: apt: move proxy config to pmxcfs and symlink to it
  fix #8031: api: cluster options: update APT proxy config on change

 PVE/API2/APT.pm     | 48 ++++++++++++++++++++++++++++++++++++---------
 PVE/API2/Cluster.pm | 12 ++++++++++++
 2 files changed, 51 insertions(+), 9 deletions(-)

-- 
2.47.3

[PATCH pve-mamager v3 1/3] api: apt: factor out writing the APT proxy config

[Jonas Theisen]

Move writing the APT proxy config out of the update_database
endpoint into its own function, so that it can also be called when
the datacenter http_proxy option is changed.

The function takes the already parsed datacenter config, so callers
holding the datacenter.cfg lock can pass their updated copy.

Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
---
 PVE/API2/APT.pm | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 93562b06..8e16b813 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -290,6 +290,19 @@ __PACKAGE__->register_method({
     },
 });
 
+sub update_apt_proxy_config {
+    my ($dcconf) = @_;
+
+    my $aptconf = "// no proxy configured\n";
+
+    if (my $http_proxy = $dcconf->{http_proxy}) {
+        my $http_proxy_uri = URI->new($http_proxy);
+        $aptconf = "Acquire::http::Proxy \"${http_proxy_uri}\";\n";
+    }
+    my $aptcfn = "/etc/apt/apt.conf.d/76pveproxy";
+    PVE::Tools::file_set_contents($aptcfn, $aptconf);
+}
+
 __PACKAGE__->register_method({
     name => 'update_database',
     path => 'update',
@@ -334,15 +347,7 @@ __PACKAGE__->register_method({
         my $realcmd = sub {
             my $upid = shift;
 
-            # setup proxy for apt
-
-            my $aptconf = "// no proxy configured\n";
-            if (my $http_proxy = $dcconf->{http_proxy}) {
-                my $http_proxy_uri = URI->new($http_proxy);
-                $aptconf = "Acquire::http::Proxy \"${http_proxy_uri}\";\n";
-            }
-            my $aptcfn = "/etc/apt/apt.conf.d/76pveproxy";
-            PVE::Tools::file_set_contents($aptcfn, $aptconf);
+            update_apt_proxy_config($dcconf);
 
             my $cmd = ['apt-get', 'update'];
 
-- 
2.47.3

[PATCH pve-mamager v3 2/3] api: apt: move proxy config to pmxcfs and symlink to it

[Jonas Theisen]

The APT proxy config was written to /etc/apt/apt.conf.d/76pveproxy
only if a database update was triggered through the PVE tooling.

Write the config to /etc/pve/apt-proxy.conf instead and replace the
local file with a symlink to it, so that changes are applied
cluster-wide.

The symlink is auto-generated on the next database update and checks
for existance of the target file before creating it.

The config is also only updated if it differs from the current
to avoid unncessarry cluster traffic.

Updateing the database without write access to /etc/pve
still works and only issues a warning.

Note that files in /etc/pve are not readable for users outside the
www-data group, so APT invoked by such users will fail to read the
proxy config.

Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
---
 PVE/API2/APT.pm | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 8e16b813..2359a5f5 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -290,6 +290,24 @@ __PACKAGE__->register_method({
     },
 });
 
+my $apt_proxy_cfn = "/etc/apt/apt.conf.d/76pveproxy";
+my $pve_apt_proxy_cfn = "/etc/pve/apt-proxy.conf";
+
+my $setup_apt_proxy_symlink = sub {
+    my $target = readlink($apt_proxy_cfn);
+    return if defined($target) && $target eq $pve_apt_proxy_cfn;
+
+    my $tmpname = "$apt_proxy_cfn.tmp.$$";
+    unlink($tmpname);
+    symlink($pve_apt_proxy_cfn, $tmpname)
+        || die "unable to create symlink '$tmpname' - $!\n";
+    if (!rename($tmpname, $apt_proxy_cfn)) {
+        my $err = $!;
+        unlink($tmpname);
+        die "unable to replace '$apt_proxy_cfn' - $err\n";
+    }
+};
+
 sub update_apt_proxy_config {
     my ($dcconf) = @_;
 
@@ -299,8 +317,14 @@ sub update_apt_proxy_config {
         my $http_proxy_uri = URI->new($http_proxy);
         $aptconf = "Acquire::http::Proxy \"${http_proxy_uri}\";\n";
     }
-    my $aptcfn = "/etc/apt/apt.conf.d/76pveproxy";
-    PVE::Tools::file_set_contents($aptcfn, $aptconf);
+
+    my $current;
+    $current = PVE::Tools::file_get_contents($pve_apt_proxy_cfn) if -f $pve_apt_proxy_cfn;
+    if (!defined($current) || $current ne $aptconf) {
+        PVE::Tools::file_set_contents($pve_apt_proxy_cfn, $aptconf);
+    }
+
+    $setup_apt_proxy_symlink->();
 }
 
 __PACKAGE__->register_method({
@@ -347,7 +371,8 @@ __PACKAGE__->register_method({
         my $realcmd = sub {
             my $upid = shift;
 
-            update_apt_proxy_config($dcconf);
+            eval { update_apt_proxy_config($dcconf) };
+            warn "unable to update APT proxy config - $@" if $@;
 
             my $cmd = ['apt-get', 'update'];
 
-- 
2.47.3

[PATCH pve-mamager v3 3/3] fix #8031: api: cluster options: update APT proxy config on change

[Jonas Theisen]

Changing the http_proxy datacenter option did not update the APT
proxy config until the next package database update. Write it right
away, so APT on all nodes uses the new proxy immediately.

Do so while still holding the datacenter.cfg lock, so concurrent
changes cannot write the APT config in a different order than the
datacenter config.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8031
Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
---
 PVE/API2/Cluster.pm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 4e5efbfd..29820c59 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -23,6 +23,7 @@ use PVE::Tools qw(extract_param);
 
 use PVE::API2::ACMEAccount;
 use PVE::API2::ACMEPlugin;
+use PVE::API2::APT;
 use PVE::API2::Backup;
 use PVE::API2::Cluster::BackupInfo;
 use PVE::API2::Cluster::BulkAction;
@@ -842,8 +843,17 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
+        my $http_proxy_change = 0;
+
         my $delete = extract_param($param, 'delete');
 
+        if (
+            defined($param->{http_proxy})
+            || (defined($delete) && $delete =~ m/\bhttp_proxy\b/)
+        ) {
+            $http_proxy_change = 1;
+        }
+
         cfs_lock_file(
             'datacenter.cfg',
             undef,
@@ -855,6 +865,8 @@ __PACKAGE__->register_method({
                 delete $conf->{$_} for PVE::Tools::split_list($delete);
 
                 cfs_write_file('datacenter.cfg', $conf);
+
+                PVE::API2::APT::update_apt_proxy_config($conf) if $http_proxy_change;
             },
         );
         die $@ if $@;
-- 
2.47.3

Re: [PATCH pve-mamager v3 0/3] fix #8031: Write APT proxy config on http_proxy change

[Jonas Theisen]

Please ignore this mail and associated patches.
They were wrongly declared as v3 but were supposed to be v4.
Now sent correctly at: 
https://lore.proxmox.com/pve-devel/20261012145712.296002-1-j.theisen@proxmox.com/T/#t