From 35a504198b548d8dc8980489a1f28d523ec55001 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 31 Jul 2024 09:58:02 -0700 Subject: [PATCH] zone-setup cleanup/refactoring (#6175) (This is a rebased and expanded version of #6015 now that the self-assembling switch zone has landed.) I need to make some changes to zone-setup to support Reconfigurator rebalancing boundary NTP zones, and found it a little trickier to work with than expected. I think there were two reasons for this: * The use of clap's builder interface instead of derive (addressed in https://github.com/oxidecomputer/omicron/commit/3708774aefde0aa6abcb8d12321c993f6268b4ae), which requires a surprising amount of boilerplate and custom parsing functions, most of which are now gone * Using Result<(), CmdError::Failure> throughout, when really only main cares about that and everywhere else could use anyhow::Result<()> (mostly addressed in https://github.com/oxidecomputer/omicron/commit/cac26ddcd3e2345780d6f29863263b0611396f24); I also tried to make sure we always attach some .context() to errors (the old code did this in some cases but not all) I also made a cleanup pass through the new switch zone user setup code; the biggest win there was reusing `illumos_utils::execute()` to check for success and build detailed error messages on failure. There are some other minor changes like long flags with different names (AFAICT we only ever use the short flags in SMF manifests, so this shouldn't require other changes) and potential bugfixes (e.g., the switch zone setup code was only correct for 0 or 1 profile, and should now be correct for 2+ if we ever need more than one). While all these _should_ be correct, I want to run this through madrid before merging. I think it's ready for review, though; will update here once I do the madrid test. --- Cargo.lock | 2 + illumos-utils/src/route.rs | 1 + zone-setup/Cargo.toml | 4 +- zone-setup/src/bin/zone-setup.rs | 956 +++++++++++++---------------- zone-setup/src/switch_zone_user.rs | 334 ++++------ 5 files changed, 543 insertions(+), 754 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab73ab9242..30d00d1256 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11775,12 +11775,14 @@ name = "zone-setup" version = "0.1.0" dependencies = [ "anyhow", + "camino", "clap", "dropshot", "illumos-utils", "omicron-common", "omicron-sled-agent", "omicron-workspace-hack", + "oxnet", "serde_json", "sled-hardware-types", "slog", diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 12f74bfd78..e49bda2210 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -12,6 +12,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; /// Wraps commands for interacting with routing tables. pub struct Route {} +#[derive(Debug, Clone, Copy)] pub enum Gateway { Ipv4(Ipv4Addr), Ipv6(Ipv6Addr), diff --git a/zone-setup/Cargo.toml b/zone-setup/Cargo.toml index c0c6fd6cb4..969254d0fe 100644 --- a/zone-setup/Cargo.toml +++ b/zone-setup/Cargo.toml @@ -9,15 +9,17 @@ workspace = true [dependencies] anyhow.workspace = true +camino.workspace = true clap.workspace = true dropshot.workspace = true illumos-utils.workspace = true omicron-common.workspace = true omicron-workspace-hack.workspace = true +oxnet.workspace = true serde_json.workspace = true sled-hardware-types.workspace = true slog.workspace = true tokio.workspace = true uzers.workspace = true zone.workspace = true -omicron-sled-agent.workspace = true \ No newline at end of file +omicron-sled-agent.workspace = true diff --git a/zone-setup/src/bin/zone-setup.rs b/zone-setup/src/bin/zone-setup.rs index 800dcfa368..f335512d83 100644 --- a/zone-setup/src/bin/zone-setup.rs +++ b/zone-setup/src/bin/zone-setup.rs @@ -4,8 +4,11 @@ //! CLI to set up zone configuration -use anyhow::anyhow; -use clap::{arg, command, value_parser, Arg, ArgMatches, Command}; +use anyhow::{anyhow, Context}; +use clap::builder::{ + NonEmptyStringValueParser, StringValueParser, TypedValueParser, +}; +use clap::{ArgAction, Args, Parser, Subcommand}; use illumos_utils::addrobj::{AddrObject, IPV6_LINK_LOCAL_ADDROBJ_NAME}; use illumos_utils::ipadm::Ipadm; use illumos_utils::route::{Gateway, Route}; @@ -16,290 +19,189 @@ use omicron_common::backoff::{retry_notify, retry_policy_local, BackoffError}; use omicron_common::cmd::fatal; use omicron_common::cmd::CmdError; use omicron_sled_agent::services::SWITCH_ZONE_BASEBOARD_FILE; -use serde_json::Value; +use oxnet::Ipv6Net; use sled_hardware_types::underlay::BOOTSTRAP_PREFIX; use slog::{info, Logger}; +use std::fmt::Write as _; use std::fs::{metadata, read_to_string, set_permissions, write, OpenOptions}; -use std::io::Write; +use std::io::Write as _; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::os::unix::fs::chown; -use std::path::Path; +use std::path::{Path, PathBuf}; use uzers::{get_group_by_name, get_user_by_name}; use zone_setup::switch_zone_user::SwitchZoneUser; -pub const HOSTS_FILE: &str = "/etc/inet/hosts"; -pub const CHRONY_CONFIG_FILE: &str = "/etc/inet/chrony.conf"; -pub const LOGADM_CONFIG_FILE: &str = "/etc/logadm.d/chrony.logadm.conf"; -pub const ROOT: &str = "root"; -pub const SYS: &str = "sys"; - -pub const COMMON_NW_CMD: &str = "common-networking"; -pub const OPTE_INTERFACE_CMD: &str = "opte-interface"; -pub const CHRONY_SETUP_CMD: &str = "chrony-setup"; -pub const SWITCH_ZONE_SETUP_CMD: &str = "switch-zone"; - -fn parse_ip(s: &str) -> anyhow::Result { - if s == "unknown" { - return Err(anyhow!("ERROR: Missing input value")); - }; - s.parse().map_err(|_| anyhow!("ERROR: Invalid IP address")) +#[derive(Debug, Parser)] +struct ZoneSetup { + #[command(subcommand)] + command: ZoneSetupCommand, } -fn parse_ipv4(s: &str) -> anyhow::Result { - if s == "unknown" { - return Err(anyhow!("ERROR: Missing input value")); - }; - s.parse().map_err(|_| anyhow!("ERROR: Invalid IPv4 address")) +#[derive(Debug, Subcommand)] +enum ZoneSetupCommand { + /// Sets up common networking configuration across all zones + CommonNetworking(CommonNetworkingArgs), + /// Sets up OPTE interface + OpteInterface(OpteInterfaceArgs), + /// Sets up Chrony configuration for NTP zone + ChronySetup(ChronySetupArgs), + /// Sets up switch zone configuration + SwitchZone(SwitchZoneArgs), } -fn parse_ipv6(s: &str) -> anyhow::Result { - if s == "unknown" { - return Err(anyhow!("ERROR: Missing input value")); - }; - s.parse().map_err(|_| anyhow!("ERROR: Invalid IPv6 address")) +// Some of our `String` cli arguments may be set to the literal "unknown" if +// sled-agent hasn't populated our SMF config correctly. Reject this at argument +// parsing time. +fn parse_string_rejecting_unknown(s: &str) -> anyhow::Result { + if s.is_empty() || s == "unknown" { + Err(anyhow!("missing input value")) + } else { + Ok(s.to_string()) + } } -fn parse_datalink(s: &str) -> anyhow::Result { - if s == "unknown" { - return Err(anyhow!("ERROR: Missing data link")); - }; - s.parse().map_err(|_| anyhow!("ERROR: Invalid data link")) +#[derive(Debug, Args)] +struct CommonNetworkingArgs { + #[arg(short, long, value_parser = parse_string_rejecting_unknown)] + datalink: String, + // The `num_args = 0..=1` here allows passing the `-g` gateway flag with no + // value; the SMF manifest for the switch zone will do this when the + // underlay isn't up yet. + #[arg(short, long, num_args = 0..=1)] + gateway: Option, + /// list of static addresses + #[arg(short, long, num_args = 1..)] + static_addrs: Vec, } -fn parse_opte_iface(s: &str) -> anyhow::Result { - if s == "unknown" { - return Err(anyhow!("ERROR: Missing OPTE interface")); - }; - s.parse().map_err(|_| anyhow!("ERROR: Invalid OPTE interface")) +#[derive(Debug, Args)] +struct OpteInterfaceArgs { + #[arg(short, long, value_parser = parse_string_rejecting_unknown)] + interface: String, + /// OPTE-specific gateway for external connectivity via boundary services + #[arg(short, long)] + gateway: Ipv4Addr, + #[arg(short = 'p', long)] + ip: IpAddr, } -fn parse_chrony_conf(s: &str) -> anyhow::Result { - if s == "" { - return Err(anyhow!("ERROR: Missing chrony configuration file")); - }; - - s.parse().map_err(|_| anyhow!("ERROR: Invalid chrony configuration file")) +#[derive(Debug, Args)] +struct ChronySetupArgs { + /// chrony configuration file + #[arg(short, long, default_value = CHRONY_CONFIG_FILE)] + file: PathBuf, + /// whether this is a boundary or internal NTP zone + #[arg(short, long, action = ArgAction::Set)] + boundary: bool, + /// list of NTP servers + #[arg( + short, + long, + num_args = 1.., + value_parser = NonEmptyStringValueParser::default(), + )] + servers: Vec, + /// allowed IPv6 range + #[arg(short, long)] + allow: Ipv6Net, } -fn parse_wicket_conf(s: &str) -> anyhow::Result { - if s == "" { - return Err(anyhow!("ERROR: Missing baseboard configuration file")); - }; - - s.parse() - .map_err(|_| anyhow!("ERROR: Invalid baseboard configuration file")) +// The default clap parser for `serde_json::Value` is to wrap the argument in a +// `Value::String`; this value parser parses the argument as JSON instead. +fn json_value_parser() -> impl TypedValueParser { + StringValueParser::new().try_map(|s| s.parse()) } -fn parse_baseboard_info(s: &str) -> anyhow::Result { - if s == "" { - return Err(anyhow!("ERROR: Missing baseboard information")); - }; - - let _: Value = serde_json::from_str(s) - .map_err(|_| anyhow!("ERROR: Value cannot be parsed as JSON"))?; - - s.parse() - .map_err(|_| anyhow!("ERROR: Invalid baseboard configuration file")) +#[derive(Debug, Args)] +struct SwitchZoneArgs { + /// path to the baseboard file + #[arg(short, long, default_value = SWITCH_ZONE_BASEBOARD_FILE)] + baseboard_file: PathBuf, + /// baseboard info JSON blob + #[arg(short = 'i', long, value_parser = json_value_parser())] + baseboard_info: serde_json::Value, + /// bootstrap IPv6 address + #[arg(short = 'a', long)] + bootstrap_addr: Ipv6Addr, + /// bootstrap VNIC name + #[arg( + short = 'v', + long, + value_parser = NonEmptyStringValueParser::default(), + )] + bootstrap_vnic: String, + /// global zone local link IPv6 address + #[arg(short, long)] + gz_local_link_addr: Ipv6Addr, + /// list of links that require link local addresses + #[arg( + short, + long, + num_args = 0.., + value_parser = NonEmptyStringValueParser::default(), + )] + link_local_links: Vec, } -fn parse_boundary(s: &str) -> anyhow::Result { - s.parse().map_err(|_| anyhow!("ERROR: Invalid boundary input")) -} +const HOSTS_FILE: &str = "/etc/inet/hosts"; +const CHRONY_CONFIG_FILE: &str = "/etc/inet/chrony.conf"; +const LOGADM_CONFIG_FILE: &str = "/etc/logadm.d/chrony.logadm.conf"; +const ROOT: &str = "root"; +const SYS: &str = "sys"; #[tokio::main] async fn main() { - if let Err(message) = do_run().await { - fatal(message); + if let Err(err) = do_run().await { + fatal(CmdError::Failure(err)); } } -async fn do_run() -> Result<(), CmdError> { +async fn do_run() -> anyhow::Result<()> { let log = dropshot::ConfigLogging::File { path: "/dev/stderr".into(), level: dropshot::ConfigLoggingLevel::Info, if_exists: dropshot::ConfigLoggingIfExists::Append, } .to_logger("zone-setup") - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - - let matches = command!() - .subcommand( - Command::new(COMMON_NW_CMD) - .about( - "Sets up common networking configuration across all zones", - ) - .arg( - arg!( - -d --datalink "datalink" - ) - .required(true) - .value_parser(parse_datalink), - ) - .arg( - // We are taking a list of values so that clap - // allows us to set a flag without values in the - // SMF manifest. This will happen with the switch - // zone when the underlay isn't up yet. - Arg::new("gateway") - .short('g') - .long("gateway") - .num_args(0..=1) - .value_parser(value_parser!(Ipv6Addr)) - .help("Underlay address") - ) - .arg( - Arg::new("static_addrs") - .short('s') - .long("static_addrs") - .num_args(1..) - .value_delimiter(' ') - .value_parser(parse_ipv6) - .help("List of static addresses separated by a space") - .required(true) - ), - ) - .subcommand( - Command::new(OPTE_INTERFACE_CMD) - .about("Sets up OPTE interface") - .arg( - arg!( - -i --opte_interface "opte_interface" - ) - .required(true) - .value_parser(parse_opte_iface), - ) - .arg( - arg!( - -g --opte_gateway "opte_gateway" - ) - .required(true) - .value_parser(parse_ipv4), - ) - .arg( - arg!( - -p --opte_ip "opte_ip" - ) - .required(true) - .value_parser(parse_ip), - ), - ) - .subcommand( - Command::new(SWITCH_ZONE_SETUP_CMD) - .about("Sets up switch zone configuration") - .arg( - arg!( - -b --baseboard_file "baseboard_file" - ) - .default_value(SWITCH_ZONE_BASEBOARD_FILE) - .value_parser(parse_wicket_conf), - ) - .arg( - arg!( - -i --baseboard_info "baseboard_info" - ) - .required(true) - .value_parser(parse_baseboard_info), - ) - .arg( - arg!( - -a --bootstrap_addr "bootstrap_addr" - ) - .required(true) - .value_parser(parse_ipv6), - ) - .arg( - arg!( - -v --bootstrap_vnic "bootstrap_vnic" - ) - .required(true), - ) - .arg( - arg!( - -g --gz_local_link_addr "gz_local_link_addr" - ) - .required(true) - .value_parser(parse_ipv6), - ) - .arg( - Arg::new("link_local_links") - .short('l') - .long("link_local_links") - .num_args(0..) - .value_delimiter(' ') - .value_parser(value_parser!(String)) - .help("List of links that require link local addresses") - ), - ) - .subcommand( - Command::new(CHRONY_SETUP_CMD) - .about("Sets up Chrony configuration for NTP zone") - .arg( - arg!(-f --file "Chrony configuration file") - .default_value(CHRONY_CONFIG_FILE) - .value_parser(parse_chrony_conf) - ) - .arg( - arg!(-b --boundary "Whether this is a boundary or internal NTP zone") - .required(true) - .value_parser(parse_boundary), - ) - .arg( - Arg::new("servers") - .short('s') - .long("servers") - .num_args(1..) - .value_delimiter(' ') - .value_parser(value_parser!(String)) - .help("List of NTP servers separated by a space") - .required(true) - ) - .arg( - arg!(-a --allow "Allowed IPv6 range") - .num_args(0..=1) - ), - ) - .get_matches(); + .context("failed to construct stderr logger")?; - if let Some(matches) = matches.subcommand_matches(COMMON_NW_CMD) { - common_nw_set_up(matches, log.clone()).await?; - } + let args = ZoneSetup::parse(); - if let Some(matches) = matches.subcommand_matches(OPTE_INTERFACE_CMD) { - opte_interface_set_up(matches, log.clone()).await?; - } - - if let Some(matches) = matches.subcommand_matches(CHRONY_SETUP_CMD) { - chrony_setup(matches, log.clone()).await?; - } - - if let Some(matches) = matches.subcommand_matches(SWITCH_ZONE_SETUP_CMD) { - switch_zone_setup(matches, log.clone()).await?; + match args.command { + ZoneSetupCommand::CommonNetworking(args) => { + common_nw_set_up(args, &log).await + } + ZoneSetupCommand::OpteInterface(args) => { + opte_interface_set_up(args, &log).await + } + ZoneSetupCommand::ChronySetup(args) => chrony_setup(args, &log).await, + ZoneSetupCommand::SwitchZone(args) => { + switch_zone_setup(args, &log).await + } } - - Ok(()) } async fn switch_zone_setup( - matches: &ArgMatches, - log: Logger, -) -> Result<(), CmdError> { - let file: &String = matches.get_one("baseboard_file").unwrap(); - let info: &String = matches.get_one("baseboard_info").unwrap(); - let bootstrap_addr: &Ipv6Addr = matches.get_one("bootstrap_addr").unwrap(); - let bootstrap_vnic: &String = matches.get_one("bootstrap_vnic").unwrap(); - let gz_local_link_addr: &Ipv6Addr = - matches.get_one("gz_local_link_addr").unwrap(); - let links = if let Some(l) = matches.get_many::("link_local_links") - { - Some(l.collect::>()) - } else { - None - }; + args: SwitchZoneArgs, + log: &Logger, +) -> anyhow::Result<()> { + let SwitchZoneArgs { + baseboard_file: file, + baseboard_info: info, + bootstrap_addr, + bootstrap_vnic, + gz_local_link_addr, + link_local_links: links, + } = args; - info!(&log, "Generating baseboard.json file"; "baseboard file" => ?file, "baseboard info" => ?info); - generate_switch_zone_baseboard_file(file, info)?; + info!( + log, "Generating baseboard.json file"; + "baseboard file" => %file.display(), + "baseboard info" => ?info, + ); + generate_switch_zone_baseboard_file(&file, info)?; - info!(&log, "Setting up the users required for wicket and support"); + info!(log, "Setting up the users required for wicket and support"); let wicket_user = SwitchZoneUser::new( "wicket".to_string(), "wicket".to_string(), @@ -315,178 +217,149 @@ async fn switch_zone_setup( false, "/bin/bash".to_string(), ) - .with_homedir("/home/support".to_string()) + .with_homedir("/home/support".into()) .with_profiles(vec!["Primary Administrator".to_string()]); - let users = vec![wicket_user, support_user]; + let users = [wicket_user, support_user]; for u in users { - u.setup_switch_zone_user(&log)?; + u.setup_switch_zone_user(log)?; } - if let Some(links) = links { - info!(&log, "Ensuring link local links"; "links" => ?links); + if links.is_empty() { + info!(log, "No link local links to be configured"); + } else { + info!( + log, "Ensuring link local links"; + "links" => ?links, + ); for link in &links { - Zones::ensure_has_link_local_v6_address( - None, - &AddrObject::new(link, IPV6_LINK_LOCAL_ADDROBJ_NAME).unwrap(), - ) - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not ensure link local link {:?}: {}", - links, - err - )) + let addrobj = AddrObject::new(link, IPV6_LINK_LOCAL_ADDROBJ_NAME) + .with_context(|| { + format!("invalid link name for addrobj: {link:?}") })?; + Zones::ensure_has_link_local_v6_address(None, &addrobj) + .with_context(|| { + format!("Could not ensure link local link {link:?}") + })?; } - } else { - info!(&log, "No link local links to be configured"); - }; + } - info!(&log, "Ensuring bootstrap address exists in zone"; - "bootstrap address" => ?bootstrap_addr, "bootstrap vnic" => ?bootstrap_vnic); + info!( + log, "Ensuring bootstrap address exists in zone"; + "bootstrap address" => ?bootstrap_addr, + "bootstrap vnic" => ?bootstrap_vnic, + ); let addrtype = - AddressRequest::new_static(std::net::IpAddr::V6(*bootstrap_addr), None); + AddressRequest::new_static(std::net::IpAddr::V6(bootstrap_addr), None); let addrobj_name = "bootstrap6"; let addrobj = - AddrObject::new(&bootstrap_vnic, addrobj_name).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not create new addrobj {:?}: {}", - addrobj_name, - err - )) + AddrObject::new(&bootstrap_vnic, addrobj_name).with_context(|| { + format!("Could not create new addrobj {addrobj_name:?}") })?; - let _ = Zones::create_address_internal(None, &addrobj, addrtype).map_err( - |err| { - CmdError::Failure(anyhow!( - "Could not create bootstrap address {} {:?}: {}", - addrobj, - addrtype, - err - )) - }, - )?; + let _ = Zones::create_address_internal(None, &addrobj, addrtype) + .with_context(|| { + format!("Could not create bootstrap address {addrobj} {addrtype:?}") + })?; info!( - &log, - "Forwarding bootstrap traffic via {} to {}", - bootstrap_vnic, - gz_local_link_addr + log, + "Forwarding bootstrap traffic via {bootstrap_vnic} to \ + {gz_local_link_addr}", ); Route::add_bootstrap_route( BOOTSTRAP_PREFIX, - *gz_local_link_addr, + gz_local_link_addr, &bootstrap_vnic, ) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + .with_context(|| { + format!( + "Could not add bootstrap route via {bootstrap_vnic} \ + to {gz_local_link_addr}" + ) + })?; Ok(()) } fn generate_switch_zone_baseboard_file( - file: &String, - info: &String, -) -> Result<(), CmdError> { - let mut config_file = OpenOptions::new() + file: &Path, + info: serde_json::Value, +) -> anyhow::Result<()> { + let config_file = OpenOptions::new() .write(true) .create(true) .truncate(true) .open(file) - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not create baseboard configuration file {}: {}", - file, - err - )) + .with_context(|| { + format!( + "Could not create baseboard configuration file {}", + file.display(), + ) })?; - config_file.write(info.as_bytes()).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not write to baseboard configuration file {}: {}", - file, - err - )) + serde_json::to_writer(config_file, &info).with_context(|| { + format!( + "Could not write to baseboard configuration file {}", + file.display(), + ) })?; Ok(()) } async fn chrony_setup( - matches: &ArgMatches, - log: Logger, -) -> Result<(), CmdError> { - let servers = - matches.get_many::("servers").unwrap().collect::>(); - let allow: Option<&String> = matches.get_one("allow"); - - let file: &String = matches.get_one("file").unwrap(); - let is_boundary: &bool = matches.get_one("boundary").unwrap(); - println!( - "servers: {:?}\nfile: {}\nallow: {:?}\nboundary: {:?}", - servers, file, allow, is_boundary - ); + args: ChronySetupArgs, + log: &Logger, +) -> anyhow::Result<()> { + info!(log, "running chrony setup"; "args" => ?args); - generate_chrony_config(&log, is_boundary, allow, file, servers)?; + generate_chrony_config(args, log)?; // The NTP zone delivers a logadm fragment into /etc/logadm.d/ that needs to - // be added to the system's /etc/logadm.conf. Unfortunately, the service which - // does this - system/logadm-upgrade - only processes files with mode 444 and - // root:sys ownership so we need to adjust things here (until omicron package - // supports including ownership and permissions in the generated tar files). - info!(&log, "Setting mode 444 and root:sys ownership to logadm fragment file"; "logadm config" => ?LOGADM_CONFIG_FILE); + // be added to the system's /etc/logadm.conf. Unfortunately, the service + // which does this - system/logadm-upgrade - only processes files with mode + // 444 and root:sys ownership so we need to adjust things here (until + // omicron package supports including ownership and permissions in the + // generated tar files). + info!( + log, "Setting mode 444 and root:sys ownership to logadm fragment file"; + "logadm config" => ?LOGADM_CONFIG_FILE, + ); set_permissions_for_logadm_config()?; - info!(&log, "Updating logadm"; "logadm config" => ?LOGADM_CONFIG_FILE); + info!( + log, "Updating logadm"; + "logadm config" => ?LOGADM_CONFIG_FILE, + ); Svcadm::refresh_logadm_upgrade() - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + .context("failed to refresh logadm-upgrade")?; Ok(()) } -fn set_permissions_for_logadm_config() -> Result<(), CmdError> { +fn set_permissions_for_logadm_config() -> anyhow::Result<()> { let mut perms = metadata(LOGADM_CONFIG_FILE) - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not retrieve chrony logadm configuration file {} metadata: {}", - LOGADM_CONFIG_FILE, - err - )) + .with_context(|| { + format!("failed to read metadata of {LOGADM_CONFIG_FILE}") })? .permissions(); perms.set_readonly(true); - set_permissions(LOGADM_CONFIG_FILE, perms).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not set 444 permissions on chrony logadm configuration file {}: {}", - LOGADM_CONFIG_FILE, - err - )) - })?; + set_permissions(LOGADM_CONFIG_FILE, perms).with_context(|| { + format!("failed to set 444 permissions on {LOGADM_CONFIG_FILE}") + })?; - let root_uid = match get_user_by_name(ROOT) { - Some(user) => user.uid(), - None => { - return Err(CmdError::Failure(anyhow!(format!( - "Could not retrieve ID from user: {}", - ROOT - )))) - } - }; + let root_uid = get_user_by_name(ROOT) + .with_context(|| format!("failed to look up user {ROOT}"))? + .uid(); - let sys_gid = match get_group_by_name(SYS) { - Some(group) => group.gid(), - None => { - return Err(CmdError::Failure(anyhow!(format!( - "Could not retrieve ID from group: {}", - SYS - )))) - } - }; + let sys_gid = get_group_by_name(SYS) + .with_context(|| format!("failed to look up group {SYS}"))? + .gid(); - chown(LOGADM_CONFIG_FILE, Some(root_uid), Some(sys_gid)).map_err( - |err| { - CmdError::Failure(anyhow!( - "Could not set ownership of logadm configuration file {}: {}", - LOGADM_CONFIG_FILE, - err - )) + chown(LOGADM_CONFIG_FILE, Some(root_uid), Some(sys_gid)).with_context( + || { + format!( + "could not set ownership to {ROOT}:{SYS} on {LOGADM_CONFIG_FILE}" + ) }, )?; @@ -494,14 +367,10 @@ fn set_permissions_for_logadm_config() -> Result<(), CmdError> { } fn generate_chrony_config( + args: ChronySetupArgs, log: &Logger, - is_boundary: &bool, - allow: Option<&String>, - file: &String, - servers: Vec<&String>, -) -> Result<(), CmdError> { - let internal_ntp_tpl = String::from( - "# +) -> anyhow::Result<()> { + let internal_ntp_tpl = "# # Configuration file for an internal NTP server - one which communicates with # boundary NTP servers within the rack. # @@ -527,11 +396,9 @@ makestep 1.0 3 leapsecmode slew maxslewrate 2708.333 -", - ); +"; - let boundary_ntp_tpl = String::from( - "# + let boundary_ntp_tpl = "# # Configuration file for a boundary NTP server - one which communicates with # NTP servers outside the rack. # @@ -573,42 +440,44 @@ makestep 0.1 3 leapsecmode slew maxslewrate 2708.333 -", - ); +"; - let mut contents = - if *is_boundary { boundary_ntp_tpl } else { internal_ntp_tpl }; + let ChronySetupArgs { + file: config_path, + boundary: is_boundary, + servers, + allow, + } = args; - if let Some(allow) = allow { - contents = contents.replace("@ALLOW@", &allow.to_string()); - } + let mut new_config = + if is_boundary { boundary_ntp_tpl } else { internal_ntp_tpl } + .to_string(); + + new_config = new_config.replace("@ALLOW@", &allow.to_string()); - let new_config = if *is_boundary { + if is_boundary { for s in servers { - let str_line = format!( - "pool {} iburst maxdelay 0.1 minpoll 0 maxpoll 3 maxsources 16\n", - s - ); - contents.push_str(&str_line) + writeln!( + &mut new_config, + "pool {s} iburst maxdelay 0.1 minpoll 0 maxpoll 3 maxsources 16" + ) + .expect("write to String is infallible"); } - contents } else { for s in servers { - let str_line = format!("server {} iburst minpoll 0 maxpoll 4\n", s); - contents.push_str(&str_line) + writeln!(&mut new_config, "server {s} iburst minpoll 0 maxpoll 4") + .expect("write to String is infallible"); } - contents - }; + } // We read the contents from the old configuration file if it existed // so that we can verify if it changed. - let old_file = if Path::exists(Path::new(file)) { - Some(read_to_string(file).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not read old chrony configuration file {}: {}", - file, - err - )) + let old_file = if config_path.exists() { + Some(read_to_string(&config_path).with_context(|| { + format!( + "failed reading old chrony config file {}", + config_path.display(), + ) })?) } else { None @@ -618,142 +487,109 @@ maxslewrate 2708.333 .write(true) .create(true) .truncate(true) - .open(file) - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not create chrony configuration file {}: {}", - file, - err - )) + .open(&config_path) + .with_context(|| { + format!( + "failed to create chrony config config_path {}", + config_path.display(), + ) })?; - config_file.write(new_config.as_bytes()).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not write to chrony configuration file {}: {}", - file, - err - )) + config_file.write(new_config.as_bytes()).with_context(|| { + format!( + "failed writing chrony configuration file {}", + config_path.display(), + ) })?; - if old_file.clone().is_some_and(|f| f != new_config) { - info!(&log, "Chrony configuration file has changed"; - "old configuration file" => ?old_file, "new configuration file" => ?new_config,); + if let Some(old_config) = old_file { + if old_config != new_config { + info!( + log, "Chrony configuration file has changed"; + "old configuration file" => ?old_config, + "new configuration file" => ?new_config, + ); + } } Ok(()) } async fn common_nw_set_up( - matches: &ArgMatches, - log: Logger, -) -> Result<(), CmdError> { - let datalink: &String = matches.get_one("datalink").unwrap(); - let static_addrs = matches - .get_many::("static_addrs") - .unwrap() - .collect::>(); - let gateway = if let Some(g) = matches.get_many::("gateway") { - Some(g.collect::>()) - } else { - None - }; - let zonename = zone::current().await.map_err(|err| { - CmdError::Failure(anyhow!( - "Could not determine local zone name: {}", - err - )) - })?; + args: CommonNetworkingArgs, + log: &Logger, +) -> anyhow::Result<()> { + let zonename = + zone::current().await.context("could not determine local zone name")?; + let CommonNetworkingArgs { datalink, gateway, static_addrs } = args; info!( - &log, + log, "Ensuring IP interface exists on datalink"; - "datalink" => datalink + "datalink" => &datalink ); - Ipadm::ensure_ip_interface_exists(datalink) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + Ipadm::ensure_ip_interface_exists(&datalink).with_context(|| { + format!( + "failed to ensure temporary IP interface on datalink {datalink}", + ) + })?; - info!(&log, "Setting MTU to 9000 for IPv6 and IPv4"; "datalink" => ?datalink); + info!( + log, "Setting MTU to 9000 for IPv6 and IPv4"; + "datalink" => &datalink, + ); Ipadm::set_interface_mtu(&datalink) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + .with_context(|| format!("failed to set MTU on datalink {datalink}"))?; if static_addrs.is_empty() { info!( - &log, - "No static addresses provided, will not ensure static and auto-configured addresses are set on the IP interface" + log, + "No static addresses provided; will not ensure static and \ + auto-configured addresses are set on the IP interface" ); + } else { + for &addr in &static_addrs { + if addr != Ipv6Addr::LOCALHOST { + info!( + log, + "Ensuring static and auto-configured addresses are set on \ + the IP interface"; + "data link" => ?datalink, + "static address" => %addr); + Ipadm::create_static_and_autoconfigured_addrs(&datalink, &addr) + .with_context(|| { + format!( + "failed to ensure static address {addr} on \ + datalink {datalink}", + ) + })?; + } else { + info!( + log, + "Static address is localhost; will not ensure it's set on \ + the IP interface" + ); + } + } } - for addr in &static_addrs { - if **addr != Ipv6Addr::LOCALHOST { - info!( - &log, - "Ensuring static and auto-configured addresses are set on the IP interface"; - "data link" => ?datalink, - "static address" => ?addr, - ); - Ipadm::create_static_and_autoconfigured_addrs(&datalink, addr) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - } else { + match gateway { + // Only the switch zone will sometimes have an unknown underlay address + // at zone boot. + None => { info!( - &log, - "Static address is localhost, will not ensure it's set on the IP interface" + log, + "Underlay is not available yet; will not ensure default route", ); } - } - - match gateway { - // Only the switch zone will sometimes have an unknown underlay address at zone boot. - None => info!(&log, "Underlay is not available yet. Not ensuring there is a default route"), Some(gw) => { - if gw.is_empty() { - info!(&log, "Underlay is not available yet. Not ensuring there is a default route"); - } else { - // We can safely retrieve the first address only as the CLI only accepts a single item. - let gw = gw.first().unwrap(); - - // Ensuring default route with gateway must happen after peer agents have been initialized. - // Omicron zones will be able ensure a default route with gateway immediately, but the - // switch zone on the secondary scrimlet might need a few tries while it waits. - retry_notify( - retry_policy_local(), - || async { - info!(&log, "Ensuring there is a default route"; "gateway" => ?gw); - Route::ensure_default_route_with_gateway(Gateway::Ipv6(**gw)) - .map_err(|err| { - match err { - ExecutionError::CommandFailure(ref e) => { - if e.stdout.contains("Network is unreachable") { - BackoffError::transient( - CmdError::Failure(anyhow!(err)), - ) - } else { - BackoffError::permanent( - CmdError::Failure(anyhow!(err)), - ) - } - } - _ => { - BackoffError::permanent( - CmdError::Failure(anyhow!(err)), - ) - } - } - }) - }, - |err, delay| { - info!( - &log, - "Cannot ensure there is a default route yet (retrying in {:?})", - delay; - "error" => ?err - ); - }, - ) - .await?; - } + ensure_default_route_via_gateway_with_retries(gw, log).await?; } } - info!(&log, "Populating hosts file for zone"; "zonename" => ?zonename); + info!( + log, "Populating hosts file for zone"; + "zonename" => ?zonename, + ); let mut hosts_contents = String::from( r#" ::1 localhost loghost @@ -761,39 +597,101 @@ async fn common_nw_set_up( "#, ); - for addr in static_addrs.clone() { - let s = format!( - r#"{addr} {zonename}.local {zonename} -"# - ); - hosts_contents.push_str(s.as_str()) + for addr in static_addrs { + writeln!(&mut hosts_contents, "{addr} {zonename}.local {zonename}") + .expect("write to String is infallible"); } write(HOSTS_FILE, hosts_contents) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + .with_context(|| format!("failed to write hosts file {HOSTS_FILE}"))?; Ok(()) } +async fn ensure_default_route_via_gateway_with_retries( + gateway: Ipv6Addr, + log: &Logger, +) -> anyhow::Result<()> { + // Helper to attach error context in the retry loop below. + let err_with_context = |err: ExecutionError| { + anyhow!(err).context(format!( + "failed to ensure default route via gateway {gateway}", + )) + }; + + let gateway = Gateway::Ipv6(gateway); + + // Ensuring default route with gateway must happen after peer agents + // have been initialized. Omicron zones will be able ensure a + // default route with gateway immediately, but the switch zone on + // the secondary scrimlet might need a few tries while it waits. + retry_notify( + retry_policy_local(), + || async { + info!( + log, "Ensuring there is a default route"; + "gateway" => ?gateway, + ); + Route::ensure_default_route_with_gateway(gateway).map_err(|err| { + match err { + ExecutionError::CommandFailure(ref e) => { + if e.stdout.contains("Network is unreachable") { + BackoffError::transient(err_with_context(err)) + } else { + BackoffError::permanent(err_with_context(err)) + } + } + _ => BackoffError::permanent(err_with_context(err)), + } + }) + }, + |err, delay| { + info!( + log, + "Cannot ensure there is a default route yet (retrying in {:?})", + delay; + "error" => #%err + ); + }, + ) + .await +} + async fn opte_interface_set_up( - matches: &ArgMatches, - log: Logger, -) -> Result<(), CmdError> { - let interface: &String = matches.get_one("opte_interface").unwrap(); - let gateway: Ipv4Addr = *matches.get_one("opte_gateway").unwrap(); - let opte_ip: &IpAddr = matches.get_one("opte_ip").unwrap(); - - info!(&log, "Creating gateway on the OPTE IP interface if it doesn't already exist"; "OPTE interface" => ?interface); - Ipadm::create_opte_gateway(interface) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - - info!(&log, "Ensuring there is a gateway route"; "OPTE gateway" => ?gateway, "OPTE interface" => ?interface, "OPTE IP" => ?opte_ip); - Route::ensure_opte_route(&gateway, interface, &opte_ip) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - - info!(&log, "Ensuring there is a default route"; "gateway" => ?gateway); + args: OpteInterfaceArgs, + log: &Logger, +) -> anyhow::Result<()> { + let OpteInterfaceArgs { interface, gateway, ip } = args; + info!( + log, + "Creating gateway on the OPTE IP interface if it doesn't already exist"; + "OPTE interface" => ?interface + ); + Ipadm::create_opte_gateway(&interface).with_context(|| { + format!("failed to create OPTE gateway on interface {interface}") + })?; + + info!( + log, "Ensuring there is a gateway route"; + "OPTE gateway" => ?gateway, + "OPTE interface" => ?interface, + "OPTE IP" => ?ip, + ); + Route::ensure_opte_route(&gateway, &interface, &ip).with_context(|| { + format!( + "failed to ensure OPTE gateway route on interface {interface} \ + with gateway {gateway} and IP {ip}", + ) + })?; + + info!( + log, "Ensuring there is a default route"; + "gateway" => ?gateway, + ); Route::ensure_default_route_with_gateway(Gateway::Ipv4(gateway)) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + .with_context(|| { + format!("failed to ensure default route via gateway {gateway}") + })?; Ok(()) } diff --git a/zone-setup/src/switch_zone_user.rs b/zone-setup/src/switch_zone_user.rs index d8dda9a599..0c4e478cc0 100644 --- a/zone-setup/src/switch_zone_user.rs +++ b/zone-setup/src/switch_zone_user.rs @@ -2,8 +2,9 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use anyhow::anyhow; -use omicron_common::cmd::CmdError; +use anyhow::Context; +use camino::{Utf8Path, Utf8PathBuf}; +use illumos_utils::{execute, ExecutionError}; use slog::{info, Logger}; use std::fs::{copy, create_dir_all}; use uzers::{get_group_by_name, get_user_by_name}; @@ -14,9 +15,9 @@ pub struct SwitchZoneUser { group: String, gecos: String, nopasswd: bool, - homedir: Option, + homedir: Option, shell: String, - profiles: Option>, + profiles: Vec, } impl SwitchZoneUser { @@ -34,271 +35,156 @@ impl SwitchZoneUser { nopasswd, homedir: None, shell, - profiles: None, + profiles: Vec::new(), } } - pub fn with_homedir(mut self, homedir: String) -> Self { + pub fn with_homedir(mut self, homedir: Utf8PathBuf) -> Self { self.homedir = Some(homedir); self } pub fn with_profiles(mut self, profiles: Vec) -> Self { - self.profiles = Some(profiles); + self.profiles = profiles; self } - fn add_new_group_for_user(&self) -> Result<(), CmdError> { - match get_group_by_name(&self.group) { - Some(_) => {} - None => { - let cmd = std::process::Command::new("groupadd") - .arg(&self.group) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `groupadd {}`: {}", - self.group, - err - )) - })?; - - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not add group: {} status: {}", - self.group, - cmd.status - ))); - } - } - }; - Ok(()) - } - - fn add_new_user(&self) -> Result<(), CmdError> { - match get_user_by_name(&self.user) { - Some(_) => {} - None => { - let cmd = std::process::Command::new("useradd") - .args([ - "-m", - "-s", - &self.shell, - "-g", - &self.group, - "-c", - &self.gecos, - &self.user, - ]) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `useradd -m -s {} -g {} -c {} {}`: {}", - self.shell, self.group, self.gecos, self.user, err - )) - })?; - - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not add user: {} status: {}", - self.user, - cmd.status - ))); - } - } - }; + fn add_new_group_for_user(&self) -> Result<(), ExecutionError> { + if get_group_by_name(&self.group).is_none() { + execute( + &mut std::process::Command::new("groupadd").arg(&self.group), + )?; + } Ok(()) } - fn enable_passwordless_login(&self) -> Result<(), CmdError> { - let cmd = std::process::Command::new("passwd") - .args(["-d", &self.user]) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `passwd -d {}`: {}", - self.user, - err - )) - })?; - - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not enable password-less login: {} status: {}", - self.user, - cmd.status - ))); + fn add_new_user(&self) -> Result<(), ExecutionError> { + if get_user_by_name(&self.user).is_none() { + execute(&mut std::process::Command::new("useradd").args([ + "-m", + "-s", + &self.shell, + "-g", + &self.group, + "-c", + &self.gecos, + &self.user, + ]))?; } Ok(()) } - fn disable_password_based_login(&self) -> Result<(), CmdError> { - let cmd = std::process::Command::new("passwd") - .args(["-N", &self.user]) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `passwd -N {}`: {}", - self.user, - err - )) - })?; - - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not disable password-based logins: {} status: {}", - self.user, - cmd.status - ))); - } + fn enable_passwordless_login(&self) -> Result<(), ExecutionError> { + execute( + &mut std::process::Command::new("passwd").args(["-d", &self.user]), + )?; Ok(()) } - fn assign_user_profiles(&self) -> Result<(), CmdError> { - let Some(ref profiles) = self.profiles else { - return Err(CmdError::Failure(anyhow!( - "Profile list must not be empty to assign user profiles", - ))); - }; - - let mut profile_list: String = Default::default(); - for profile in profiles { - profile_list.push_str(&profile) - } - - let cmd = std::process::Command::new("usermod") - .args(["-P", &profile_list, &self.user]) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `usermod -P {} {}`: {}", - profile_list, - self.user, - err - )) - })?; - - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not assign user profiles: {} status: {}", - self.user, - cmd.status - ))); - } + fn disable_password_based_login(&self) -> Result<(), ExecutionError> { + execute( + &mut std::process::Command::new("passwd").args(["-N", &self.user]), + )?; Ok(()) } - fn remove_user_profiles(&self) -> Result<(), CmdError> { - let cmd = std::process::Command::new("usermod") - .args(["-P", "", &self.user]) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `usermod -P '' {}`: {}", - self.user, - err - )) - })?; + fn assign_user_profiles(&self) -> Result<(), ExecutionError> { + let profiles = self.profiles.join(","); - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not remove user profiles: {} status: {}", - self.user, - cmd.status - ))); - } + execute( + &mut std::process::Command::new("usermod") + .args(["-P", &profiles, &self.user]), + )?; Ok(()) } - fn set_up_home_directory_and_startup_files(&self) -> Result<(), CmdError> { - let Some(ref homedir) = self.homedir else { - return Err(CmdError::Failure(anyhow!( - "A home directory must be provided to set up home directory and startup files", - ))); - }; - - create_dir_all(&homedir).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute create directory {} and its parents: {}", - &homedir, - err - )) + fn set_up_home_directory_and_startup_files( + &self, + homedir: &Utf8Path, + ) -> anyhow::Result<()> { + create_dir_all(&homedir).with_context(|| { + format!( + "Could not execute create directory {} and its parents", + homedir, + ) })?; - let mut home_bashrc = homedir.clone(); - home_bashrc.push_str("/.bashrc"); - copy("/root/.bashrc", &home_bashrc).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not copy file from /root/.bashrc to {}: {}", - &homedir, - err - )) + let home_bashrc = homedir.join(".bashrc"); + copy("/root/.bashrc", &home_bashrc).with_context(|| { + format!("Could not copy file from /root/.bashrc to {homedir}") })?; - let mut home_profile = homedir.clone(); - home_profile.push_str("/.profile"); - copy("/root/.profile", &home_profile).map_err(|err| { - CmdError::Failure(anyhow!( - "Could not copy file from /root/.profile to {}: {}", - &homedir, - err - )) + let home_profile = homedir.join(".profile"); + copy("/root/.profile", &home_profile).with_context(|| { + format!("Could not copy file from /root/.profile to {homedir}") })?; // Not using std::os::unix::fs::chown here because it doesn't support // recursive option. - let cmd = std::process::Command::new("chown") - .args(["-R", &self.user, &homedir]) - .output() - .map_err(|err| { - CmdError::Failure(anyhow!( - "Could not execute `chown -R {} {}`: {}", - self.user, - homedir, - err - )) - })?; + execute(&mut std::process::Command::new("chown").args([ + "-R", + &self.user, + homedir.as_str(), + ]))?; - if !cmd.status.success() { - return Err(CmdError::Failure(anyhow!( - "Could not change ownership: {} status: {}", - homedir, - cmd.status - ))); - } Ok(()) } - pub fn setup_switch_zone_user(self, log: &Logger) -> Result<(), CmdError> { - info!(&log, "Add a new group for the user"; "group" => ?self.group, "user" => ?self.user); - self.add_new_group_for_user()?; - - info!(&log, "Add the user"; "user" => ?self.user, "shell" => ?self.shell, - "group" => &self.group, "gecos" => &self.gecos); - self.add_new_user()?; - - // Either enable password-less login (wicket) or disable password-based logins - // completely (support, which logs in via ssh key). + pub fn setup_switch_zone_user(self, log: &Logger) -> anyhow::Result<()> { + info!( + log, "Add a new group for the user"; + "group" => &self.group, + "user" => &self.user, + ); + self.add_new_group_for_user().context("failed to create group")?; + + info!( + log, "Add the user"; + "user" => &self.user, + "shell" => &self.shell, + "group" => &self.group, + "gecos" => &self.gecos, + ); + self.add_new_user().context("failed to create user")?; + + // Either enable password-less login (wicket) or disable password-based + // logins completely (support, which logs in via ssh key). if self.nopasswd { - info!(&log, "Enable password-less login for user"; "user" => ?self.user); - self.enable_passwordless_login()?; + info!( + log, "Enable password-less login for user"; + "user" => &self.user, + ); + self.enable_passwordless_login() + .context("failed to enable passwordless login")?; } else { - info!(&log, "Disable password-based logins"; "user" => ?self.user); - self.disable_password_based_login()?; + info!( + log, "Disable password-based logins"; + "user" => &self.user, + ); + self.disable_password_based_login() + .context("failed to disable passwordless login")?; }; - if let Some(_) = &self.profiles { - info!(&log, "Assign user profiles"; "user" => ?self.user, "profiles" => ?self.profiles); - self.assign_user_profiles()?; - } else { - info!(&log, "Remove user profiles"; "user" => ?self.user); - self.remove_user_profiles()?; - }; - - if let Some(_) = self.homedir { - info!(&log, "Set up home directory and startup files"; "user" => ?self.user, "home directory" => ?self.homedir); - self.set_up_home_directory_and_startup_files()?; + // If `self.profiles` is empty, this will _remove_ all profiles. This is + // intentional. + info!( + log, "Assign user profiles"; + "user" => &self.user, + "profiles" => ?self.profiles, + ); + self.assign_user_profiles() + .context("failed to assign user profiles")?; + + if let Some(homedir) = &self.homedir { + info!( + log, "Set up home directory and startup files"; + "user" => &self.user, + "home directory" => ?homedir, + ); + self.set_up_home_directory_and_startup_files(homedir) + .context("failed to set up home directory")?; } + Ok(()) } }