Re: [PATCH container] fix #7164: lxc: restore: apply password and ssh keys during container restore

[Erik Fastermann <e.fastermann@proxmox.com>]

See comments inline.

> When restoring a container with `pct restore`, the `--password` and
> `--ssh-public-keys` options are silently ignored. The `create_vm` API
> endpoint only invokes the `post_create_hook` during initial container
> creation, skipping it entirely for restores.
> 
> To resolve this, make the credential updates in
> `PVE::LXC::Setup::Base::post_create_hook` conditional. The
> hook now applies the password or SSH keys only if they are explicitly
> defined.
> 
> Invoke the hook for create/restore.
> 
> Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
> ---
>   src/PVE/API2/LXC.pm       | 5 ++---
>   src/PVE/LXC/Setup/Base.pm | 2 +-
>   2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 88067dd..8e6e7e8 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -570,6 +570,7 @@ __PACKAGE__->register_method({
>                           );
>                       }
>   
> +                    my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
>                       if ($restore) {
>                           print "merging backed-up and given configuration..\n";
>                           PVE::LXC::Create::restore_configuration(
> @@ -583,13 +584,11 @@ __PACKAGE__->register_method({
>                               $skip_fw_config_restore,
>                           );
>                           PVE::LXC::create_ifaces_ipams_ips($conf, $vmid) if $unique;
> -                        my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
>                           $lxc_setup->template_fixup($conf);

Calling `template_fixup` here should have no effect as it's now called 
again by `post_create_hook` below.

>                       } else {
> -                        my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir); # detect OS
>                           PVE::LXC::Config->write_config($vmid, $conf); # safe config (after OS detection)
> -                        $lxc_setup->post_create_hook($password, $ssh_keys);
>                       }
> +                    $lxc_setup->post_create_hook($password, $ssh_keys);

Calling `post_create_hook` unconditionally has some unintended 
consequences. E.g. now `clear_machine_id` unlinks `/etc/machine-id`, 
which we probably don't want. There might be a lot of other issues, but 
I haven't looked further into this.

>                   };
>                   my $err = $@;
>                   PVE::LXC::umount_all($vmid, $storage_cfg, $conf, $err ? 1 : 0);
> diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
> index 567f9d6..13dd6b9 100644
> --- a/src/PVE/LXC/Setup/Base.pm
> +++ b/src/PVE/LXC/Setup/Base.pm
> @@ -724,7 +724,7 @@ sub post_create_hook {
>   
>       &$randomize_crontab($self, $conf);
>   
> -    $self->set_user_password($conf, 'root', $root_password);
> +    $self->set_user_password($conf, 'root', $root_password) if defined($root_password);

This breaks the non-restore path, because if the password is undef, 
`set_user_password` previously disabled the password based login by 
setting the password to an asterisk in `/etc/shadow`. Now this depends 
on the behavior of the template, as `set_user_password` is not called 
anymore in this case.

>       $self->set_user_authorized_ssh_keys($conf, 'root', $ssh_keys) if $ssh_keys;
>       $self->setup_init($conf);
>       $self->setup_network($conf);