From 2c2acdef88e01bf0aa19cf1c89b26c2a0605a3bb Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Mon, 28 Oct 2024 19:08:54 +0000 Subject: [PATCH 01/17] Update config tool --- .../config/src/generate_testnet_config.rs | 250 ++++++++++++++++++ rs/ic_os/config/src/lib.rs | 31 ++- rs/ic_os/config/src/main.rs | 214 ++++++++++++--- rs/ic_os/config/src/types.rs | 24 +- 4 files changed, 460 insertions(+), 59 deletions(-) create mode 100644 rs/ic_os/config/src/generate_testnet_config.rs diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs new file mode 100644 index 00000000000..2d23d8d84fb --- /dev/null +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -0,0 +1,250 @@ +use anyhow::Result; +use mac_address::mac_address::FormattedMacAddress; +use std::net::{Ipv4Addr, Ipv6Addr}; +use std::path::PathBuf; +use url::Url; + +use crate::serialize_and_write_config; +use crate::types::*; + +pub struct GenerateTestnetConfigArgs { + // NetworkSettings arguments + pub ipv6_config_type: Option, // "Deterministic", "Fixed", "RouterAdvertisement" + pub deterministic_prefix: Option, + pub deterministic_prefix_length: Option, + pub deterministic_gateway: Option, + pub fixed_address: Option, + pub fixed_gateway: Option, + pub ipv4_address: Option, + pub ipv4_gateway: Option, + pub ipv4_prefix_length: Option, + pub ipv4_domain: Option, + + // ICOSSettings arguments + pub mgmt_mac: Option, + pub deployment_environment: Option, + pub elasticsearch_hosts: Option, + pub elasticsearch_tags: Option, + pub nns_public_key_exists: Option, + pub nns_urls: Option>, + pub node_operator_private_key_exists: Option, + pub use_ssh_authorized_keys: Option, + + // GuestOSSettings arguments + pub inject_ic_crypto: Option, + pub inject_ic_state: Option, + pub inject_ic_registry_local_store: Option, + + // GuestOSDevSettings arguments + pub backup_retention_time_seconds: Option, + pub backup_purging_interval_seconds: Option, + pub malicious_behavior: Option, + pub query_stats_epoch_length: Option, + pub bitcoind_addr: Option, + pub jaeger_addr: Option, + pub socks_proxy: Option, + pub hostname: Option, +} + +/// Generates a writes a serialized GuestOSConfig to guestos_config_json_path +/// Any required config fields that aren't specified will receive dummy values +pub fn generate_testnet_config( + config: GenerateTestnetConfigArgs, + guestos_config_json_path: PathBuf, +) -> Result<()> { + let GenerateTestnetConfigArgs { + ipv6_config_type, + deterministic_prefix, + deterministic_prefix_length, + deterministic_gateway, + fixed_address, + fixed_gateway, + ipv4_address, + ipv4_gateway, + ipv4_prefix_length, + ipv4_domain, + mgmt_mac, + deployment_environment, + elasticsearch_hosts, + elasticsearch_tags, + nns_public_key_exists, + nns_urls, + node_operator_private_key_exists, + use_ssh_authorized_keys, + inject_ic_crypto, + inject_ic_state, + inject_ic_registry_local_store, + backup_retention_time_seconds, + backup_purging_interval_seconds, + malicious_behavior, + query_stats_epoch_length, + bitcoind_addr, + jaeger_addr, + socks_proxy, + hostname, + } = config; + + // Construct the NetworkSettings + let ipv6_config = match ipv6_config_type.as_deref() { + Some("Deterministic") => { + let prefix = deterministic_prefix.ok_or_else(|| { + anyhow::anyhow!( + "deterministic_prefix is required when ipv6_config_type is 'Deterministic'" + ) + })?; + let prefix_length = deterministic_prefix_length.ok_or_else(|| { + anyhow::anyhow!( + "deterministic_prefix_length is required when ipv6_config_type is 'Deterministic'" + ) + })?; + let gateway_str = deterministic_gateway.ok_or_else(|| { + anyhow::anyhow!( + "deterministic_gateway is required when ipv6_config_type is 'Deterministic'" + ) + })?; + let gateway = gateway_str + .parse::() + .map_err(|e| anyhow::anyhow!("Failed to parse deterministic_gateway: {}", e))?; + + Ipv6Config::Deterministic(DeterministicIpv6Config { + prefix, + prefix_length, + gateway, + }) + } + Some("Fixed") => { + let address = fixed_address.ok_or_else(|| { + anyhow::anyhow!("fixed_address is required when ipv6_config_type is 'Fixed'") + })?; + let gateway_str = fixed_gateway.ok_or_else(|| { + anyhow::anyhow!("fixed_gateway is required when ipv6_config_type is 'Fixed'") + })?; + let gateway = gateway_str + .parse::() + .map_err(|e| anyhow::anyhow!("Failed to parse fixed_gateway: {}", e))?; + + Ipv6Config::Fixed(FixedIpv6Config { address, gateway }) + } + // Default to RouterAdvertisement if not provided + Some("RouterAdvertisement") | None => Ipv6Config::RouterAdvertisement, + Some(other) => { + anyhow::bail!("Invalid ipv6_config_type '{}'. Must be 'Deterministic', 'Fixed', or 'RouterAdvertisement'.", other); + } + }; + + let ipv4_config = match (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) { + (Some(addr_str), Some(gw_str), Some(prefix_len), Some(domain)) => Some(Ipv4Config { + address: addr_str + .parse::() + .map_err(|e| anyhow::anyhow!("Failed to parse ipv4_address: {}", e))?, + gateway: gw_str + .parse::() + .map_err(|e| anyhow::anyhow!("Failed to parse ipv4_gateway: {}", e))?, + prefix_length: prefix_len, + domain, + }), + (None, None, None, None) => None, + _ => { + anyhow::bail!("Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) are required for IPv4 configuration."); + } + }; + + let network_settings = NetworkSettings { + ipv6_config, + ipv4_config, + }; + + // Construct ICOSSettings + let mgmt_mac = match mgmt_mac { + Some(mac_str) => FormattedMacAddress::try_from(mac_str.as_str())?, + None => { + // Use a dummy MAC address + FormattedMacAddress::try_from("00:00:00:00:00:00")? + } + }; + + let deployment_environment = deployment_environment.unwrap_or_else(|| "testnet".to_string()); + + let logging = Logging { + elasticsearch_hosts: elasticsearch_hosts.unwrap_or_else(|| "".to_string()), + elasticsearch_tags, + }; + + let nns_public_key_exists = nns_public_key_exists.unwrap_or(true); + + let nns_urls = match nns_urls { + Some(urls) => urls + .iter() + .map(|s| Url::parse(s)) + .collect::, _>>()?, + None => vec![Url::parse("https://wiki.internetcomputer.org")?], + }; + + let node_operator_private_key_exists = node_operator_private_key_exists.unwrap_or(false); + + let use_ssh_authorized_keys = use_ssh_authorized_keys.unwrap_or(true); + + let icos_settings = ICOSSettings { + mgmt_mac, + deployment_environment, + logging, + nns_public_key_exists, + nns_urls, + node_operator_private_key_exists, + use_ssh_authorized_keys, + icos_dev_settings: ICOSDevSettings::default(), + }; + + // Construct GuestOSDevSettings + let backup_spool = + if backup_retention_time_seconds.is_some() || backup_purging_interval_seconds.is_some() { + Some(BackupSpoolSettings { + backup_retention_time_seconds, + backup_purging_interval_seconds, + }) + } else { + None + }; + + let malicious_behavior = if let Some(mb_str) = malicious_behavior { + Some(serde_json::from_str(&mb_str)?) + } else { + None + }; + + let guestos_dev_settings = GuestOSDevSettings { + backup_spool, + malicious_behavior, + query_stats_epoch_length, + bitcoind_addr, + jaeger_addr, + socks_proxy, + hostname, + }; + + // Construct GuestOSSettings + let guestos_settings = GuestOSSettings { + inject_ic_crypto: inject_ic_crypto.unwrap_or(false), + inject_ic_state: inject_ic_state.unwrap_or(false), + inject_ic_registry_local_store: inject_ic_registry_local_store.unwrap_or(false), + guestos_dev_settings, + }; + + // Assemble GuestOSConfig + let guestos_config = GuestOSConfig { + network_settings, + icos_settings, + guestos_settings, + }; + + println!("GuestOSConfig: {:?}", guestos_config); + + serialize_and_write_config(&guestos_config_json_path, &guestos_config)?; + + println!( + "GuestOSConfig has been written to {}", + guestos_config_json_path.display() + ); + + Ok(()) +} diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index edf66b7c3a6..e4a519427b1 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -1,5 +1,6 @@ pub mod config_ini; pub mod deployment_json; +pub mod generate_testnet_config; pub mod types; use anyhow::{Context, Result}; @@ -11,15 +12,13 @@ use std::path::Path; pub static DEFAULT_SETUPOS_CONFIG_OBJECT_PATH: &str = "/var/ic/config/config.json"; pub static DEFAULT_SETUPOS_CONFIG_INI_FILE_PATH: &str = "/config/config.ini"; pub static DEFAULT_SETUPOS_DEPLOYMENT_JSON_PATH: &str = "/data/deployment.json"; -pub static DEFAULT_SETUPOS_NNS_PUBLIC_KEY_PATH: &str = "/data/nns_public_key.pem"; pub static DEFAULT_SETUPOS_SSH_AUTHORIZED_KEYS_PATH: &str = "/config/ssh_authorized_keys"; -pub static DEFAULT_SETUPOS_NODE_OPERATOR_PRIVATE_KEY_PATH: &str = - "/config/node_operator_private_key.pem"; pub static DEFAULT_SETUPOS_HOSTOS_CONFIG_OBJECT_PATH: &str = "/var/ic/config/config-hostos.json"; -pub static DEFAULT_HOSTOS_CONFIG_INI_FILE_PATH: &str = "/boot/config/config.ini"; -pub static DEFAULT_HOSTOS_DEPLOYMENT_JSON_PATH: &str = "/boot/config/deployment.json"; +pub static DEFAULT_HOSTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config.json"; +pub static DEFAULT_HOSTOS_GUESTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config-guestos.json"; +pub static DEFAULT_GUESTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config.json"; pub fn serialize_and_write_config(path: &Path, config: &T) -> Result<()> { let serialized_config = @@ -34,11 +33,12 @@ pub fn serialize_and_write_config(path: &Path, config: &T) -> Resu Ok(()) } -pub fn deserialize_config Deserialize<'de>>(file_path: &str) -> Result { - let file = File::open(file_path).context(format!("Failed to open file: {}", file_path))?; +pub fn deserialize_config Deserialize<'de>, P: AsRef>(file_path: P) -> Result { + let file = + File::open(&file_path).context(format!("Failed to open file: {:?}", file_path.as_ref()))?; serde_json::from_reader(file).context(format!( - "Failed to deserialize JSON from file: {}", - file_path + "Failed to deserialize JSON from file: {:?}", + file_path.as_ref() )) } @@ -46,7 +46,6 @@ pub fn deserialize_config Deserialize<'de>>(file_path: &str) -> Resu mod tests { use super::*; use mac_address::mac_address::FormattedMacAddress; - use std::path::PathBuf; use types::*; #[test] @@ -75,10 +74,10 @@ mod tests { mgmt_mac: FormattedMacAddress::try_from("ec:2a:72:31:a2:0c")?, deployment_environment: "Mainnet".to_string(), logging, - nns_public_key_path: PathBuf::from("/path/to/key"), + nns_public_key_exists: true, nns_urls: vec!["http://localhost".parse().unwrap()], - node_operator_private_key_path: None, - ssh_authorized_keys_path: None, + node_operator_private_key_exists: true, + use_ssh_authorized_keys: false, icos_dev_settings, }; let setupos_settings = SetupOSSettings; @@ -88,9 +87,9 @@ mod tests { verbose: false, }; let guestos_settings = GuestOSSettings { - ic_crypto_path: None, - ic_state_path: None, - ic_registry_local_store_path: None, + inject_ic_crypto: false, + inject_ic_state: false, + inject_ic_registry_local_store: false, guestos_dev_settings: GuestOSDevSettings::default(), }; diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index 825885dda63..ad772a39291 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -1,5 +1,5 @@ use anyhow::Result; -use clap::{Parser, Subcommand}; +use clap::{Args, Parser, Subcommand}; use config::config_ini::{get_config_ini_settings, ConfigIniSettings}; use config::deployment_json::get_deployment_settings; use config::serialize_and_write_config; @@ -7,9 +7,11 @@ use mac_address::mac_address::{get_ipmi_mac, FormattedMacAddress}; use std::fs::File; use std::path::{Path, PathBuf}; +use config::generate_testnet_config::{generate_testnet_config, GenerateTestnetConfigArgs}; use config::types::*; #[derive(Subcommand)] +#[allow(clippy::large_enum_variant)] pub enum Commands { /// Creates SetupOSConfig object CreateSetuposConfig { @@ -19,14 +21,14 @@ pub enum Commands { #[arg(long, default_value = config::DEFAULT_SETUPOS_DEPLOYMENT_JSON_PATH, value_name = "deployment.json")] deployment_json_path: PathBuf, - #[arg(long, default_value = config::DEFAULT_SETUPOS_NNS_PUBLIC_KEY_PATH, value_name = "nns_public_key.pem")] - nns_public_key_path: PathBuf, + #[arg(long, default_value_t = true)] + nns_public_key_exists: bool, - #[arg(long, default_value = config::DEFAULT_SETUPOS_SSH_AUTHORIZED_KEYS_PATH, value_name = "ssh_authorized_keys")] - ssh_authorized_keys_path: PathBuf, + #[arg(long, default_value_t = false)] + use_ssh_authorized_keys: bool, - #[arg(long, default_value = config::DEFAULT_SETUPOS_NODE_OPERATOR_PRIVATE_KEY_PATH, value_name = "node_operator_private_key.pem")] - node_operator_private_key_path: PathBuf, + #[arg(long, default_value_t = true)] + node_operator_private_key_exists: bool, #[arg(long, default_value = config::DEFAULT_SETUPOS_CONFIG_OBJECT_PATH, value_name = "config.json")] setupos_config_json_path: PathBuf, @@ -38,6 +40,17 @@ pub enum Commands { #[arg(long, default_value = config::DEFAULT_SETUPOS_HOSTOS_CONFIG_OBJECT_PATH, value_name = "config-hostos.json")] hostos_config_json_path: PathBuf, }, + /// Creates GuestOSConfig object from existing HostOS config.json file + GenerateGuestosConfig { + #[arg(long, default_value = config::DEFAULT_HOSTOS_CONFIG_OBJECT_PATH, value_name = "config.json")] + hostos_config_json_path: PathBuf, + #[arg(long, default_value = config::DEFAULT_HOSTOS_GUESTOS_CONFIG_OBJECT_PATH, value_name = "config-guestos.json")] + guestos_config_json_path: PathBuf, + #[arg(long, value_name = "ipv6_address")] + guestos_ipv6_address: String, + }, + /// Creates a GuestOSConfig object directly from GenerateTestnetConfigClapArgs + GenerateTestnetConfig(GenerateTestnetConfigClapArgs), } #[derive(Parser)] @@ -47,6 +60,78 @@ struct ConfigArgs { command: Option, } +#[derive(Args)] +pub struct GenerateTestnetConfigClapArgs { + #[arg(long)] + pub ipv6_config_type: Option, // "Deterministic", "Fixed", "RouterAdvertisement" + #[arg(long)] + pub deterministic_prefix: Option, + #[arg(long)] + pub deterministic_prefix_length: Option, + #[arg(long)] + pub deterministic_gateway: Option, + #[arg(long)] + pub fixed_address: Option, + #[arg(long)] + pub fixed_gateway: Option, + #[arg(long)] + pub ipv4_address: Option, + #[arg(long)] + pub ipv4_gateway: Option, + #[arg(long)] + pub ipv4_prefix_length: Option, + #[arg(long)] + pub ipv4_domain: Option, + + // ICOSSettings arguments + #[arg(long)] + pub mgmt_mac: Option, + #[arg(long)] + pub deployment_environment: Option, + #[arg(long)] + pub elasticsearch_hosts: Option, + #[arg(long)] + pub elasticsearch_tags: Option, + #[arg(long)] + pub nns_public_key_exists: Option, + #[arg(long)] + pub nns_urls: Option>, + #[arg(long)] + pub node_operator_private_key_exists: Option, + #[arg(long)] + pub use_ssh_authorized_keys: Option, + + // GuestOSSettings arguments + #[arg(long)] + pub inject_ic_crypto: Option, + #[arg(long)] + pub inject_ic_state: Option, + #[arg(long)] + pub inject_ic_registry_local_store: Option, + + // GuestOSDevSettings arguments + #[arg(long)] + pub backup_retention_time_seconds: Option, + #[arg(long)] + pub backup_purging_interval_seconds: Option, + #[arg(long)] + pub malicious_behavior: Option, + #[arg(long)] + pub query_stats_epoch_length: Option, + #[arg(long)] + pub bitcoind_addr: Option, + #[arg(long)] + pub jaeger_addr: Option, + #[arg(long)] + pub socks_proxy: Option, + #[arg(long)] + pub hostname: Option, + + // Output path + #[arg(long)] + pub guestos_config_json_path: PathBuf, +} + pub fn main() -> Result<()> { let opts = ConfigArgs::parse(); @@ -54,9 +139,9 @@ pub fn main() -> Result<()> { Some(Commands::CreateSetuposConfig { config_ini_path, deployment_json_path, - nns_public_key_path, - ssh_authorized_keys_path, - node_operator_private_key_path, + nns_public_key_exists, + use_ssh_authorized_keys, + node_operator_private_key_exists, setupos_config_json_path, }) => { // get config.ini settings @@ -123,14 +208,10 @@ pub fn main() -> Result<()> { mgmt_mac, deployment_environment: deployment_json_settings.deployment.name, logging, - nns_public_key_path: nns_public_key_path.to_path_buf(), + nns_public_key_exists, nns_urls: deployment_json_settings.nns.url.clone(), - node_operator_private_key_path: node_operator_private_key_path - .exists() - .then_some(node_operator_private_key_path), - ssh_authorized_keys_path: ssh_authorized_keys_path - .exists() - .then_some(ssh_authorized_keys_path), + node_operator_private_key_exists, + use_ssh_authorized_keys, icos_dev_settings: ICOSDevSettings::default(), }; @@ -176,21 +257,9 @@ pub fn main() -> Result<()> { let setupos_config: SetupOSConfig = serde_json::from_reader(File::open(setupos_config_json_path)?)?; - // update select file paths for HostOS - let mut hostos_icos_settings = setupos_config.icos_settings; - let hostos_config_path = Path::new("/boot/config"); - if let Some(ref mut path) = hostos_icos_settings.ssh_authorized_keys_path { - *path = hostos_config_path.join("ssh_authorized_keys"); - } - if let Some(ref mut path) = hostos_icos_settings.node_operator_private_key_path { - *path = hostos_config_path.join("node_operator_private_key.pem"); - } - hostos_icos_settings.nns_public_key_path = - hostos_config_path.join("nns_public_key.pem"); - let hostos_config = HostOSConfig { network_settings: setupos_config.network_settings, - icos_settings: hostos_icos_settings, + icos_settings: setupos_config.icos_settings, hostos_settings: setupos_config.hostos_settings, guestos_settings: setupos_config.guestos_settings, }; @@ -205,6 +274,89 @@ pub fn main() -> Result<()> { Ok(()) } - None => Ok(()), + Some(Commands::GenerateGuestosConfig { + hostos_config_json_path, + guestos_config_json_path, + guestos_ipv6_address, + }) => { + let hostos_config_json_path = Path::new(&hostos_config_json_path); + + let hostos_config: HostOSConfig = + serde_json::from_reader(File::open(hostos_config_json_path)?)?; + + // TODO: We won't have to modify networking between the hostos and + // guestos config after completing the networking revamp (NODE-1327) + let mut guestos_network_settings = hostos_config.network_settings; + // Update the GuestOS networking if `guestos_ipv6_address` is provided + match &guestos_network_settings.ipv6_config { + Ipv6Config::Deterministic(deterministic_ipv6_config) => { + guestos_network_settings.ipv6_config = Ipv6Config::Fixed(FixedIpv6Config { + address: guestos_ipv6_address, + gateway: deterministic_ipv6_config.gateway, + }); + } + _ => { + anyhow::bail!( + "HostOSConfig Ipv6Config should always be of type Deterministic. Cannot reassign GuestOS networking." + ); + } + } + + let guestos_config = GuestOSConfig { + network_settings: guestos_network_settings, + icos_settings: hostos_config.icos_settings, + guestos_settings: hostos_config.guestos_settings, + }; + + let guestos_config_json_path = Path::new(&guestos_config_json_path); + serialize_and_write_config(guestos_config_json_path, &guestos_config)?; + + println!( + "GuestOSConfig has been written to {}", + guestos_config_json_path.display() + ); + + Ok(()) + } + Some(Commands::GenerateTestnetConfig(clap_args)) => { + // Convert `clap_args` into `GenerateTestnetConfigArgs` + let args = GenerateTestnetConfigArgs { + ipv6_config_type: clap_args.ipv6_config_type, + deterministic_prefix: clap_args.deterministic_prefix, + deterministic_prefix_length: clap_args.deterministic_prefix_length, + deterministic_gateway: clap_args.deterministic_gateway, + fixed_address: clap_args.fixed_address, + fixed_gateway: clap_args.fixed_gateway, + ipv4_address: clap_args.ipv4_address, + ipv4_gateway: clap_args.ipv4_gateway, + ipv4_prefix_length: clap_args.ipv4_prefix_length, + ipv4_domain: clap_args.ipv4_domain, + mgmt_mac: clap_args.mgmt_mac, + deployment_environment: clap_args.deployment_environment, + elasticsearch_hosts: clap_args.elasticsearch_hosts, + elasticsearch_tags: clap_args.elasticsearch_tags, + nns_public_key_exists: clap_args.nns_public_key_exists, + nns_urls: clap_args.nns_urls, + node_operator_private_key_exists: clap_args.node_operator_private_key_exists, + use_ssh_authorized_keys: clap_args.use_ssh_authorized_keys, + inject_ic_crypto: clap_args.inject_ic_crypto, + inject_ic_state: clap_args.inject_ic_state, + inject_ic_registry_local_store: clap_args.inject_ic_registry_local_store, + backup_retention_time_seconds: clap_args.backup_retention_time_seconds, + backup_purging_interval_seconds: clap_args.backup_purging_interval_seconds, + malicious_behavior: clap_args.malicious_behavior, + query_stats_epoch_length: clap_args.query_stats_epoch_length, + bitcoind_addr: clap_args.bitcoind_addr, + jaeger_addr: clap_args.jaeger_addr, + socks_proxy: clap_args.socks_proxy, + hostname: clap_args.hostname, + }; + + generate_testnet_config(args, clap_args.guestos_config_json_path) + } + None => { + println!("No command provided. Use --help for usage information."); + Ok(()) + } } } diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index 5ad9c788686..1b1832d66a4 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -2,7 +2,6 @@ use ic_types::malicious_behaviour::MaliciousBehaviour; use mac_address::mac_address::FormattedMacAddress; use serde::{Deserialize, Serialize}; use std::net::{Ipv4Addr, Ipv6Addr}; -use std::path::PathBuf; use url::Url; /// SetupOS configuration. User-facing configuration files @@ -52,13 +51,13 @@ pub struct GuestOSSettings { /// Must be a directory with contents matching the internal representation of the ic_crypto directory. /// When given, this provides the private keys of the node. /// If not given, the node will generate its own private/public key pair. - pub ic_crypto_path: Option, - pub ic_state_path: Option, + pub inject_ic_crypto: bool, + pub inject_ic_state: bool, /// Initial registry state. /// Must be a directory with contents matching the internal representation of the ic_registry_local_store. /// When given, this provides the initial state of the registry. /// If not given, the node will fetch (initial) registry state from the NNS. - pub ic_registry_local_store_path: Option, + pub inject_ic_registry_local_store: bool, pub guestos_dev_settings: GuestOSDevSettings, } @@ -71,6 +70,8 @@ pub struct GuestOSDevSettings { pub bitcoind_addr: Option, pub jaeger_addr: Option, pub socks_proxy: Option, + // An optional hostname to override the deterministically generated hostname + pub hostname: Option, } /// Configures the usage of the backup spool directory. @@ -90,20 +91,18 @@ pub struct ICOSSettings { /// "mainnet" or "testnet" pub deployment_environment: String, pub logging: Logging, - /// This file must be a text file containing the public key of the NNS to be used. - pub nns_public_key_path: PathBuf, + pub nns_public_key_exists: bool, /// The URL (HTTP) of the NNS node(s). pub nns_urls: Vec, - /// This file contains the Node Operator private key, - /// which is registered with the NNS and used to sign the IC join request. - pub node_operator_private_key_path: Option, - /// This directory contains individual files named `admin`, `backup`, `readonly`. + pub node_operator_private_key_exists: bool, + /// This ssh keys directory contains individual files named `admin`, `backup`, `readonly`. /// The contents of these files serve as `authorized_keys` for their respective role account. /// This means that, for example, `accounts_ssh_authorized_keys/admin` /// is transferred to `~admin/.ssh/authorized_keys` on the target system. /// backup and readonly can only be modified via an NNS proposal /// and are in place for subnet recovery or issue debugging purposes. - pub ssh_authorized_keys_path: Option, + /// use_ssh_authorized_keys triggers the use of the ssh keys directory + pub use_ssh_authorized_keys: bool, pub icos_dev_settings: ICOSDevSettings, } @@ -148,6 +147,7 @@ pub struct DeterministicIpv6Config { #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct FixedIpv6Config { - pub address: Ipv6Addr, + // fixed ipv6 address includes subnet mask /64 + pub address: String, pub gateway: Ipv6Addr, } From 54f2c6721a64618f07b84335a846f9160e284470 Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Mon, 28 Oct 2024 19:23:22 +0000 Subject: [PATCH 02/17] Fix config constants --- rs/ic_os/config/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index e4a519427b1..88f62bc3c81 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -12,13 +12,11 @@ use std::path::Path; pub static DEFAULT_SETUPOS_CONFIG_OBJECT_PATH: &str = "/var/ic/config/config.json"; pub static DEFAULT_SETUPOS_CONFIG_INI_FILE_PATH: &str = "/config/config.ini"; pub static DEFAULT_SETUPOS_DEPLOYMENT_JSON_PATH: &str = "/data/deployment.json"; -pub static DEFAULT_SETUPOS_SSH_AUTHORIZED_KEYS_PATH: &str = "/config/ssh_authorized_keys"; pub static DEFAULT_SETUPOS_HOSTOS_CONFIG_OBJECT_PATH: &str = "/var/ic/config/config-hostos.json"; -pub static DEFAULT_HOSTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config.json"; -pub static DEFAULT_HOSTOS_GUESTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config-guestos.json"; -pub static DEFAULT_GUESTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config.json"; +pub static DEFAULT_HOSTOS_CONFIG_INI_FILE_PATH: &str = "/boot/config/config.ini"; +pub static DEFAULT_HOSTOS_DEPLOYMENT_JSON_PATH: &str = "/boot/config/deployment.json"; pub fn serialize_and_write_config(path: &Path, config: &T) -> Result<()> { let serialized_config = From 82e35cb6026cddf1f9039a5f28c2d31502c0493c Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Mon, 28 Oct 2024 19:41:27 +0000 Subject: [PATCH 03/17] Fix config tool constants --- rs/ic_os/config/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index 88f62bc3c81..3db6e8d8ed5 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -17,6 +17,9 @@ pub static DEFAULT_SETUPOS_HOSTOS_CONFIG_OBJECT_PATH: &str = "/var/ic/config/con pub static DEFAULT_HOSTOS_CONFIG_INI_FILE_PATH: &str = "/boot/config/config.ini"; pub static DEFAULT_HOSTOS_DEPLOYMENT_JSON_PATH: &str = "/boot/config/deployment.json"; +pub static DEFAULT_HOSTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config.json"; +pub static DEFAULT_HOSTOS_GUESTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config-guestos.json"; +pub static DEFAULT_GUESTOS_CONFIG_OBJECT_PATH: &str = "/boot/config/config.json"; pub fn serialize_and_write_config(path: &Path, config: &T) -> Result<()> { let serialized_config = From 88e6988807243e44aac85afd7351f5765c1b994b Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Tue, 29 Oct 2024 17:50:51 +0000 Subject: [PATCH 04/17] Convert ipv6_config_type to Enum --- .../config/src/generate_testnet_config.rs | 20 +++++++++++-------- rs/ic_os/config/src/main.rs | 6 ++++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index 2d23d8d84fb..565ee8a89f8 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -9,7 +9,7 @@ use crate::types::*; pub struct GenerateTestnetConfigArgs { // NetworkSettings arguments - pub ipv6_config_type: Option, // "Deterministic", "Fixed", "RouterAdvertisement" + pub ipv6_config_type: Option, pub deterministic_prefix: Option, pub deterministic_prefix_length: Option, pub deterministic_gateway: Option, @@ -46,6 +46,13 @@ pub struct GenerateTestnetConfigArgs { pub hostname: Option, } +#[derive(Clone, clap::ValueEnum)] +pub enum Ipv6ConfigType { + Deterministic, + Fixed, + RouterAdvertisement, +} + /// Generates a writes a serialized GuestOSConfig to guestos_config_json_path /// Any required config fields that aren't specified will receive dummy values pub fn generate_testnet_config( @@ -85,8 +92,8 @@ pub fn generate_testnet_config( } = config; // Construct the NetworkSettings - let ipv6_config = match ipv6_config_type.as_deref() { - Some("Deterministic") => { + let ipv6_config = match ipv6_config_type { + Some(Ipv6ConfigType::Deterministic) => { let prefix = deterministic_prefix.ok_or_else(|| { anyhow::anyhow!( "deterministic_prefix is required when ipv6_config_type is 'Deterministic'" @@ -112,7 +119,7 @@ pub fn generate_testnet_config( gateway, }) } - Some("Fixed") => { + Some(Ipv6ConfigType::Fixed) => { let address = fixed_address.ok_or_else(|| { anyhow::anyhow!("fixed_address is required when ipv6_config_type is 'Fixed'") })?; @@ -126,10 +133,7 @@ pub fn generate_testnet_config( Ipv6Config::Fixed(FixedIpv6Config { address, gateway }) } // Default to RouterAdvertisement if not provided - Some("RouterAdvertisement") | None => Ipv6Config::RouterAdvertisement, - Some(other) => { - anyhow::bail!("Invalid ipv6_config_type '{}'. Must be 'Deterministic', 'Fixed', or 'RouterAdvertisement'.", other); - } + Some(Ipv6ConfigType::RouterAdvertisement) | None => Ipv6Config::RouterAdvertisement, }; let ipv4_config = match (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) { diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index ad772a39291..45938b272e2 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -7,7 +7,9 @@ use mac_address::mac_address::{get_ipmi_mac, FormattedMacAddress}; use std::fs::File; use std::path::{Path, PathBuf}; -use config::generate_testnet_config::{generate_testnet_config, GenerateTestnetConfigArgs}; +use config::generate_testnet_config::{ + generate_testnet_config, GenerateTestnetConfigArgs, Ipv6ConfigType, +}; use config::types::*; #[derive(Subcommand)] @@ -63,7 +65,7 @@ struct ConfigArgs { #[derive(Args)] pub struct GenerateTestnetConfigClapArgs { #[arg(long)] - pub ipv6_config_type: Option, // "Deterministic", "Fixed", "RouterAdvertisement" + pub ipv6_config_type: Option, #[arg(long)] pub deterministic_prefix: Option, #[arg(long)] From f8faacdb435a9a5e2a742fcf1da0257557f5abbd Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Tue, 29 Oct 2024 18:08:48 +0000 Subject: [PATCH 05/17] Add generate_testnet_config validation tests --- .../config/src/generate_testnet_config.rs | 197 ++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index 565ee8a89f8..d77db64dd90 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -7,6 +7,7 @@ use url::Url; use crate::serialize_and_write_config; use crate::types::*; +#[derive(Default)] pub struct GenerateTestnetConfigArgs { // NetworkSettings arguments pub ipv6_config_type: Option, @@ -252,3 +253,199 @@ pub fn generate_testnet_config( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + #[test] + fn test_valid_configuration() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::RouterAdvertisement), + mgmt_mac: Some("00:11:22:33:44:55".to_string()), + nns_urls: Some(vec!["https://example.com".to_string()]), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/tmp/guestos_config.json")); + assert!(result.is_ok()); + } + + #[test] + fn test_missing_deterministic_prefix() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Deterministic), + deterministic_prefix: None, + deterministic_prefix_length: Some(64), + deterministic_gateway: Some("fe80::1".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "deterministic_prefix is required when ipv6_config_type is 'Deterministic'" + ); + } + + #[test] + fn test_missing_deterministic_prefix_length() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Deterministic), + deterministic_prefix: Some("2001:db8::".to_string()), + deterministic_prefix_length: None, + deterministic_gateway: Some("fe80::1".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "deterministic_prefix_length is required when ipv6_config_type is 'Deterministic'" + ); + } + + #[test] + fn test_missing_deterministic_gateway() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Deterministic), + deterministic_prefix: Some("2001:db8::".to_string()), + deterministic_prefix_length: Some(64), + deterministic_gateway: None, + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "deterministic_gateway is required when ipv6_config_type is 'Deterministic'" + ); + } + + #[test] + fn test_invalid_deterministic_gateway() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Deterministic), + deterministic_prefix: Some("2001:db8::".to_string()), + deterministic_prefix_length: Some(64), + deterministic_gateway: Some("invalid_ip".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to parse deterministic_gateway")); + } + + #[test] + fn test_missing_fixed_address() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Fixed), + fixed_address: None, + fixed_gateway: Some("fe80::1".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "fixed_address is required when ipv6_config_type is 'Fixed'" + ); + } + + #[test] + fn test_missing_fixed_gateway() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Fixed), + fixed_address: Some("2001:db8::1/64".to_string()), + fixed_gateway: None, + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "fixed_gateway is required when ipv6_config_type is 'Fixed'" + ); + } + + #[test] + fn test_invalid_fixed_gateway() { + let args = GenerateTestnetConfigArgs { + ipv6_config_type: Some(Ipv6ConfigType::Fixed), + fixed_address: Some("2001:db8::1/64".to_string()), + fixed_gateway: Some("invalid_ip".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to parse fixed_gateway")); + } + + #[test] + fn test_incomplete_ipv4_config() { + let args = GenerateTestnetConfigArgs { + ipv4_address: Some("192.0.2.1".to_string()), + ipv4_gateway: Some("192.0.2.254".to_string()), + ipv4_prefix_length: None, + ipv4_domain: Some("example.com".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) are required for IPv4 configuration." + ); + } + + #[test] + fn test_invalid_ipv4_address() { + let args = GenerateTestnetConfigArgs { + ipv4_address: Some("invalid_ip".to_string()), + ipv4_gateway: Some("192.0.2.254".to_string()), + ipv4_prefix_length: Some(24), + ipv4_domain: Some("example.com".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to parse ipv4_address")); + } + + #[test] + fn test_invalid_ipv4_gateway() { + let args = GenerateTestnetConfigArgs { + ipv4_address: Some("192.0.2.1".to_string()), + ipv4_gateway: Some("invalid_ip".to_string()), + ipv4_prefix_length: Some(24), + ipv4_domain: Some("example.com".to_string()), + ..Default::default() + }; + + let result = generate_testnet_config(args, PathBuf::from("/dev/null")); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to parse ipv4_gateway")); + } +} From f0fa8a98d429f8252578e0e399728a2a9fcff84f Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Tue, 29 Oct 2024 18:14:36 +0000 Subject: [PATCH 06/17] Expand comment for GenerateTestnetConfig --- rs/ic_os/config/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index 45938b272e2..13c488ce689 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -51,7 +51,7 @@ pub enum Commands { #[arg(long, value_name = "ipv6_address")] guestos_ipv6_address: String, }, - /// Creates a GuestOSConfig object directly from GenerateTestnetConfigClapArgs + /// Creates a GuestOSConfig object directly from GenerateTestnetConfigClapArgs. Only used for testing purposes. GenerateTestnetConfig(GenerateTestnetConfigClapArgs), } From f2b2341eec0f85d65cd4811725c260a1a484cac2 Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Wed, 30 Oct 2024 17:04:53 +0000 Subject: [PATCH 07/17] Add CONFIG_VERSION to track versions backwards compatibility --- .../config/src/generate_testnet_config.rs | 1 + rs/ic_os/config/src/lib.rs | 126 ++++++++++++++++++ rs/ic_os/config/src/main.rs | 1 + rs/ic_os/config/src/types.rs | 58 ++++---- 4 files changed, 159 insertions(+), 27 deletions(-) diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index d77db64dd90..d5d70887644 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -190,6 +190,7 @@ pub fn generate_testnet_config( let use_ssh_authorized_keys = use_ssh_authorized_keys.unwrap_or(true); let icos_settings = ICOSSettings { + config_version: CONFIG_VERSION.to_string(), mgmt_mac, deployment_environment, logging, diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index 3db6e8d8ed5..b01b6fa9e5a 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -72,6 +72,7 @@ mod tests { }; let icos_dev_settings = ICOSDevSettings::default(); let icos_settings = ICOSSettings { + config_version: CONFIG_VERSION.to_string(), mgmt_mac: FormattedMacAddress::try_from("ec:2a:72:31:a2:0c")?, deployment_environment: "Mainnet".to_string(), logging, @@ -136,4 +137,129 @@ mod tests { Ok(()) } + + // Test config version 1.0.0 + const HOSTOS_CONFIG_JSON_V1_0_0: &str = r#" + { + "network_settings": { + "ipv6_config": { + "Deterministic": { + "prefix": "2a00:fb01:400:200", + "prefix_length": 64, + "gateway": "2a00:fb01:400:200::1" + } + }, + "ipv4_config": { + "address": "192.168.0.2", + "gateway": "192.168.0.1", + "prefix_length": 24, + "domain": "example.com" + } + }, + "icos_settings": { + "config_version": "1.0.0", + "mgmt_mac": "ec:2a:72:31:a2:0c", + "deployment_environment": "Mainnet", + "logging": { + "elasticsearch_hosts": "elasticsearch-node-0.mercury.dfinity.systems:443 elasticsearch-node-1.mercury.dfinity.systems:443", + "elasticsearch_tags": "tag1 tag2" + }, + "nns_public_key_exists": true, + "nns_urls": [ + "http://localhost" + ], + "node_operator_private_key_exists": true, + "use_ssh_authorized_keys": false, + "icos_dev_settings": {} + }, + "hostos_settings": { + "vm_memory": 490, + "vm_cpu": "kvm", + "verbose": false + }, + "guestos_settings": { + "inject_ic_crypto": false, + "inject_ic_state": false, + "inject_ic_registry_local_store": false, + "guestos_dev_settings": { + "backup_spool": { + "backup_retention_time_seconds": 3600, + "backup_purging_interval_seconds": 600 + }, + "malicious_behavior": null, + "query_stats_epoch_length": 1000, + "bitcoind_addr": "127.0.0.1:8333", + "jaeger_addr": "127.0.0.1:6831", + "socks_proxy": "127.0.0.1:1080", + "hostname": "my-node" + } + } + } + "#; + + const GUESTOS_CONFIG_JSON_V1_0_0: &str = r#" + { + "network_settings": { + "ipv6_config": { + "Fixed": { + "address": "2a00:fb01:400:200::2/64", + "gateway": "2a00:fb01:400:200::1" + } + }, + "ipv4_config": null + }, + "icos_settings": { + "config_version": "1.0.0", + "mgmt_mac": "ec:2a:72:31:a2:0c", + "deployment_environment": "Mainnet", + "logging": { + "elasticsearch_hosts": "elasticsearch-node-0.mercury.dfinity.systems:443", + "elasticsearch_tags": "tag1 tag2" + }, + "nns_public_key_exists": true, + "nns_urls": [ + "http://localhost" + ], + "node_operator_private_key_exists": true, + "use_ssh_authorized_keys": false, + "icos_dev_settings": {} + }, + "guestos_settings": { + "inject_ic_crypto": true, + "inject_ic_state": true, + "inject_ic_registry_local_store": true, + "guestos_dev_settings": { + "backup_spool": { + "backup_retention_time_seconds": 7200, + "backup_purging_interval_seconds": 1200 + }, + "malicious_behavior": null, + "query_stats_epoch_length": 2000, + "bitcoind_addr": "127.0.0.1:8333", + "jaeger_addr": "127.0.0.1:6831", + "socks_proxy": "127.0.0.1:1080", + "hostname": "guest-node" + } + } + } + "#; + + #[test] + fn test_deserialize_hostos_config_v1_0_0() -> Result<(), Box> { + let config: HostOSConfig = serde_json::from_str(HOSTOS_CONFIG_JSON_V1_0_0)?; + assert_eq!(config.icos_settings.config_version, "1.0.0"); + assert_eq!(config.hostos_settings.vm_cpu, "kvm"); + Ok(()) + } + + #[test] + fn test_deserialize_guestos_config_v1_0_0() -> Result<(), Box> { + let config: GuestOSConfig = serde_json::from_str(GUESTOS_CONFIG_JSON_V1_0_0)?; + assert_eq!(config.icos_settings.config_version, "1.0.0"); + assert_eq!( + config.icos_settings.mgmt_mac.to_string(), + "ec:2a:72:31:a2:0c" + ); + Ok(()) + } } diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index 13c488ce689..668d1634b4b 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -207,6 +207,7 @@ pub fn main() -> Result<()> { }; let icos_settings = ICOSSettings { + config_version: CONFIG_VERSION.to_string(), mgmt_mac, deployment_environment: deployment_json_settings.deployment.name, logging, diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index 1b1832d66a4..b2001546655 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize}; use std::net::{Ipv4Addr, Ipv6Addr}; use url::Url; +pub const CONFIG_VERSION: &str = "1.0.0"; + /// SetupOS configuration. User-facing configuration files /// (e.g., `config.ini`, `deployment.json`) are transformed into `SetupOSConfig`. #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] @@ -32,6 +34,34 @@ pub struct GuestOSConfig { pub guestos_settings: GuestOSSettings, } +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] +pub struct ICOSSettings { + /// Tracks the config version, set to CONFIG_VERSION at runtime. + pub config_version: String, + /// In nested testing, mgmt_mac is set in deployment.json.template, + /// else found dynamically in call to config tool CreateSetuposConfig + pub mgmt_mac: FormattedMacAddress, + /// "mainnet" or "testnet" + pub deployment_environment: String, + pub logging: Logging, + pub nns_public_key_exists: bool, + /// The URL (HTTP) of the NNS node(s). + pub nns_urls: Vec, + pub node_operator_private_key_exists: bool, + /// This ssh keys directory contains individual files named `admin`, `backup`, `readonly`. + /// The contents of these files serve as `authorized_keys` for their respective role account. + /// This means that, for example, `accounts_ssh_authorized_keys/admin` + /// is transferred to `~admin/.ssh/authorized_keys` on the target system. + /// backup and readonly can only be modified via an NNS proposal + /// and are in place for subnet recovery or issue debugging purposes. + /// use_ssh_authorized_keys triggers the use of the ssh keys directory + pub use_ssh_authorized_keys: bool, + pub icos_dev_settings: ICOSDevSettings, +} + +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)] +pub struct ICOSDevSettings {} + /// Placeholder for SetupOS-specific settings. #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct SetupOSSettings; @@ -83,32 +113,6 @@ pub struct BackupSpoolSettings { pub backup_purging_interval_seconds: Option, } -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] -pub struct ICOSSettings { - /// in nested testing, mgmt_mac is set in deployment.json.template, - /// else found dynamically in call to config tool CreateSetuposConfig - pub mgmt_mac: FormattedMacAddress, - /// "mainnet" or "testnet" - pub deployment_environment: String, - pub logging: Logging, - pub nns_public_key_exists: bool, - /// The URL (HTTP) of the NNS node(s). - pub nns_urls: Vec, - pub node_operator_private_key_exists: bool, - /// This ssh keys directory contains individual files named `admin`, `backup`, `readonly`. - /// The contents of these files serve as `authorized_keys` for their respective role account. - /// This means that, for example, `accounts_ssh_authorized_keys/admin` - /// is transferred to `~admin/.ssh/authorized_keys` on the target system. - /// backup and readonly can only be modified via an NNS proposal - /// and are in place for subnet recovery or issue debugging purposes. - /// use_ssh_authorized_keys triggers the use of the ssh keys directory - pub use_ssh_authorized_keys: bool, - pub icos_dev_settings: ICOSDevSettings, -} - -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)] -pub struct ICOSDevSettings {} - #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct Logging { /// Space-separated lists of hosts to ship logs to. @@ -147,7 +151,7 @@ pub struct DeterministicIpv6Config { #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct FixedIpv6Config { - // fixed ipv6 address includes subnet mask /64 + // Fixed ipv6 address includes subnet mask /64 pub address: String, pub gateway: Ipv6Addr, } From 6125aafc4238caa78b42e338113445f4e47f8f82 Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Wed, 30 Oct 2024 17:10:35 +0000 Subject: [PATCH 08/17] Update config unit test --- rs/ic_os/config/src/lib.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index b01b6fa9e5a..033e591d078 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -201,19 +201,25 @@ mod tests { { "network_settings": { "ipv6_config": { - "Fixed": { - "address": "2a00:fb01:400:200::2/64", + "Deterministic": { + "prefix": "2a00:fb01:400:200", + "prefix_length": 64, "gateway": "2a00:fb01:400:200::1" } }, - "ipv4_config": null + "ipv4_config": { + "address": "192.168.0.2", + "gateway": "192.168.0.1", + "prefix_length": 24, + "domain": "example.com" + } }, "icos_settings": { "config_version": "1.0.0", "mgmt_mac": "ec:2a:72:31:a2:0c", "deployment_environment": "Mainnet", "logging": { - "elasticsearch_hosts": "elasticsearch-node-0.mercury.dfinity.systems:443", + "elasticsearch_hosts": "elasticsearch-node-0.mercury.dfinity.systems:443 elasticsearch-node-1.mercury.dfinity.systems:443", "elasticsearch_tags": "tag1 tag2" }, "nns_public_key_exists": true, @@ -225,20 +231,20 @@ mod tests { "icos_dev_settings": {} }, "guestos_settings": { - "inject_ic_crypto": true, - "inject_ic_state": true, - "inject_ic_registry_local_store": true, + "inject_ic_crypto": false, + "inject_ic_state": false, + "inject_ic_registry_local_store": false, "guestos_dev_settings": { "backup_spool": { - "backup_retention_time_seconds": 7200, - "backup_purging_interval_seconds": 1200 + "backup_retention_time_seconds": 3600, + "backup_purging_interval_seconds": 600 }, "malicious_behavior": null, - "query_stats_epoch_length": 2000, + "query_stats_epoch_length": 1000, "bitcoind_addr": "127.0.0.1:8333", "jaeger_addr": "127.0.0.1:6831", "socks_proxy": "127.0.0.1:1080", - "hostname": "guest-node" + "hostname": "my-node" } } } From 657ec7b81e4208ec5c1c1c16b9d9810612fee633 Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Wed, 30 Oct 2024 17:34:03 +0000 Subject: [PATCH 09/17] Add Configuration Update Protocol --- rs/ic_os/config/README.md | 5 ++++- rs/ic_os/config/src/types.rs | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/rs/ic_os/config/README.md b/rs/ic_os/config/README.md index 53567a8c9f6..61628086964 100644 --- a/rs/ic_os/config/README.md +++ b/rs/ic_os/config/README.md @@ -6,6 +6,9 @@ SetupOS transforms user-facing configuration files (like `config.ini`, `deployme All access to configuration and the config partition should go through the config structures. -For testing, IC-OS Config is also used to create HostOS and GuestOS configuration directly. +For testing, IC-OS Config is also used to create GuestOS configuration directly. + +When updating the IC-OS configuration, it's crucial to ensure backwards compatibility. +For detailed guidelines on updating the configuration, please refer to the documentation in [`types.rs`](src/types.rs). For details on the IC-OS configuration mechanism, refer to [ic-os/docs/Configuration.adoc](../../../ic-os/docs/Configuration.adoc) \ No newline at end of file diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index b2001546655..cc355a5809f 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -1,3 +1,19 @@ +//! # Configuration Update Protocol +//! +//! When updating the IC-OS configuration, it's crucial to maintain backwards compatibility. +//! Please adhere to the following guidelines when making changes to the configuration structures: +//! +//! - **Backwards Compatibility**: All updates should be backwards compatible to ensure that older configuration files are still deserializable. +//! +//! - **Updating `CONFIG_VERSION`**: Always update the `CONFIG_VERSION` constant (increment the minor version) whenever you modify the configuration. +//! +//! - **Unit Tests**: Add a unit test in `lib.rs` that tests deserialization of your new configuration version. +//! +//! - **Adding New Fields**: If adding a new field to a configuration struct, make sure it is optional or has a default value by implementing `Default` or via `#[serde(default)]`. +//! +//! - **Removing Fields**: If removing a field, ensure all references to it in the IC-OS codebase are eliminated. +//! +//! - **Renaming Fields**: Avoid renaming fields unless absolutely necessary. If you must rename a field, use `#[serde(rename = "old_name")]`. use ic_types::malicious_behaviour::MaliciousBehaviour; use mac_address::mac_address::FormattedMacAddress; use serde::{Deserialize, Serialize}; From e0a78c8624fbb887f8888f6b9ffc1589c4d1fcaa Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Wed, 30 Oct 2024 18:05:09 +0000 Subject: [PATCH 10/17] Update types.rs documentation --- rs/ic_os/config/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index cc355a5809f..02b7968fd45 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -3,7 +3,7 @@ //! When updating the IC-OS configuration, it's crucial to maintain backwards compatibility. //! Please adhere to the following guidelines when making changes to the configuration structures: //! -//! - **Backwards Compatibility**: All updates should be backwards compatible to ensure that older configuration files are still deserializable. +//! - **Backwards Compatibility**: Configuration persists across reboots, so all config updates should be backwards compatible to ensure that older configuration files are still deserializable across GuestOS and HostOS upgrades. //! //! - **Updating `CONFIG_VERSION`**: Always update the `CONFIG_VERSION` constant (increment the minor version) whenever you modify the configuration. //! From 30dbd3f34f1ef4bd3bd486ed77235cbb231c95ac Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Thu, 31 Oct 2024 19:39:28 +0000 Subject: [PATCH 11/17] Add generate_ic_boundary_tls_cert --- rs/ic_os/config/src/generate_testnet_config.rs | 3 +++ rs/ic_os/config/src/main.rs | 3 +++ rs/ic_os/config/src/types.rs | 3 +++ 3 files changed, 9 insertions(+) diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index d5d70887644..66c434fd705 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -45,6 +45,7 @@ pub struct GenerateTestnetConfigArgs { pub jaeger_addr: Option, pub socks_proxy: Option, pub hostname: Option, + pub generate_ic_boundary_tls_cert: Option, } #[derive(Clone, clap::ValueEnum)] @@ -90,6 +91,7 @@ pub fn generate_testnet_config( jaeger_addr, socks_proxy, hostname, + generate_ic_boundary_tls_cert, } = config; // Construct the NetworkSettings @@ -226,6 +228,7 @@ pub fn generate_testnet_config( jaeger_addr, socks_proxy, hostname, + generate_ic_boundary_tls_cert, }; // Construct GuestOSSettings diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index 668d1634b4b..d4a9b944a33 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -128,6 +128,8 @@ pub struct GenerateTestnetConfigClapArgs { pub socks_proxy: Option, #[arg(long)] pub hostname: Option, + #[arg(long)] + pub generate_ic_boundary_tls_cert: Option, // Output path #[arg(long)] @@ -353,6 +355,7 @@ pub fn main() -> Result<()> { jaeger_addr: clap_args.jaeger_addr, socks_proxy: clap_args.socks_proxy, hostname: clap_args.hostname, + generate_ic_boundary_tls_cert: clap_args.generate_ic_boundary_tls_cert, }; generate_testnet_config(args, clap_args.guestos_config_json_path) diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index 02b7968fd45..bf5fa1636c0 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -118,6 +118,9 @@ pub struct GuestOSDevSettings { pub socks_proxy: Option, // An optional hostname to override the deterministically generated hostname pub hostname: Option, + // Generate and inject a self-signed TLS certificate and key for ic-boundary + // for the given domain name. To be used in system tests only. + pub generate_ic_boundary_tls_cert: Option, } /// Configures the usage of the backup spool directory. From ded916628d323d918d2e290cd79420dc7960d25f Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Fri, 1 Nov 2024 14:37:24 +0000 Subject: [PATCH 12/17] Add generate_ic_boundary_tls_cert to config unit test --- rs/ic_os/config/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index 033e591d078..b0c72af1b19 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -191,7 +191,8 @@ mod tests { "bitcoind_addr": "127.0.0.1:8333", "jaeger_addr": "127.0.0.1:6831", "socks_proxy": "127.0.0.1:1080", - "hostname": "my-node" + "hostname": "my-node", + "generate_ic_boundary_tls_cert": "domain" } } } @@ -244,7 +245,8 @@ mod tests { "bitcoind_addr": "127.0.0.1:8333", "jaeger_addr": "127.0.0.1:6831", "socks_proxy": "127.0.0.1:1080", - "hostname": "my-node" + "hostname": "my-node", + "generate_ic_boundary_tls_cert": "domain" } } } From d9702c0323efb1c189a653550a6848a46fc02718 Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Fri, 1 Nov 2024 14:51:20 +0000 Subject: [PATCH 13/17] Uncouple domain_name from ipv4_config --- rs/ic_os/config/src/config_ini.rs | 11 +++++---- .../config/src/generate_testnet_config.rs | 19 +++++++-------- rs/ic_os/config/src/lib.rs | 13 +++++----- rs/ic_os/config/src/main.rs | 24 +++++++++---------- rs/ic_os/config/src/types.rs | 2 +- 5 files changed, 34 insertions(+), 35 deletions(-) diff --git a/rs/ic_os/config/src/config_ini.rs b/rs/ic_os/config/src/config_ini.rs index 4bc6c92219c..da8f899102b 100644 --- a/rs/ic_os/config/src/config_ini.rs +++ b/rs/ic_os/config/src/config_ini.rs @@ -15,7 +15,7 @@ pub struct ConfigIniSettings { pub ipv4_address: Option, pub ipv4_gateway: Option, pub ipv4_prefix_length: Option, - pub domain: Option, + pub domain_name: Option, pub verbose: bool, } @@ -82,7 +82,7 @@ pub fn get_config_ini_settings(config_file_path: &Path) -> Result Result()? ); assert_eq!(config_ini_settings.ipv4_prefix_length.unwrap(), 28); - assert_eq!(config_ini_settings.domain, Some("example.com".to_string())); + assert_eq!( + config_ini_settings.domain_name, + Some("example.com".to_string()) + ); assert!(!config_ini_settings.verbose); // Test missing ipv6 diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index 66c434fd705..6085e7945c1 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -19,7 +19,7 @@ pub struct GenerateTestnetConfigArgs { pub ipv4_address: Option, pub ipv4_gateway: Option, pub ipv4_prefix_length: Option, - pub ipv4_domain: Option, + pub domain_name: Option, // ICOSSettings arguments pub mgmt_mac: Option, @@ -71,7 +71,7 @@ pub fn generate_testnet_config( ipv4_address, ipv4_gateway, ipv4_prefix_length, - ipv4_domain, + domain_name, mgmt_mac, deployment_environment, elasticsearch_hosts, @@ -139,8 +139,8 @@ pub fn generate_testnet_config( Some(Ipv6ConfigType::RouterAdvertisement) | None => Ipv6Config::RouterAdvertisement, }; - let ipv4_config = match (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) { - (Some(addr_str), Some(gw_str), Some(prefix_len), Some(domain)) => Some(Ipv4Config { + let ipv4_config = match (ipv4_address, ipv4_gateway, ipv4_prefix_length) { + (Some(addr_str), Some(gw_str), Some(prefix_len)) => Some(Ipv4Config { address: addr_str .parse::() .map_err(|e| anyhow::anyhow!("Failed to parse ipv4_address: {}", e))?, @@ -148,17 +148,17 @@ pub fn generate_testnet_config( .parse::() .map_err(|e| anyhow::anyhow!("Failed to parse ipv4_gateway: {}", e))?, prefix_length: prefix_len, - domain, }), - (None, None, None, None) => None, + (None, None, None) => None, _ => { - anyhow::bail!("Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) are required for IPv4 configuration."); + anyhow::bail!("Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length) are required for IPv4 configuration."); } }; let network_settings = NetworkSettings { ipv6_config, ipv4_config, + domain_name, }; // Construct ICOSSettings @@ -405,7 +405,6 @@ mod tests { ipv4_address: Some("192.0.2.1".to_string()), ipv4_gateway: Some("192.0.2.254".to_string()), ipv4_prefix_length: None, - ipv4_domain: Some("example.com".to_string()), ..Default::default() }; @@ -413,7 +412,7 @@ mod tests { assert!(result.is_err()); assert_eq!( result.unwrap_err().to_string(), - "Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length, ipv4_domain) are required for IPv4 configuration." + "Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length) are required for IPv4 configuration." ); } @@ -423,7 +422,6 @@ mod tests { ipv4_address: Some("invalid_ip".to_string()), ipv4_gateway: Some("192.0.2.254".to_string()), ipv4_prefix_length: Some(24), - ipv4_domain: Some("example.com".to_string()), ..Default::default() }; @@ -441,7 +439,6 @@ mod tests { ipv4_address: Some("192.0.2.1".to_string()), ipv4_gateway: Some("invalid_ip".to_string()), ipv4_prefix_length: Some(24), - ipv4_domain: Some("example.com".to_string()), ..Default::default() }; diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index b0c72af1b19..08e53f533a7 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -59,6 +59,7 @@ mod tests { let network_settings = NetworkSettings { ipv6_config, ipv4_config: None, + domain_name: None, }; let logging = Logging { elasticsearch_hosts: [ @@ -152,9 +153,9 @@ mod tests { "ipv4_config": { "address": "192.168.0.2", "gateway": "192.168.0.1", - "prefix_length": 24, - "domain": "example.com" - } + "prefix_length": 24 + }, + "domain_name": "example.com" }, "icos_settings": { "config_version": "1.0.0", @@ -211,9 +212,9 @@ mod tests { "ipv4_config": { "address": "192.168.0.2", "gateway": "192.168.0.1", - "prefix_length": 24, - "domain": "example.com" - } + "prefix_length": 24 + }, + "domain_name": "example.com" }, "icos_settings": { "config_version": "1.0.0", diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index d4a9b944a33..b81e534b197 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -83,7 +83,7 @@ pub struct GenerateTestnetConfigClapArgs { #[arg(long)] pub ipv4_prefix_length: Option, #[arg(long)] - pub ipv4_domain: Option, + pub domain_name: Option, // ICOSSettings arguments #[arg(long)] @@ -156,7 +156,7 @@ pub fn main() -> Result<()> { ipv4_address, ipv4_gateway, ipv4_prefix_length, - domain, + domain_name, verbose, } = get_config_ini_settings(&config_ini_path)?; @@ -167,16 +167,13 @@ pub fn main() -> Result<()> { gateway: ipv6_gateway, }; - let ipv4_config = match (ipv4_address, ipv4_gateway, ipv4_prefix_length, domain) { - (Some(address), Some(gateway), Some(prefix_length), Some(domain)) => { - Some(Ipv4Config { - address, - gateway, - prefix_length, - domain, - }) - } - (None, None, None, None) => None, + let ipv4_config = match (ipv4_address, ipv4_gateway, ipv4_prefix_length) { + (Some(address), Some(gateway), Some(prefix_length)) => Some(Ipv4Config { + address, + gateway, + prefix_length, + }), + (None, None, None) => None, _ => { println!("Warning: Partial IPv4 configuration provided. All parameters are required for IPv4 configuration."); None @@ -186,6 +183,7 @@ pub fn main() -> Result<()> { let network_settings = NetworkSettings { ipv6_config: Ipv6Config::Deterministic(deterministic_config), ipv4_config, + domain_name, }; // get deployment.json variables @@ -335,7 +333,7 @@ pub fn main() -> Result<()> { ipv4_address: clap_args.ipv4_address, ipv4_gateway: clap_args.ipv4_gateway, ipv4_prefix_length: clap_args.ipv4_prefix_length, - ipv4_domain: clap_args.ipv4_domain, + domain_name: clap_args.domain_name, mgmt_mac: clap_args.mgmt_mac, deployment_environment: clap_args.deployment_environment, elasticsearch_hosts: clap_args.elasticsearch_hosts, diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index bf5fa1636c0..65f918d8830 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -144,6 +144,7 @@ pub struct Logging { pub struct NetworkSettings { pub ipv6_config: Ipv6Config, pub ipv4_config: Option, + pub domain_name: Option, } #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] @@ -151,7 +152,6 @@ pub struct Ipv4Config { pub address: Ipv4Addr, pub gateway: Ipv4Addr, pub prefix_length: u8, - pub domain: String, } #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] From 33aed8b57983386838c9e5cd5fc82950dc041b2e Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Fri, 1 Nov 2024 16:35:46 +0000 Subject: [PATCH 14/17] Use tempfile and remove redundant is_err assertion --- .../config/src/generate_testnet_config.rs | 160 +++++++++++++----- 1 file changed, 115 insertions(+), 45 deletions(-) diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index 6085e7945c1..0a56b690f9e 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -261,7 +261,7 @@ pub fn generate_testnet_config( #[cfg(test)] mod tests { use super::*; - use std::path::PathBuf; + use tempfile::NamedTempFile; #[test] fn test_valid_configuration() { @@ -272,8 +272,13 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/tmp/guestos_config.json")); - assert!(result.is_ok()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + generate_testnet_config(args, temp_file_path) + .expect("Expected valid configuration to succeed"); } #[test] @@ -286,10 +291,16 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected an error due to missing deterministic_prefix"); + assert_eq!( - result.unwrap_err().to_string(), + err.to_string(), "deterministic_prefix is required when ipv6_config_type is 'Deterministic'" ); } @@ -304,10 +315,16 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected an error due to missing deterministic_prefix_length"); + assert_eq!( - result.unwrap_err().to_string(), + err.to_string(), "deterministic_prefix_length is required when ipv6_config_type is 'Deterministic'" ); } @@ -322,10 +339,16 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected an error due to missing deterministic_gateway"); + assert_eq!( - result.unwrap_err().to_string(), + err.to_string(), "deterministic_gateway is required when ipv6_config_type is 'Deterministic'" ); } @@ -340,12 +363,20 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to parse deterministic_gateway")); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected parsing error due to invalid deterministic_gateway"); + + assert!( + err.to_string() + .contains("Failed to parse deterministic_gateway"), + "Expected parsing error, got: {}", + err + ); } #[test] @@ -357,10 +388,16 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected an error due to missing fixed_address"); + assert_eq!( - result.unwrap_err().to_string(), + err.to_string(), "fixed_address is required when ipv6_config_type is 'Fixed'" ); } @@ -374,10 +411,16 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected an error due to missing fixed_gateway"); + assert_eq!( - result.unwrap_err().to_string(), + err.to_string(), "fixed_gateway is required when ipv6_config_type is 'Fixed'" ); } @@ -391,12 +434,19 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to parse fixed_gateway")); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected parsing error due to invalid fixed_gateway"); + + assert!( + err.to_string().contains("Failed to parse fixed_gateway"), + "Expected parsing error, got: {}", + err + ); } #[test] @@ -408,10 +458,16 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected an error due to incomplete IPv4 configuration"); + assert_eq!( - result.unwrap_err().to_string(), + err.to_string(), "Incomplete IPv4 configuration provided. All parameters (ipv4_address, ipv4_gateway, ipv4_prefix_length) are required for IPv4 configuration." ); } @@ -425,12 +481,19 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to parse ipv4_address")); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected parsing error due to invalid ipv4_address"); + + assert!( + err.to_string().contains("Failed to parse ipv4_address"), + "Expected parsing error, got: {}", + err + ); } #[test] @@ -442,11 +505,18 @@ mod tests { ..Default::default() }; - let result = generate_testnet_config(args, PathBuf::from("/dev/null")); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to parse ipv4_gateway")); + let temp_file_path = NamedTempFile::new() + .expect("Failed to create temp file") + .path() + .to_path_buf(); + + let err = generate_testnet_config(args, temp_file_path) + .expect_err("Expected parsing error due to invalid ipv4_gateway"); + + assert!( + err.to_string().contains("Failed to parse ipv4_gateway"), + "Expected parsing error, got: {}", + err + ); } } From c26af760d5dd5fed5e71e25f6a27051d641fc1ca Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Tue, 5 Nov 2024 19:09:25 +0000 Subject: [PATCH 15/17] Move config_version to *OSConfig structs --- rs/ic_os/config/src/generate_testnet_config.rs | 2 +- rs/ic_os/config/src/lib.rs | 12 +++++++----- rs/ic_os/config/src/main.rs | 4 +++- rs/ic_os/config/src/types.rs | 8 ++++++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/rs/ic_os/config/src/generate_testnet_config.rs b/rs/ic_os/config/src/generate_testnet_config.rs index 0a56b690f9e..16598a53e65 100644 --- a/rs/ic_os/config/src/generate_testnet_config.rs +++ b/rs/ic_os/config/src/generate_testnet_config.rs @@ -192,7 +192,6 @@ pub fn generate_testnet_config( let use_ssh_authorized_keys = use_ssh_authorized_keys.unwrap_or(true); let icos_settings = ICOSSettings { - config_version: CONFIG_VERSION.to_string(), mgmt_mac, deployment_environment, logging, @@ -241,6 +240,7 @@ pub fn generate_testnet_config( // Assemble GuestOSConfig let guestos_config = GuestOSConfig { + config_version: CONFIG_VERSION.to_string(), network_settings, icos_settings, guestos_settings, diff --git a/rs/ic_os/config/src/lib.rs b/rs/ic_os/config/src/lib.rs index 08e53f533a7..9a129ca6b3e 100644 --- a/rs/ic_os/config/src/lib.rs +++ b/rs/ic_os/config/src/lib.rs @@ -73,7 +73,6 @@ mod tests { }; let icos_dev_settings = ICOSDevSettings::default(); let icos_settings = ICOSSettings { - config_version: CONFIG_VERSION.to_string(), mgmt_mac: FormattedMacAddress::try_from("ec:2a:72:31:a2:0c")?, deployment_environment: "Mainnet".to_string(), logging, @@ -97,6 +96,7 @@ mod tests { }; let setupos_config_struct = SetupOSConfig { + config_version: CONFIG_VERSION.to_string(), network_settings: network_settings.clone(), icos_settings: icos_settings.clone(), setupos_settings: setupos_settings.clone(), @@ -104,12 +104,14 @@ mod tests { guestos_settings: guestos_settings.clone(), }; let hostos_config_struct = HostOSConfig { + config_version: CONFIG_VERSION.to_string(), network_settings: network_settings.clone(), icos_settings: icos_settings.clone(), hostos_settings: hostos_settings.clone(), guestos_settings: guestos_settings.clone(), }; let guestos_config_struct = GuestOSConfig { + config_version: CONFIG_VERSION.to_string(), network_settings: network_settings.clone(), icos_settings: icos_settings.clone(), guestos_settings: guestos_settings.clone(), @@ -142,6 +144,7 @@ mod tests { // Test config version 1.0.0 const HOSTOS_CONFIG_JSON_V1_0_0: &str = r#" { + "config_version": "1.0.0", "network_settings": { "ipv6_config": { "Deterministic": { @@ -158,7 +161,6 @@ mod tests { "domain_name": "example.com" }, "icos_settings": { - "config_version": "1.0.0", "mgmt_mac": "ec:2a:72:31:a2:0c", "deployment_environment": "Mainnet", "logging": { @@ -201,6 +203,7 @@ mod tests { const GUESTOS_CONFIG_JSON_V1_0_0: &str = r#" { + "config_version": "1.0.0", "network_settings": { "ipv6_config": { "Deterministic": { @@ -217,7 +220,6 @@ mod tests { "domain_name": "example.com" }, "icos_settings": { - "config_version": "1.0.0", "mgmt_mac": "ec:2a:72:31:a2:0c", "deployment_environment": "Mainnet", "logging": { @@ -256,7 +258,7 @@ mod tests { #[test] fn test_deserialize_hostos_config_v1_0_0() -> Result<(), Box> { let config: HostOSConfig = serde_json::from_str(HOSTOS_CONFIG_JSON_V1_0_0)?; - assert_eq!(config.icos_settings.config_version, "1.0.0"); + assert_eq!(config.config_version, "1.0.0"); assert_eq!(config.hostos_settings.vm_cpu, "kvm"); Ok(()) } @@ -264,7 +266,7 @@ mod tests { #[test] fn test_deserialize_guestos_config_v1_0_0() -> Result<(), Box> { let config: GuestOSConfig = serde_json::from_str(GUESTOS_CONFIG_JSON_V1_0_0)?; - assert_eq!(config.icos_settings.config_version, "1.0.0"); + assert_eq!(config.config_version, "1.0.0"); assert_eq!( config.icos_settings.mgmt_mac.to_string(), "ec:2a:72:31:a2:0c" diff --git a/rs/ic_os/config/src/main.rs b/rs/ic_os/config/src/main.rs index b81e534b197..4c2d34e0e27 100644 --- a/rs/ic_os/config/src/main.rs +++ b/rs/ic_os/config/src/main.rs @@ -207,7 +207,6 @@ pub fn main() -> Result<()> { }; let icos_settings = ICOSSettings { - config_version: CONFIG_VERSION.to_string(), mgmt_mac, deployment_environment: deployment_json_settings.deployment.name, logging, @@ -233,6 +232,7 @@ pub fn main() -> Result<()> { let guestos_settings = GuestOSSettings::default(); let setupos_config = SetupOSConfig { + config_version: CONFIG_VERSION.to_string(), network_settings, icos_settings, setupos_settings, @@ -261,6 +261,7 @@ pub fn main() -> Result<()> { serde_json::from_reader(File::open(setupos_config_json_path)?)?; let hostos_config = HostOSConfig { + config_version: setupos_config.config_version, network_settings: setupos_config.network_settings, icos_settings: setupos_config.icos_settings, hostos_settings: setupos_config.hostos_settings, @@ -306,6 +307,7 @@ pub fn main() -> Result<()> { } let guestos_config = GuestOSConfig { + config_version: hostos_config.config_version, network_settings: guestos_network_settings, icos_settings: hostos_config.icos_settings, guestos_settings: hostos_config.guestos_settings, diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index 65f918d8830..e0d18aa4eb4 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -26,6 +26,8 @@ pub const CONFIG_VERSION: &str = "1.0.0"; /// (e.g., `config.ini`, `deployment.json`) are transformed into `SetupOSConfig`. #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct SetupOSConfig { + /// Tracks the config version, set to CONFIG_VERSION at runtime. + pub config_version: String, pub network_settings: NetworkSettings, pub icos_settings: ICOSSettings, pub setupos_settings: SetupOSSettings, @@ -36,6 +38,8 @@ pub struct SetupOSConfig { /// HostOS configuration. In production, this struct inherits settings from `SetupOSConfig`. #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct HostOSConfig { + /// Tracks the config version, set to CONFIG_VERSION at runtime. + pub config_version: String, pub network_settings: NetworkSettings, pub icos_settings: ICOSSettings, pub hostos_settings: HostOSSettings, @@ -45,6 +49,8 @@ pub struct HostOSConfig { /// GuestOS configuration. In production, this struct inherits settings from `HostOSConfig`. #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct GuestOSConfig { + /// Tracks the config version, set to CONFIG_VERSION at runtime. + pub config_version: String, pub network_settings: NetworkSettings, pub icos_settings: ICOSSettings, pub guestos_settings: GuestOSSettings, @@ -52,8 +58,6 @@ pub struct GuestOSConfig { #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct ICOSSettings { - /// Tracks the config version, set to CONFIG_VERSION at runtime. - pub config_version: String, /// In nested testing, mgmt_mac is set in deployment.json.template, /// else found dynamically in call to config tool CreateSetuposConfig pub mgmt_mac: FormattedMacAddress, From 0812800bcdfb6078e4c856ee953322b85ebcab3c Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Tue, 5 Nov 2024 19:26:34 +0000 Subject: [PATCH 16/17] Update config policy documentation --- rs/ic_os/config/README.md | 3 ++- rs/ic_os/config/src/types.rs | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/rs/ic_os/config/README.md b/rs/ic_os/config/README.md index 61628086964..5be108377cc 100644 --- a/rs/ic_os/config/README.md +++ b/rs/ic_os/config/README.md @@ -1,6 +1,6 @@ # IC-OS Config -IC-OS Config is responsible for managing the configuration of IC-OS images. +IC-OS Config is responsible for managing the configuration of IC-OS images. SetupOS transforms user-facing configuration files (like `config.ini`, `deployment.json`, etc.) into a SetupOSConfig struct. Then, in production, configuration is propagated from SetupOS → HostOS → GuestOS (→ replica) via the HostOSConfig and GuestOSConfig structures. @@ -10,5 +10,6 @@ For testing, IC-OS Config is also used to create GuestOS configuration directly. When updating the IC-OS configuration, it's crucial to ensure backwards compatibility. For detailed guidelines on updating the configuration, please refer to the documentation in [`types.rs`](src/types.rs). +Any changes to the configuration should undergo a thorough review process to ensure they follow the guidlines. For details on the IC-OS configuration mechanism, refer to [ic-os/docs/Configuration.adoc](../../../ic-os/docs/Configuration.adoc) \ No newline at end of file diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index e0d18aa4eb4..68dda00eb34 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -11,7 +11,9 @@ //! //! - **Adding New Fields**: If adding a new field to a configuration struct, make sure it is optional or has a default value by implementing `Default` or via `#[serde(default)]`. //! -//! - **Removing Fields**: If removing a field, ensure all references to it in the IC-OS codebase are eliminated. +//! - **Removing Fields**: To prevent backwards-compatibility deserialization errors, required fields must not be removed directly: +//! In a first step, they have to be made optional and code that reads the value must be removed/handle missing values. +//! In a second step, after the first step has rolled out to all OSes and there is no risk of a rollback, the field can be removed. //! //! - **Renaming Fields**: Avoid renaming fields unless absolutely necessary. If you must rename a field, use `#[serde(rename = "old_name")]`. use ic_types::malicious_behaviour::MaliciousBehaviour; From 1632a9b2bddf917921b615bae597122b79c02238 Mon Sep 17 00:00:00 2001 From: Andrew Battat Date: Tue, 5 Nov 2024 23:01:18 +0000 Subject: [PATCH 17/17] Fix cargo clippy --- rs/ic_os/config/src/types.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rs/ic_os/config/src/types.rs b/rs/ic_os/config/src/types.rs index 68dda00eb34..50a53e17fbb 100644 --- a/rs/ic_os/config/src/types.rs +++ b/rs/ic_os/config/src/types.rs @@ -11,9 +11,7 @@ //! //! - **Adding New Fields**: If adding a new field to a configuration struct, make sure it is optional or has a default value by implementing `Default` or via `#[serde(default)]`. //! -//! - **Removing Fields**: To prevent backwards-compatibility deserialization errors, required fields must not be removed directly: -//! In a first step, they have to be made optional and code that reads the value must be removed/handle missing values. -//! In a second step, after the first step has rolled out to all OSes and there is no risk of a rollback, the field can be removed. +//! - **Removing Fields**: To prevent backwards-compatibility deserialization errors, required fields must not be removed directly: In a first step, they have to be made optional and code that reads the value must be removed/handle missing values. In a second step, after the first step has rolled out to all OSes and there is no risk of a rollback, the field can be removed. //! //! - **Renaming Fields**: Avoid renaming fields unless absolutely necessary. If you must rename a field, use `#[serde(rename = "old_name")]`. use ic_types::malicious_behaviour::MaliciousBehaviour;