public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer] auto-installer: move ssh keys setup to low-level installer
@ 2024-04-23 14:44 Christoph Heiss
  2024-04-23 15:26 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Christoph Heiss @ 2024-04-23 14:44 UTC (permalink / raw)
  To: pve-devel

.. thereby, also fixing a accidental shell injection.

Since run_cmd{,s}() is nowhere else used anymore, they can be removed
too.

Also mostly reverts commit

  5878dc4ae "auto-installer: handle auto-reboot info messages directly"

in the process too.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Proxmox/Install.pm                            |  7 ++
 Proxmox/Install/Config.pm                     |  4 ++
 .../src/bin/proxmox-auto-installer.rs         | 34 +--------
 proxmox-auto-installer/src/utils.rs           | 70 ++-----------------
 .../resources/parse_answer/disk_match.json    |  2 +-
 .../parse_answer/disk_match_all.json          |  2 +-
 .../parse_answer/disk_match_any.json          |  2 +-
 .../tests/resources/parse_answer/minimal.json |  2 +-
 .../resources/parse_answer/nic_matching.json  |  2 +-
 .../resources/parse_answer/specific_nic.json  |  2 +-
 .../tests/resources/parse_answer/zfs.json     |  2 +-
 proxmox-installer-common/src/setup.rs         |  2 +
 proxmox-tui-installer/src/setup.rs            |  1 +
 13 files changed, 27 insertions(+), 105 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index e2f8ad9..dcbedb2 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1271,6 +1271,13 @@ _EOD
 	my $octets = encode("utf-8", Proxmox::Install::Config::get_password());
 	run_command("chroot $targetdir /usr/sbin/chpasswd", undef, "root:$octets\n");
 
+	# set root ssh keys
+	my $ssh_keys = Proxmox::Install::Config::get_root_ssh_keys();
+	if (scalar(@$ssh_keys) > 0) {
+	    mkdir "$targetdir/root/.ssh";
+	    file_write_all("$targetdir/root/.ssh/authorized_keys", join("\n", @$ssh_keys));
+	}
+
 	my $mailto = Proxmox::Install::Config::get_mailto();
 	if ($iso_env->{product} eq 'pmg') {
 	    # save admin email
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index 5ef3438..ecd8a74 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -92,6 +92,7 @@ my sub init_cfg {
 	# root credentials & details
 	password => undef,
 	mailto => 'mail@example.invalid',
+	root_ssh_keys => [],
 
 	# network related
 	mngmt_nic => undef,
@@ -201,6 +202,9 @@ sub get_password { return get('password'); }
 sub set_mailto { set_key('mailto', $_[0]); }
 sub get_mailto { return get('mailto'); }
 
+sub set_root_ssh_keys { set_key('root_ssh_keys', $_[0]); }
+sub get_root_ssh_keys { return get('root_ssh_keys'); }
+
 sub set_mngmt_nic { set_key('mngmt_nic', $_[0]); }
 sub get_mngmt_nic { return get('mngmt_nic'); }
 
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 97b5746..2e7d20d 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -5,8 +5,6 @@ use std::{
     io::{BufRead, BufReader, Write},
     path::PathBuf,
     process::ExitCode,
-    thread,
-    time::Duration,
 };
 
 use proxmox_installer_common::setup::{
@@ -17,7 +15,7 @@ use proxmox_auto_installer::{
     answer::Answer,
     log::AutoInstLogger,
     udevinfo::UdevInfo,
-    utils::{parse_answer, run_cmds, LowLevelMessage},
+    utils::{parse_answer, LowLevelMessage},
 };
 
 static LOGGER: AutoInstLogger = AutoInstLogger;
@@ -93,15 +91,8 @@ fn main() -> ExitCode {
         }
     }
 
-    run_postinstallation(&answer);
-
     // TODO: (optionally) do a HTTP post with basic system info, like host SSH public key(s) here
 
-    for secs in (0..=5).rev() {
-        info!("Installation finished - auto-rebooting in {secs} seconds ..");
-        thread::sleep(Duration::from_secs(1));
-    }
-
     ExitCode::SUCCESS
 }
 
@@ -178,8 +169,7 @@ fn run_installation(
                     if state == "err" {
                         bail!("{message}");
                     }
-                    // Do not print anything if the installation was successful,
-                    // as we handle that here ourselves
+                    info!("Finished: '{state}' {message}");
                 }
             };
         }
@@ -187,23 +177,3 @@ fn run_installation(
     };
     inner().map_err(|err| format_err!("low level installer returned early: {err}"))
 }
-
-fn run_postinstallation(answer: &Answer) {
-    if !answer.global.root_ssh_keys.is_empty() {
-        // FIXME: move handling this into the low-level installer and just pass in installation
-        // config, as doing parts of the installation/configuration here and parts in the
-        // low-level installer is not nice (seemingly spooky actions at a distance).
-        info!("Adding root ssh-keys to the installed system ..");
-        run_cmds(
-            "ssh-key-setup",
-            true,
-            &[
-                "mkdir -p /target/root/.ssh",
-                &format!(
-                    "printf '{}' >>/target/root/.ssh/authorized_keys",
-                    answer.global.root_ssh_keys.join("\n"),
-                ),
-            ],
-        );
-    }
-}
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index b6df6c2..7e1366c 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -1,11 +1,8 @@
 use anyhow::{bail, Context as _, Result};
 use clap::ValueEnum;
 use glob::Pattern;
-use log::{debug, error, info};
-use std::{
-    collections::BTreeMap,
-    process::{Command, Stdio},
-};
+use log::info;
+use std::{collections::BTreeMap, process::Command};
 
 use crate::{
     answer::{self, Answer},
@@ -300,61 +297,6 @@ pub fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<(
     Ok(())
 }
 
-pub fn run_cmds(step: &str, in_chroot: bool, cmds: &[&str]) {
-    let run = || {
-        debug!("Running commands for '{step}':");
-        for cmd in cmds {
-            run_cmd(cmd)?;
-        }
-        Ok::<(), anyhow::Error>(())
-    };
-
-    if in_chroot {
-        if let Err(err) = run_cmd("proxmox-chroot prepare") {
-            error!("Failed to setup chroot for '{step}': {err}");
-            return;
-        }
-    }
-
-    if let Err(err) = run() {
-        error!("Running commands for '{step}' failed: {err:?}");
-    } else {
-        debug!("Running commands in chroot for '{step}' finished");
-    }
-
-    if in_chroot {
-        if let Err(err) = run_cmd("proxmox-chroot cleanup") {
-            error!("Failed to clean up chroot for '{step}': {err}");
-        }
-    }
-}
-
-fn run_cmd(cmd: &str) -> Result<()> {
-    debug!("Command '{cmd}':");
-    let child = match Command::new("/bin/bash")
-        .arg("-c")
-        .arg(cmd)
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .spawn()
-    {
-        Ok(child) => child,
-        Err(err) => bail!("error running command {cmd}: {err}"),
-    };
-    match child.wait_with_output() {
-        Ok(output) => {
-            if output.status.success() {
-                debug!("{}", String::from_utf8(output.stdout).unwrap());
-            } else {
-                bail!("{}", String::from_utf8(output.stderr).unwrap());
-            }
-        }
-        Err(err) => bail!("{err}"),
-    }
-
-    Ok(())
-}
-
 pub fn parse_answer(
     answer: &Answer,
     udev_info: &UdevInfo,
@@ -372,7 +314,7 @@ pub fn parse_answer(
     verify_locale_settings(answer, locales)?;
 
     let mut config = InstallConfig {
-        autoreboot: 0,
+        autoreboot: 1_usize,
         filesys: filesystem,
         hdsize: 0.,
         swapsize: None,
@@ -390,6 +332,7 @@ pub fn parse_answer(
 
         password: answer.global.root_password.clone(),
         mailto: answer.global.mailto.clone(),
+        root_ssh_keys: answer.global.root_ssh_keys.clone(),
 
         mngmt_nic: network_settings.ifname,
 
@@ -431,11 +374,6 @@ pub fn parse_answer(
                 .unwrap_or(runtime_info.disks[first_selected_disk].size);
         }
     }
-
-    // never print the auto reboot text after finishing to avoid the delay, as this is handled by
-    // the auto-installer itself anyway. The auto-installer might still perform some post-install
-    // steps after running the low-level installer.
-    config.autoreboot = 0;
     Ok(config)
 }
 
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
index 837de52..3a117b6 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "192.168.1.114/24",
   "country": "at",
   "dns": "192.168.1.254",
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
index 18c61c0..5325fc3 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "192.168.1.114/24",
   "country": "at",
   "dns": "192.168.1.254",
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
index a756946..18e22d1 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "192.168.1.114/24",
   "country": "at",
   "dns": "192.168.1.254",
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
index 8514199..bb72713 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "192.168.1.114/24",
   "country": "at",
   "dns": "192.168.1.254",
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
index 3635b0d..de94165 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "10.10.10.10/24",
   "country": "at",
   "dns": "10.10.10.1",
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
index b5e0de4..5b4fcfc 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "10.10.10.10/24",
   "country": "at",
   "dns": "10.10.10.1",
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/zfs.json b/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
index 508ad69..65724a8 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
@@ -1,5 +1,5 @@
 {
-  "autoreboot": 0,
+  "autoreboot": 1,
   "cidr": "192.168.1.114/24",
   "country": "at",
   "dns": "192.168.1.254",
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 98cd47c..64d05af 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -487,6 +487,8 @@ pub struct InstallConfig {
 
     pub password: String,
     pub mailto: String,
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub root_ssh_keys: Vec<String>,
 
     pub mngmt_nic: String,
 
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index 5d786b7..8c01e42 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -25,6 +25,7 @@ impl From<InstallerOptions> for InstallConfig {
 
             password: options.password.root_password,
             mailto: options.password.email,
+            root_ssh_keys: vec![],
 
             mngmt_nic: options.network.ifname,
 
-- 
2.44.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-23 15:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-23 14:44 [pve-devel] [PATCH installer] auto-installer: move ssh keys setup to low-level installer Christoph Heiss
2024-04-23 15:26 ` [pve-devel] applied: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal