From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 11B5B1FF09B for ; Mon, 12 Oct 2026 18:33:31 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 87479213C3; Mon, 12 Oct 2026 18:33:28 +0200 (CEST) Message-ID: <21c83162-6324-4602-8ed8-282701020df1@proxmox.com> Date: Mon, 12 Oct 2026 18:33:22 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH pve-mamager v4 2/3] api: apt: move proxy config to pmxcfs and symlink to it To: Jonas Theisen , pve-devel@lists.proxmox.com References: <20261012145712.296002-1-j.theisen@proxmox.com> <20261012145712.296002-3-j.theisen@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20261012145712.296002-3-j.theisen@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791822802486 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.592 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: PSVTJ3OT3YGOVKR2EICHB4AKZJ4ZJYXF X-Message-ID-Hash: PSVTJ3OT3YGOVKR2EICHB4AKZJ4ZJYXF X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 12.10.26 um 16:57 schrieb 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. But your change here still needs such a refresh (or after the next patch a change in proyy value) at least once per node, this is not activated immediately automatically. > 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. typo: s/existance/existence/ > > The config is also only updated if it differs from the current > to avoid unncessarry cluster traffic. typo: s/unncessarry/unnecessary/ > > Updateing the database without write access to /etc/pve typo: s/Updateing/Updating/ > still works and only issues a warning. But not if pmxcfs fails to start, which can make disaster recovery harder. I mean, sure if pmxcfs fails to start completely it's gonna be annoying either way, but still not a good excuse to add more coupling to core system configs here. > > 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. But that's breaking existing unprivileged apt use cases then. Also cases where some nodes use a different proxy than others, bit of an odd edge case, but not nice to break, at least not as normal update during a stable release series. I'd rather skip this approach, at least in current form. A hook for apt that checks configured and setup proxy and warns on mismatch might be nicer here? If we really want to auto-apply it - which can be probably OK, I just did not got around to think it fully through - that hook could also do that and print a nice visible log line for when something was changed, making the admin updating aware of it. > > Signed-off-by: Jonas Theisen > --- > 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']; >