[pve-devel] [PATCH installer] auto-installer: move ssh keys setup to low-level installer
Christoph Heiss
c.heiss at proxmox.com
Tue Apr 23 16:44:29 CEST 2024
.. 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 at proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss at 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 at 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
More information about the pve-devel
mailing list