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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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 <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'];
>