all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change
@ 2026-09-15  9:16 Jonas Theisen
  2026-09-15  9:16 ` [PATCH pve-manager v3 1/2] fix #8031: factor out APT proxy write from update_database call Jonas Theisen
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jonas Theisen @ 2026-09-15  9:16 UTC (permalink / raw)
  To: pve-devel

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 is to ensure a consistent state after a change of the HTTP proxy
without having to run a Refresh / apt update through the PVE tooling.

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

Changes from v2:
 * Refactored check for proxy change for more human-readability
 * Verified regex pattern

Rationale for the regex pattern's \b boundaries is that potential
new parameters could be named http_proxy_port or similar which
would still match the regex pattern without the \b boundaries.
This can be intentional but should be checked if the problem arises.

Tested:
 * Set HTTP Proxy through GUI updates apt proxy config
 * Removal of HTTP proxy through GUI updates config
 * Removal of http_proxy and bwlimit through API updates config

Thanks for the feedback @m.sandoval!

Jonas Theisen (2):
  fix #8031: factor out APT proxy write from update_database call
  fix #8031: write APT proxy config on http_proxy change

 PVE/API2/APT.pm     | 23 ++++++++++++++---------
 PVE/API2/Cluster.pm | 14 ++++++++++++++
 2 files changed, 28 insertions(+), 9 deletions(-)

-- 
2.47.3





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

* [PATCH pve-manager v3 1/2] fix #8031: factor out APT proxy write from update_database call
  2026-09-15  9:16 [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
@ 2026-09-15  9:16 ` Jonas Theisen
  2026-09-15  9:16 ` [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
  2026-09-16  9:32 ` [PATCH pve-manager v3 0/2] " Maximiliano Sandoval
  2 siblings, 0 replies; 5+ messages in thread
From: Jonas Theisen @ 2026-09-15  9:16 UTC (permalink / raw)
  To: pve-devel

To allow reuse of the same function for the Cluster set_options call
this commit factors the function to write the APT proxy config file
out of the update_database call into a sub function.

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 6dd4c261..87928e58 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 = PVE::Cluster::cfs_read_file('datacenter.cfg');
+    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();
 
             my $cmd = ['apt-get', 'update'];
 
-- 
2.47.3





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

* [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change
  2026-09-15  9:16 [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
  2026-09-15  9:16 ` [PATCH pve-manager v3 1/2] fix #8031: factor out APT proxy write from update_database call Jonas Theisen
@ 2026-09-15  9:16 ` Jonas Theisen
  2026-10-02 10:32   ` Elias Huhsovitz
  2026-09-16  9:32 ` [PATCH pve-manager v3 0/2] " Maximiliano Sandoval
  2 siblings, 1 reply; 5+ messages in thread
From: Jonas Theisen @ 2026-09-15  9:16 UTC (permalink / raw)
  To: pve-devel

Writes the necessary file for APT if the http_proxy variable
on the datacenter level is changed.

This reuses the same function from the APT API for a regular
database update.

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

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 4e5efbfd..efda3906 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -36,6 +36,7 @@ use PVE::API2::ClusterConfig;
 use PVE::API2::Firewall::Cluster;
 use PVE::API2::HAConfig;
 use PVE::API2::ReplicationConfig;
+use PVE::API2::APT;
 
 my $have_sdn;
 eval {
@@ -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,
@@ -859,6 +869,10 @@ __PACKAGE__->register_method({
         );
         die $@ if $@;
 
+        if ($http_proxy_change) {
+            PVE::API2::APT::update_apt_proxy_config();
+        }
+
         return undef;
     },
 });
-- 
2.47.3





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

* Re: [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change
  2026-09-15  9:16 [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
  2026-09-15  9:16 ` [PATCH pve-manager v3 1/2] fix #8031: factor out APT proxy write from update_database call Jonas Theisen
  2026-09-15  9:16 ` [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
@ 2026-09-16  9:32 ` Maximiliano Sandoval
  2 siblings, 0 replies; 5+ messages in thread
From: Maximiliano Sandoval @ 2026-09-16  9:32 UTC (permalink / raw)
  To: Jonas Theisen; +Cc: pve-devel

Jonas Theisen <j.theisen@proxmox.com> writes:

> 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 is to ensure a consistent state after a change of the HTTP proxy
> without having to run a Refresh / apt update through the PVE tooling.
>
> https://bugzilla.proxmox.com/show_bug.cgi?id=8031

Looks good to me.

Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano




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

* Re: [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change
  2026-09-15  9:16 ` [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
@ 2026-10-02 10:32   ` Elias Huhsovitz
  0 siblings, 0 replies; 5+ messages in thread
From: Elias Huhsovitz @ 2026-10-02 10:32 UTC (permalink / raw)
  To: Jonas Theisen, pve-devel

I tested this using a the baisc HTTP Proxy docker image ubuntu/squid:

https://hub.docker.com/r/ubuntu/squid

comments inlide.

On Tue Sep 15, 2026 at 11:16 AM CEST, Jonas Theisen wrote:
> Writes the necessary file for APT if the http_proxy variable
> on the datacenter level is changed.
>
> This reuses the same function from the APT API for a regular
> database update.
>
> Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
> ---
>  PVE/API2/Cluster.pm | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index 4e5efbfd..efda3906 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -36,6 +36,7 @@ use PVE::API2::ClusterConfig;
>  use PVE::API2::Firewall::Cluster;
>  use PVE::API2::HAConfig;
>  use PVE::API2::ReplicationConfig;
> +use PVE::API2::APT;

nit: imports are ordered Lexicographically.

So the new import should be after ACMEPlugin, like this:

use PVE::API2::ACMEPlugin;
use PVE::API2::APT;

>  
>  my $have_sdn;
>  eval {
> @@ -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,
> @@ -859,6 +869,10 @@ __PACKAGE__->register_method({
>          );
>          die $@ if $@;
>  
> +        if ($http_proxy_change) {
> +            PVE::API2::APT::update_apt_proxy_config();
> +        }


Potential Pitfall
-----------------
The subroutine `update_apt_proxy_config` only writes the config to
the local node, not the cluster.

So we make a HTTP PUT request to the `/cluster/options` endpoint
expecting a cluster wide change, but only the node that accepted the
request actually updates their `/etc/apt/apt.conf.d/76pveproxy` config
file. 

I verified this behaviour on a 2 node cluster.

My take
-------
>From the top of my head 2 solutions come to mind:

1. Create a `update_apt_proxy_config_cluster()` subroutine that applies
the changes cluster wide. Then simply call this function, instead of
`update_apt_proxy_config()`

2. Create some kind of sync mechanism based on changes to the
datacenter.cfg. (also would need some kind of 
`update_apt_proxy_config_cluster()` subroutine)

This could consist of 2 functions:
* synch on change: You check diff between the previous datacenter.cfg
  and the new datacenter.cfg. 
  You apply the changes to all nodes in the cluster.

  e.g. if the `http_proxy` parameter is changed: update `76pveproxy` for
  all nodes in the cluster

* sync on demand: You go through the datacenter.cfg. For each config
  that needs additional settings on the host (e.g. `http_proxy`), apply
  the changes.

  e.g. I manually edit datacenter.cfg to set 
  `http_proxy: http://192.168.29.70:3128`

  I run the new subroutine `sync_datacenter_cfg`. All nodes in the
  cluster now contain:

   76pveproxy contents:
   Acquire::http::Proxy "http://192.168.29.70:3128";

  This function has to be idempotent.

  (Perhaps something like this already exists, I am not 100% sure atm)
 
  Let me know if this makes sense, or if you come up with a better
  solution!

> +
>          return undef;
>      },
>  });
	




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

end of thread, other threads:[~2026-10-02 10:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15  9:16 [PATCH pve-manager v3 0/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
2026-09-15  9:16 ` [PATCH pve-manager v3 1/2] fix #8031: factor out APT proxy write from update_database call Jonas Theisen
2026-09-15  9:16 ` [PATCH pve-manager v3 2/2] fix #8031: write APT proxy config on http_proxy change Jonas Theisen
2026-10-02 10:32   ` Elias Huhsovitz
2026-09-16  9:32 ` [PATCH pve-manager v3 0/2] " Maximiliano Sandoval

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