Skip to content

Commit 035a2c7

Browse files
andrewbattatandrewbattat
authored
test(node): add OSConfig Default traits (#8204)
NODE-1808 Adds GuestOSConfig, HostOSConfig, and SetupOSConfig Default trains to reduce test boilerplate. Unit tests now only must specify specific fields they care about.
1 parent b8225ef commit 035a2c7

File tree

9 files changed

+44
-275
lines changed

9 files changed

+44
-275
lines changed

‎rs/ic_os/config/src/guestos/generate_ic_config.rs‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,7 @@ fn generate_tls_certificate(domain_name: &str) -> Result<()> {
296296
#[cfg(test)]
297297
mod tests {
298298
use super::*;
299-
use config_types::{
300-
CONFIG_VERSION, FixedIpv6Config, GuestOSConfig, GuestOSSettings, GuestOSUpgradeConfig,
301-
GuestVMType, ICOSSettings, Ipv6Config, NetworkSettings,
302-
};
299+
use config_types::{FixedIpv6Config, GuestOSConfig, Ipv6Config, NetworkSettings};
303300
use ic_config::{ConfigOptional, config_parser::ConfigSource};
304301

305302
#[test]
@@ -375,30 +372,14 @@ mod tests {
375372

376373
fn create_test_guestos_config() -> GuestOSConfig {
377374
GuestOSConfig {
378-
config_version: CONFIG_VERSION.to_string(),
379375
network_settings: NetworkSettings {
380376
ipv6_config: Ipv6Config::Fixed(FixedIpv6Config {
381377
address: "2001:db8::1/64".to_string(),
382378
gateway: "2001:db8::1".parse().unwrap(),
383379
}),
384-
ipv4_config: None,
385-
domain_name: None,
380+
..Default::default()
386381
},
387-
icos_settings: ICOSSettings {
388-
node_reward_type: None,
389-
mgmt_mac: "00:00:00:00:00:01".parse().unwrap(),
390-
deployment_environment: config_types::DeploymentEnvironment::Mainnet,
391-
nns_urls: vec![],
392-
use_node_operator_private_key: false,
393-
enable_trusted_execution_environment: false,
394-
use_ssh_authorized_keys: false,
395-
icos_dev_settings: config_types::ICOSDevSettings::default(),
396-
},
397-
guestos_settings: GuestOSSettings::default(),
398-
guest_vm_type: GuestVMType::Default,
399-
upgrade_config: GuestOSUpgradeConfig::default(),
400-
trusted_execution_environment_config: None,
401-
recovery_config: None,
382+
..GuestOSConfig::default()
402383
}
403384
}
404385
}

‎rs/ic_os/config/src/hostos/guestos_bootstrap_image.rs‎

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -223,45 +223,14 @@ mod tests {
223223
#[test]
224224
#[cfg(feature = "dev")]
225225
fn test_build_bootstrap_tar_with_all_options() -> Result<()> {
226-
use config_types::{
227-
DeploymentEnvironment, GuestOSUpgradeConfig, GuestVMType, ICOSSettings, Ipv6Config,
228-
NetworkSettings,
229-
};
230-
use std::net::Ipv6Addr;
231-
use std::str::FromStr;
232-
233226
let tmp_dir = tempfile::tempdir()?;
234227
let out_file = tmp_dir.path().join("bootstrap.tar");
235228

236229
// Create test files and directories
237230
let test_files_dir = tmp_dir.path().join("test_files");
238231
fs::create_dir(&test_files_dir)?;
239232

240-
let guestos_config = GuestOSConfig {
241-
config_version: "".to_string(),
242-
network_settings: NetworkSettings {
243-
ipv6_config: Ipv6Config::RouterAdvertisement,
244-
ipv4_config: None,
245-
domain_name: None,
246-
},
247-
icos_settings: ICOSSettings {
248-
node_reward_type: None,
249-
mgmt_mac: Default::default(),
250-
deployment_environment: DeploymentEnvironment::Mainnet,
251-
nns_urls: vec![],
252-
use_node_operator_private_key: false,
253-
enable_trusted_execution_environment: false,
254-
use_ssh_authorized_keys: false,
255-
icos_dev_settings: Default::default(),
256-
},
257-
guestos_settings: Default::default(),
258-
guest_vm_type: GuestVMType::Default,
259-
upgrade_config: GuestOSUpgradeConfig {
260-
peer_guest_vm_address: Some(Ipv6Addr::from_str("2001:db8::1")?),
261-
},
262-
trusted_execution_environment_config: None,
263-
recovery_config: Default::default(),
264-
};
233+
let guestos_config = GuestOSConfig::default();
265234

266235
let nns_key_override_path = test_files_dir.join("nns_public_key_override.pem");
267236
fs::write(&nns_key_override_path, "test_nns_key")?;

‎rs/ic_os/config/src/hostos/guestos_config.rs‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -127,37 +127,21 @@ fn guestos_recovery_config(recovery_file_path: &Path) -> Result<Option<RecoveryC
127127
#[cfg(test)]
128128
mod tests {
129129
use super::*;
130-
use config_types::{
131-
DeploymentEnvironment, DeterministicIpv6Config, HostOSConfig, ICOSSettings, Ipv6Config,
132-
NetworkSettings,
133-
};
130+
use config_types::{DeterministicIpv6Config, HostOSConfig, Ipv6Config, NetworkSettings};
134131
use std::net::Ipv6Addr;
135132
use tempfile::tempdir;
136133

137134
fn hostos_config_for_test() -> HostOSConfig {
138135
HostOSConfig {
139-
config_version: "1.0.0".to_string(),
140136
network_settings: NetworkSettings {
141137
ipv6_config: Ipv6Config::Deterministic(DeterministicIpv6Config {
142138
prefix: "2001:db8::".to_string(),
143139
prefix_length: 64,
144140
gateway: "2001:db8::1".parse().unwrap(),
145141
}),
146-
ipv4_config: None,
147-
domain_name: None,
148-
},
149-
icos_settings: ICOSSettings {
150-
node_reward_type: None,
151-
mgmt_mac: Default::default(),
152-
deployment_environment: DeploymentEnvironment::Testnet,
153-
nns_urls: vec![],
154-
use_node_operator_private_key: false,
155-
enable_trusted_execution_environment: false,
156-
use_ssh_authorized_keys: false,
157-
icos_dev_settings: Default::default(),
142+
..Default::default()
158143
},
159-
hostos_settings: Default::default(),
160-
guestos_settings: Default::default(),
144+
..HostOSConfig::default()
161145
}
162146
}
163147
#[test]

‎rs/ic_os/config/src/lib.rs‎

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -43,99 +43,3 @@ pub fn deserialize_config<T: for<'de> Deserialize<'de>, P: AsRef<Path>>(file_pat
4343
file_path.as_ref()
4444
))
4545
}
46-
47-
#[cfg(test)]
48-
mod tests {
49-
use config_types::*;
50-
use std::net::Ipv6Addr;
51-
use std::str::FromStr;
52-
53-
#[test]
54-
fn test_serialize_and_deserialize() {
55-
let ipv6_config = Ipv6Config::Deterministic(DeterministicIpv6Config {
56-
prefix: "2a00:fb01:400:200".to_string(),
57-
prefix_length: 64_u8,
58-
gateway: "2a00:fb01:400:200::1".parse().unwrap(),
59-
});
60-
let network_settings = NetworkSettings {
61-
ipv6_config,
62-
ipv4_config: None,
63-
domain_name: None,
64-
};
65-
let icos_dev_settings = ICOSDevSettings::default();
66-
let icos_settings = ICOSSettings {
67-
node_reward_type: Some("type3.1".to_string()),
68-
mgmt_mac: "ec:2a:72:31:a2:0c".parse().unwrap(),
69-
deployment_environment: DeploymentEnvironment::Mainnet,
70-
nns_urls: vec!["http://localhost".parse().unwrap()],
71-
use_node_operator_private_key: true,
72-
enable_trusted_execution_environment: true,
73-
use_ssh_authorized_keys: false,
74-
icos_dev_settings,
75-
};
76-
let setupos_settings = SetupOSSettings;
77-
let hostos_settings = HostOSSettings {
78-
verbose: false,
79-
hostos_dev_settings: HostOSDevSettings {
80-
vm_memory: 16,
81-
vm_cpu: "kvm".to_string(),
82-
vm_nr_of_vcpus: 64,
83-
},
84-
};
85-
let guestos_settings = GuestOSSettings {
86-
inject_ic_crypto: false,
87-
inject_ic_state: false,
88-
inject_ic_registry_local_store: false,
89-
guestos_dev_settings: GuestOSDevSettings::default(),
90-
};
91-
92-
let setupos_config_struct = SetupOSConfig {
93-
config_version: CONFIG_VERSION.to_string(),
94-
network_settings: network_settings.clone(),
95-
icos_settings: icos_settings.clone(),
96-
setupos_settings: setupos_settings.clone(),
97-
hostos_settings: hostos_settings.clone(),
98-
guestos_settings: guestos_settings.clone(),
99-
};
100-
let hostos_config_struct = HostOSConfig {
101-
config_version: CONFIG_VERSION.to_string(),
102-
network_settings: network_settings.clone(),
103-
icos_settings: icos_settings.clone(),
104-
hostos_settings: hostos_settings.clone(),
105-
guestos_settings: guestos_settings.clone(),
106-
};
107-
let guestos_config_struct = GuestOSConfig {
108-
config_version: CONFIG_VERSION.to_string(),
109-
network_settings: network_settings.clone(),
110-
icos_settings: icos_settings.clone(),
111-
guestos_settings: guestos_settings.clone(),
112-
guest_vm_type: GuestVMType::Default,
113-
upgrade_config: GuestOSUpgradeConfig {
114-
peer_guest_vm_address: Some(Ipv6Addr::from_str("2001:db8::1").unwrap()),
115-
},
116-
trusted_execution_environment_config: None,
117-
recovery_config: None,
118-
};
119-
120-
fn serialize_and_deserialize<T>(config: &T)
121-
where
122-
T: serde::Serialize
123-
+ serde::de::DeserializeOwned
124-
+ std::cmp::PartialEq
125-
+ std::fmt::Debug,
126-
{
127-
// Test serialization
128-
let buffer = serde_json::to_vec_pretty(config).expect("Failed to serialize config");
129-
assert!(!buffer.is_empty());
130-
131-
// Test deserialization
132-
let deserialized_config: T =
133-
serde_json::from_slice(&buffer).expect("Failed to deserialize config");
134-
assert_eq!(*config, deserialized_config);
135-
}
136-
137-
serialize_and_deserialize(&setupos_config_struct);
138-
serialize_and_deserialize(&hostos_config_struct);
139-
serialize_and_deserialize(&guestos_config_struct);
140-
}
141-
}

‎rs/ic_os/config_types/src/lib.rs‎

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@
2020
//! ## Logging safety
2121
//!
2222
//! All configuration objects defined in this file are safe to log. They do not contain any secret material.
23+
//!
24+
//! ## Note on `Default` implementations
25+
//!
26+
//! The `Default` implementations on the OS config structs (`SetupOSConfig`, `HostOSConfig`,
27+
//! `GuestOSConfig`) are **only intended for use in unit tests**. Some default values are
28+
//! not meaningful (e.g., `config_version` defaults to `""`, `mgmt_mac` defaults to
29+
//! `00:00:00:00:00:00`).
30+
//! **Do not use these defaults in system tests or production code.** For system tests,
31+
//! always construct a custom config explicitly.
2332
use ic_types::malicious_behavior::MaliciousBehavior;
2433
use macaddr::MacAddr6;
2534
use serde::{Deserialize, Serialize};
@@ -46,7 +55,7 @@ pub type ConfigMap = HashMap<String, String>;
4655

4756
/// SetupOS configuration. User-facing configuration files
4857
/// (e.g., `config.ini`, `deployment.json`) are transformed into `SetupOSConfig`.
49-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
58+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
5059
pub struct SetupOSConfig {
5160
/// Tracks the config version, set to CONFIG_VERSION at runtime.
5261
pub config_version: String,
@@ -58,7 +67,7 @@ pub struct SetupOSConfig {
5867
}
5968

6069
/// HostOS configuration. In production, this struct inherits settings from `SetupOSConfig`.
61-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
70+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
6271
pub struct HostOSConfig {
6372
/// Tracks the config version, set to CONFIG_VERSION at runtime.
6473
pub config_version: String,
@@ -91,7 +100,7 @@ pub struct TrustedExecutionEnvironmentConfig {
91100
}
92101

93102
/// GuestOS configuration. In production, this struct inherits settings from `HostOSConfig`.
94-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
103+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
95104
pub struct GuestOSConfig {
96105
/// Tracks the config version, set to CONFIG_VERSION at runtime.
97106
pub config_version: String,
@@ -111,7 +120,7 @@ pub struct GuestOSConfig {
111120
}
112121

113122
#[serde_as]
114-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
123+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
115124
pub struct ICOSSettings {
116125
/// The node reward type determines node rewards
117126
pub node_reward_type: Option<String>,
@@ -149,7 +158,7 @@ pub struct ICOSSettings {
149158
pub struct ICOSDevSettings {}
150159

151160
/// Placeholder for SetupOS-specific settings.
152-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
161+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
153162
pub struct SetupOSSettings;
154163

155164
/// HostOS-specific settings.
@@ -240,10 +249,11 @@ pub struct BackupSpoolSettings {
240249
pub backup_purging_interval_seconds: Option<u64>,
241250
}
242251

243-
#[derive(Copy, Clone, Eq, PartialEq, Debug)]
252+
#[derive(Copy, Clone, Eq, PartialEq, Debug, Default)]
244253
#[non_exhaustive]
245254
pub enum DeploymentEnvironment {
246255
Mainnet,
256+
#[default]
247257
Testnet,
248258
}
249259

@@ -273,7 +283,7 @@ impl FromStr for DeploymentEnvironment {
273283
}
274284
}
275285

276-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
286+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
277287
pub struct NetworkSettings {
278288
pub ipv6_config: Ipv6Config,
279289
pub ipv4_config: Option<Ipv4Config>,
@@ -287,10 +297,11 @@ pub struct Ipv4Config {
287297
pub prefix_length: u8,
288298
}
289299

290-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
300+
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
291301
pub enum Ipv6Config {
292302
Deterministic(DeterministicIpv6Config),
293303
Fixed(FixedIpv6Config),
304+
#[default]
294305
RouterAdvertisement,
295306
/// Unknown variant for forward compatibility with future versions
296307
/// (used in case a newer HostOS sends a value that an older GuestOS does not understand)
@@ -356,27 +367,7 @@ mod tests {
356367
fn test_no_reserved_field_paths_used() -> Result<(), Box<dyn std::error::Error>> {
357368
let reserved_field_paths: HashSet<&str> = RESERVED_FIELD_PATHS.iter().cloned().collect();
358369

359-
let setupos_config = SetupOSConfig {
360-
config_version: CONFIG_VERSION.to_string(),
361-
network_settings: NetworkSettings {
362-
ipv6_config: Ipv6Config::RouterAdvertisement,
363-
ipv4_config: None,
364-
domain_name: None,
365-
},
366-
icos_settings: ICOSSettings {
367-
node_reward_type: None,
368-
mgmt_mac: "00:00:00:00:00:00".parse()?,
369-
deployment_environment: DeploymentEnvironment::Testnet,
370-
nns_urls: vec![],
371-
use_node_operator_private_key: false,
372-
enable_trusted_execution_environment: false,
373-
use_ssh_authorized_keys: false,
374-
icos_dev_settings: ICOSDevSettings::default(),
375-
},
376-
setupos_settings: SetupOSSettings,
377-
hostos_settings: HostOSSettings::default(),
378-
guestos_settings: GuestOSSettings::default(),
379-
};
370+
let setupos_config = SetupOSConfig::default();
380371

381372
fn get_all_field_paths(prefix: &str, value: &Value, field_paths: &mut HashSet<String>) {
382373
match value {

0 commit comments

Comments
 (0)