[pve-devel] [PATCH common 2/2] REST environment: default to 'root at pam' for forked workers in case no user was specified

[Fabian Ebner]

Am 07.03.22 um 10:20 schrieb Oguz Bektas:
> previously we had a default of 'root at pve', which doesn't exist.
> since the username is only relevant for the task logs, we can change it
> to 'root at pam' without ill effects.
> 
> also add a warning in case there are other call sites that we missed
> where fork_worker is called without a user variable (found call sites
> only in pve-container where this was unset, namely in 'push_file' and
> 'pull_file').
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  src/PVE/RESTEnvironment.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
> index 1b2af08..5352aad 100644
> --- a/src/PVE/RESTEnvironment.pm
> +++ b/src/PVE/RESTEnvironment.pm
> @@ -492,7 +492,10 @@ sub fork_worker {
>      $dtype = 'unknown' if !defined ($dtype);
>      $id = '' if !defined ($id);
>  
> -    $user = 'root at pve' if !defined ($user);

It'd be a bit cleaner to default to $self->get_user(1) first and only if
that's not set to something else. Maybe the warning is only needed in
the latter case. Would also make the first patch unnecessary, although
it doesn't hurt of course.

> +    if (!defined($user)) {
> +	warn 'Worker user was not specified, defaulting to "root at pam"!';

Could mention that it's an internal error. And could use $self->warn().

> +	$user = 'root at pam';
> +    }
>  
>      my $sync = ($self->{type} eq 'cli' && !$background) ? 1 : 0;
>