From a02bba231097d5bff8873b1a17a899c402de2a25 Mon Sep 17 00:00:00 2001 From: Christina Ying Wang Date: Wed, 23 Aug 2023 19:32:44 -0700 Subject: [PATCH 1/5] Rename Configuration -> RemoteConfiguration for clarity Signed-off-by: Christina Ying Wang --- src/join.rs | 23 +++++++++++++---------- src/remote.rs | 14 +++++++------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/join.rs b/src/join.rs index 4577137..8044232 100644 --- a/src/join.rs +++ b/src/join.rs @@ -6,7 +6,7 @@ use crate::config_json::{ get_api_endpoint, get_root_certificate, merge_config_json, read_config_json, write_config_json, ConfigMap, }; -use crate::remote::{config_url, fetch_configuration, Configuration}; +use crate::remote::{config_url, fetch_configuration, RemoteConfiguration}; use crate::schema::{read_os_config_schema, OsConfigSchema}; use crate::systemd; use anyhow::Result; @@ -39,13 +39,13 @@ pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Resul let root_certificate = get_root_certificate(config_json)?; - let configuration = fetch_configuration( + let remote_config = fetch_configuration( &config_url(&api_endpoint, &args.config_route), root_certificate, !joining, )?; - let has_config_changes = has_config_changes(&schema, &configuration)?; + let has_config_changes = has_config_changes(&schema, &remote_config)?; if !has_config_changes { info!("No configuration changes"); @@ -65,7 +65,7 @@ pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Resul args, config_json, &schema, - &configuration, + &remote_config, has_config_changes, joining, ); @@ -81,7 +81,7 @@ fn reconfigure_core( args: &Args, config_json: &ConfigMap, schema: &OsConfigSchema, - configuration: &Configuration, + remote_config: &RemoteConfiguration, has_config_changes: bool, joining: bool, ) -> Result<()> { @@ -90,16 +90,19 @@ fn reconfigure_core( } if has_config_changes { - configure_services(schema, configuration)?; + configure_services(schema, remote_config)?; } Ok(()) } -fn has_config_changes(schema: &OsConfigSchema, configuration: &Configuration) -> Result { +fn has_config_changes( + schema: &OsConfigSchema, + remote_config: &RemoteConfiguration, +) -> Result { for service in &schema.services { for (name, config_file) in &service.files { - let future = configuration.get_config_contents(&service.id, name)?; + let future = remote_config.get_config_contents(&service.id, name)?; let current = get_config_contents(&config_file.path); if future != current { @@ -111,7 +114,7 @@ fn has_config_changes(schema: &OsConfigSchema, configuration: &Configuration) -> Ok(false) } -fn configure_services(schema: &OsConfigSchema, configuration: &Configuration) -> Result<()> { +fn configure_services(schema: &OsConfigSchema, remote_config: &RemoteConfiguration) -> Result<()> { for service in &schema.services { for systemd_service in &service.systemd_services { systemd::stop_service(systemd_service)?; @@ -126,7 +129,7 @@ fn configure_services(schema: &OsConfigSchema, configuration: &Configuration) -> names.sort(); for name in names { let config_file = &service.files[name as &str]; - let contents = configuration.get_config_contents(&service.id, name)?; + let contents = remote_config.get_config_contents(&service.id, name)?; let mode = fs::parse_mode(&config_file.perm)?; fs::write_file(Path::new(&config_file.path), contents, mode)?; info!("{} updated", &config_file.path); diff --git a/src/remote.rs b/src/remote.rs index f1f9378..752e9c2 100644 --- a/src/remote.rs +++ b/src/remote.rs @@ -6,12 +6,12 @@ use crate::schema::validate_schema_version; use anyhow::{anyhow, Context, Result}; #[derive(Debug, Serialize, Deserialize, PartialEq)] -pub struct Configuration { +pub struct RemoteConfiguration { pub services: HashMap>, pub schema_version: String, } -impl Configuration { +impl RemoteConfiguration { pub fn get_config_contents<'a>( &'a self, service_id: &str, @@ -42,7 +42,7 @@ pub fn fetch_configuration( config_url: &str, root_certificate: Option, retry: bool, -) -> Result { +) -> Result { fetch_configuration_impl(config_url, root_certificate, retry) .context("Fetching configuration failed") } @@ -51,7 +51,7 @@ fn fetch_configuration_impl( config_url: &str, root_certificate: Option, retry: bool, -) -> Result { +) -> Result { let client = build_reqwest_client(root_certificate)?; let request_fn = if retry { @@ -159,10 +159,10 @@ mod tests { }"#; #[test] - fn parse_configuration_v1() { - let parsed: Configuration = serde_json::from_str(JSON_DATA).unwrap(); + fn parse_configuration() { + let parsed: RemoteConfiguration = serde_json::from_str(JSON_DATA).unwrap(); - let expected = Configuration { + let expected = RemoteConfiguration { services: hashmap! { "openvpn".into() => hashmap!{ "config".into() => "main configuration here".into(), From f0882b960fa8810be462e7c23a64f39b5d1594ae Mon Sep 17 00:00:00 2001 From: Christina Ying Wang Date: Wed, 13 Sep 2023 15:43:23 -0700 Subject: [PATCH 2/5] Migrate existing functionality to new os-config schema The response from querying /os/v1/config has changed slightly, the changes to the response are reflected in the updated tests in this commit. See: https://balena.fibery.io/Work/Improvement/os-config-improving-the-interface-for-config.json-modification-901 See: https://github.com/balena-os/meta-balena/pull/3227 See: https://github.com/balena-io/open-balena-api/pull/1394 Signed-off-by: Christina Ying Wang Change-type: minor --- src/remote.rs | 23 ++++++--- src/schema.rs | 50 +++++++------------- tests/integration.rs | 108 ++++++++++++++++++++++++++++++++----------- 3 files changed, 113 insertions(+), 68 deletions(-) diff --git a/src/remote.rs b/src/remote.rs index 752e9c2..2c790e0 100644 --- a/src/remote.rs +++ b/src/remote.rs @@ -2,13 +2,17 @@ use std::collections::HashMap; use std::thread; use std::time::Duration; -use crate::schema::validate_schema_version; use anyhow::{anyhow, Context, Result}; #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct RemoteConfiguration { pub services: HashMap>, - pub schema_version: String, + pub config: ConfigMigrationInstructions, +} + +#[derive(Debug, Serialize, Deserialize, PartialEq)] +pub struct ConfigMigrationInstructions { + pub overrides: HashMap, } impl RemoteConfiguration { @@ -66,8 +70,6 @@ fn fetch_configuration_impl( info!("Service configuration retrieved"); - validate_schema_version(&json_data)?; - Ok(serde_json::from_str(&json_data)?) } @@ -141,7 +143,6 @@ fn build_reqwest_client( mod tests { use super::*; - use crate::schema::SCHEMA_VERSION; const JSON_DATA: &str = r#"{ "services": { @@ -155,7 +156,11 @@ mod tests { "authorized_keys": "authorized keys here" } }, - "schema_version": "1.0.0" + "config": { + "overrides": { + "logsEndpoint": "https://logs.balenadev.io" + } + } }"#; #[test] @@ -174,7 +179,11 @@ mod tests { "authorized_keys".into() => "authorized keys here".into() } }, - schema_version: SCHEMA_VERSION.into(), + config: ConfigMigrationInstructions { + overrides: hashmap! { + "logsEndpoint".into() => "https://logs.balenadev.io".into() + }, + }, }; assert_eq!(parsed, expected); diff --git a/src/schema.rs b/src/schema.rs index 0ed83fc..fba2651 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -1,18 +1,14 @@ +use crate::fs::read_file; +use anyhow::{Context, Result}; use std::collections::HashMap; use std::path::Path; -use serde_json::Value; - -use crate::fs::read_file; -use anyhow::{bail, Context, Result}; - -pub const SCHEMA_VERSION: &str = "1.0.0"; - #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct OsConfigSchema { pub services: Vec, + // Fields that should be removed from config.json when leaving a cloud env (`balena leave`) pub keys: Vec, - pub schema_version: String, + pub config: ConfigJsonSchema, } #[derive(Debug, Serialize, Deserialize, PartialEq)] @@ -28,6 +24,12 @@ pub struct ConfigFile { pub perm: String, } +#[derive(Debug, Serialize, Deserialize, PartialEq)] +pub struct ConfigJsonSchema { + // Fields that may be modified in config.json + pub whitelist: Vec, +} + pub fn read_os_config_schema(os_config_path: &Path) -> Result { read_os_config_schema_impl(os_config_path).context("Reading `os-config.json` schema failed") } @@ -35,33 +37,9 @@ pub fn read_os_config_schema(os_config_path: &Path) -> Result { fn read_os_config_schema_impl(os_config_path: &Path) -> Result { let json_data = read_file(os_config_path)?; - validate_schema_version(&json_data)?; - Ok(serde_json::from_str(&json_data)?) } -pub fn validate_schema_version(json_data: &str) -> Result<()> { - let parsed: Value = serde_json::from_str(json_data)?; - - match parsed.get("schema_version") { - Some(version_value) => match version_value.as_str() { - Some(schema_version) => { - if schema_version == SCHEMA_VERSION { - Ok(()) - } else { - bail!( - "Expected schema version {}, got {}", - SCHEMA_VERSION, - schema_version - ) - } - } - _ => bail!("`schema_version` should be a string"), - }, - _ => bail!("Missing `schema_version`"), - } -} - #[cfg(test)] mod tests { @@ -98,7 +76,9 @@ mod tests { } ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } }"#; #[test] @@ -133,7 +113,9 @@ mod tests { }, ], keys: vec!["apiKey".into(), "apiEndpoint".into(), "vpnEndpoint".into()], - schema_version: SCHEMA_VERSION.into(), + config: ConfigJsonSchema { + whitelist: vec!["logsEndpoint".into()], + }, }; assert_eq!(parsed, expected); diff --git a/tests/integration.rs b/tests/integration.rs index d83a526..d0e5022 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -77,7 +77,9 @@ fn join() { }} ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": {{ + "whitelist": ["logsEndpoint"] + }} }} "# ); @@ -99,7 +101,9 @@ fn join() { "mock-3": "MOCK-3-0123456789" } }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -257,7 +261,9 @@ fn join_flasher() { }} ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": {{ + "whitelist": ["logsEndpoint"] + }} }} "# ); @@ -272,7 +278,9 @@ fn join_flasher() { "mock-1": "MOCK-1-АБВГДЕЖЗИЙ" } }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -392,7 +400,9 @@ fn join_with_root_certificate() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#; @@ -403,7 +413,9 @@ fn join_with_root_certificate() { { "services": { }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -515,7 +527,9 @@ fn incompatible_device_types() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -608,7 +622,9 @@ fn reconfigure() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -620,7 +636,9 @@ fn reconfigure() { { "services": { }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -751,7 +769,9 @@ fn reconfigure_stored() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -763,7 +783,9 @@ fn reconfigure_stored() { { "services": { }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -935,7 +957,9 @@ fn update() { }} ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": {{ + "whitelist": ["logsEndpoint"] + }} }} "# ); @@ -961,7 +985,9 @@ fn update() { "mock-3": "MOCK-3-0123456789" } }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -1097,7 +1123,9 @@ fn update_no_config_changes() { }} ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": {{ + "whitelist": ["logsEndpoint"] + }} }} "# ); @@ -1122,7 +1150,9 @@ fn update_no_config_changes() { "mock-3": "MOCK-3-0123456789" } }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -1211,7 +1241,9 @@ fn update_with_root_certificate() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#; @@ -1222,7 +1254,9 @@ fn update_with_root_certificate() { { "services": { }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -1271,7 +1305,9 @@ fn update_unmanaged() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#; @@ -1282,7 +1318,9 @@ fn update_unmanaged() { { "services": { }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -1361,7 +1399,9 @@ fn leave() { }} ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint", "vpnPort", "registryEndpoint", "deltaEndpoint"], - "schema_version": "1.0.0" + "config": {{ + "whitelist": ["logsEndpoint"] + }} }} "# ); @@ -1378,7 +1418,9 @@ fn leave() { "mock-3": "MOCK-3-0123456789" } }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -1461,7 +1503,9 @@ fn leave_unmanaged() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -1473,7 +1517,9 @@ fn leave_unmanaged() { { "services": { }, - "schema_version": "1.0.0" + "config": { + "overrides": {} + } } "#, ); @@ -1517,7 +1563,9 @@ fn generate_api_key_unmanaged() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -1576,7 +1624,9 @@ fn generate_api_key_already_generated() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -1635,7 +1685,9 @@ fn generate_api_key_reuse() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); @@ -1717,7 +1769,9 @@ fn generate_api_key_new() { "services": [ ], "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], - "schema_version": "1.0.0" + "config": { + "whitelist": ["logsEndpoint"] + } } "#, ); From bb29ca0d1b6cde1db969cd1a781eeee2261bf4ea Mon Sep 17 00:00:00 2001 From: Christina Ying Wang Date: Wed, 13 Sep 2023 16:05:25 -0700 Subject: [PATCH 3/5] Implement config.json migration mechanism Signed-off-by: Christina Ying Wang --- src/join.rs | 27 +++-- src/main.rs | 1 + src/migrate.rs | 128 +++++++++++++++++++++++ tests/integration.rs | 237 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 387 insertions(+), 6 deletions(-) create mode 100644 src/migrate.rs diff --git a/src/join.rs b/src/join.rs index 8044232..e88fa6b 100644 --- a/src/join.rs +++ b/src/join.rs @@ -6,6 +6,7 @@ use crate::config_json::{ get_api_endpoint, get_root_certificate, merge_config_json, read_config_json, write_config_json, ConfigMap, }; +use crate::migrate::generate_config_json_migration; use crate::remote::{config_url, fetch_configuration, RemoteConfiguration}; use crate::schema::{read_os_config_schema, OsConfigSchema}; use crate::systemd; @@ -45,9 +46,12 @@ pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Resul !joining, )?; - let has_config_changes = has_config_changes(&schema, &remote_config)?; + let has_service_config_changes = has_service_config_changes(&schema, &remote_config)?; - if !has_config_changes { + let migrated_config_json = + generate_config_json_migration(&schema, &remote_config.config, &config_json.clone())?; + + if !has_service_config_changes && migrated_config_json == *config_json { info!("No configuration changes"); if !joining { @@ -66,8 +70,13 @@ pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Resul config_json, &schema, &remote_config, - has_config_changes, + has_service_config_changes, joining, + if migrated_config_json == *config_json { + None + } else { + Some(migrated_config_json) + }, ); if args.supervisor_exists { @@ -82,21 +91,27 @@ fn reconfigure_core( config_json: &ConfigMap, schema: &OsConfigSchema, remote_config: &RemoteConfiguration, - has_config_changes: bool, + has_service_config_changes: bool, joining: bool, + migrated_config_json: Option, ) -> Result<()> { if joining { write_config_json(&args.config_json_path, config_json)?; } - if has_config_changes { + if let Some(map) = migrated_config_json { + info!("Migrating config.json..."); + write_config_json(&args.config_json_path, &map)?; + } + + if has_service_config_changes { configure_services(schema, remote_config)?; } Ok(()) } -fn has_config_changes( +fn has_service_config_changes( schema: &OsConfigSchema, remote_config: &RemoteConfiguration, ) -> Result { diff --git a/src/main.rs b/src/main.rs index e2a7b61..3a83238 100644 --- a/src/main.rs +++ b/src/main.rs @@ -33,6 +33,7 @@ mod generate; mod join; mod leave; mod logger; +mod migrate; mod random; mod remote; mod schema; diff --git a/src/migrate.rs b/src/migrate.rs new file mode 100644 index 0000000..89da032 --- /dev/null +++ b/src/migrate.rs @@ -0,0 +1,128 @@ +// Config.json migration module +// +// Provides methods for migrating config.json fields based on remote directives +// from /os/vX/config. Limits migrated fields based on os-config.json schema +// whitelist. + +use crate::config_json::ConfigMap; +use crate::remote::ConfigMigrationInstructions; +use crate::schema::OsConfigSchema; +use anyhow::Result; +use std::collections::HashMap; + +pub fn generate_config_json_migration( + schema: &OsConfigSchema, + migration_config: &ConfigMigrationInstructions, + config_json: &ConfigMap, +) -> Result { + info!("Checking for config.json migrations..."); + + let mut new_config = config_json.clone(); + + handle_update_directives( + schema, + &migration_config.overrides, + config_json, + &mut new_config, + ); + + Ok(new_config) +} + +fn handle_update_directives( + schema: &OsConfigSchema, + to_update: &HashMap, + config_json: &ConfigMap, + new_config: &mut ConfigMap, +) { + for key in to_update.keys() { + if !schema.config.whitelist.contains(key) { + debug!("Key `{}` not in whitelist, skipping", key); + continue; + } + + if let Some(future) = to_update.get(key) { + if !config_json.contains_key(key) { + info!("Key `{}` not found, will insert", key); + new_config.insert(key.to_string(), future.clone()); + } else if let Some(current) = config_json.get(key) { + if current != future { + info!( + "Key `{}` found with current value `{}`, will update to `{}`", + key, current, future + ); + new_config.insert(key.to_string(), future.clone()); + } else { + debug!( + "Key `{}` found with current value `{}` equal to update value `{}`, skipping", + key, current, future + ); + } + } + } + } +} + +mod tests { + #[test] + fn test_generate_config_json_migration() { + let config_json = r#" + { + "deadbeef": 1, + "deadca1f": "2", + "deadca2f": true, + "deadca3f": "string1" + } + "# + .to_string(); + + let schema = r#" + { + "services": [ + ], + "keys": [], + "config": { + "whitelist": [ + "deadbeef", + "deadca1f", + "deadca2f", + "deadca3f", + "deadca4f" + ] + } + } + "# + .to_string(); + + let configuration = unindent::unindent( + r#" + { + "overrides": { + "deadbeef": 2, + "deadca1f": "3", + "deadca2f": false, + "deadca3f": "string0", + "deadca4f": "new_field", + "not_on_whitelist1": "not_on_whitelist" + } + } + "#, + ); + + let old_config = serde_json::from_str::(&config_json).unwrap(); + + let new_config = super::generate_config_json_migration( + &serde_json::from_str(&schema).unwrap(), + &serde_json::from_str(&configuration).unwrap(), + &old_config, + ) + .unwrap(); + + assert_eq!(new_config.get("deadbeef").unwrap(), 2); + assert_eq!(new_config.get("deadca1f").unwrap(), "3"); + assert_eq!(new_config.get("deadca2f").unwrap(), false); + assert_eq!(new_config.get("deadca3f").unwrap(), "string0"); + assert_eq!(new_config.get("deadca4f").unwrap(), "new_field"); + assert!(new_config.get("not_on_whitelist1").is_none()); + } +} diff --git a/tests/integration.rs b/tests/integration.rs index d0e5022..08c9ca4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -139,6 +139,7 @@ fn join() { r#" Fetching service configuration from http://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... Stopping balena-supervisor.service... Awaiting balena-supervisor.service to exit... Writing {tmp_dir_path}/config.json @@ -316,6 +317,7 @@ fn join_flasher() { r#" Fetching service configuration from http://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... Stopping balena-supervisor.service... Awaiting balena-supervisor.service to exit... Writing {tmp_dir_path}/config.json @@ -453,6 +455,7 @@ fn join_with_root_certificate() { r#" Fetching service configuration from https://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... No configuration changes Stopping balena-supervisor.service... Awaiting balena-supervisor.service to exit... @@ -674,6 +677,7 @@ fn reconfigure() { r#" Fetching service configuration from http://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... No configuration changes Stopping balena-supervisor.service... Awaiting balena-supervisor.service to exit... @@ -821,6 +825,7 @@ fn reconfigure_stored() { r#" Fetching service configuration from http://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... No configuration changes Stopping balena-supervisor.service... Awaiting balena-supervisor.service to exit... @@ -998,6 +1003,7 @@ fn update() { r#" Fetching service configuration from http://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... Stopping balena-supervisor.service... Awaiting balena-supervisor.service to exit... {tmp_dir_path}/not-a-service-1.conf updated @@ -1163,6 +1169,7 @@ fn update_no_config_changes() { r#" Fetching service configuration from http://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... No configuration changes "#, )); @@ -1267,6 +1274,7 @@ fn update_with_root_certificate() { r#" Fetching service configuration from https://localhost:{port}/os/v1/config... Service configuration retrieved + Checking for config.json migrations... No configuration changes "#, )); @@ -1818,6 +1826,235 @@ fn generate_api_key_new() { ); } +#[test] +#[timeout(10000)] +fn migrate_config_json() { + let port = 31014; + let tmp_dir = TempDir::new().unwrap(); + let tmp_dir_path = tmp_dir.path().to_str().unwrap().to_string(); + + let config_json = format!( + r#" + {{ + "deviceApiKey": "abcdef", + "deviceType": "intel-nuc", + "hostname": "balena", + "persistentLogging": false, + "applicationName": "aaaaaa", + "applicationId": 1234567, + "userId": 654321, + "appUpdatePollInterval": 900000, + "listenPort": 48484, + "vpnPort": 443, + "apiEndpoint": "http://{}", + "vpnEndpoint": "vpn.balenadev.io", + "registryEndpoint": "registry2.balenadev.io", + "deltaEndpoint": "https://delta.balenadev.io", + "apiKey": "12345678abcd1234efgh1234567890ab", + "version": "9.99.9+rev1", + "deadbeef": "12345678", + "mixpanelToken": "12345678abcd1234efgh1234567890ab" + }} + "#, + server_address(port) + ); + + let config_json_path = create_tmp_file(&tmp_dir, "config.json", &config_json, None); + + let schema = r#" + { + "services": [ + ], + "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], + "config": { + "whitelist": [ + "logsEndpoint", + "mixpanelToken", + "registryEndpoint", + "deltaEndpoint", + "applicationId", + "persistentLogging", + "deadbeef" + ] + } + } + "# + .to_string(); + + let os_config_path = create_tmp_file(&tmp_dir, "os-config.json", &schema, None); + + // - apiEndpoint: not on whitelist, should skip + // - logsEndpoint: value not present, should insert + // - registryEndpoint: value unchanged, should skip + // - deltaEndpoint: value changed, should update (String) + // - applicationId: value changed, should update (u64) + // - persistentLogging: value changed, should update (bool) + // - deadbeef: value changed, should update (stringified u64) + let configuration = unindent::unindent( + r#" + { + "services": {}, + "config": { + "overrides": { + "apiEndpoint": "http://api.balenadev.io", + "logsEndpoint": "https://logs.balenadev.io", + "registryEndpoint": "registry2.balenadev.io", + "deltaEndpoint": "https://delta2.balenadev.io", + "applicationId": 1234568, + "persistentLogging": true, + "deadbeef": "1234567890" + } + } + } + "#, + ); + + let mut serve = serve_config(configuration, false, port); + + // This is unfortunately necessary because the output for each line that + // begins with `Key` may vary in order. + let output_patterns = [ + format!("Fetching service configuration from http://localhost:{}/os/v1/config...\nService configuration retrieved\nChecking for config.json migrations...", port), + "Key `logsEndpoint` not found, will insert".into(), + "Key `deltaEndpoint` found with current value `\"https://delta.balenadev.io\"`, will update to `\"https://delta2.balenadev.io\"`".into(), + "Key `deadbeef` found with current value `\"12345678\"`, will update to `\"1234567890\"`".into(), + "Key `persistentLogging` found with current value `false`, will update to `true`".into(), + "Key `applicationId` found with current value `1234567`, will update to `1234568`".into(), + format!("Stopping balena-supervisor.service...\nAwaiting balena-supervisor.service to exit...\nMigrating config.json...\nWriting {}/config.json\nStarting balena-supervisor.service...", tmp_dir_path), + ]; + + let output = get_base_command() + .args(["update"]) + .timeout(Duration::from_secs(5)) + .envs(os_config_env(&os_config_path, &config_json_path)) + .assert() + .success() + .get_output() + .to_owned(); + + let string_output = String::from_utf8(output.stdout).unwrap(); + + for pattern in output_patterns.iter() { + // write string_output to local file for debugging + assert!(string_output.contains(pattern)); + } + + validate_json_file( + &config_json_path, + &format!( + r#" + {{ + "deviceApiKey": "abcdef", + "deviceType": "intel-nuc", + "hostname": "balena", + "persistentLogging": true, + "applicationName": "aaaaaa", + "applicationId": 1234568, + "userId": 654321, + "appUpdatePollInterval": 900000, + "listenPort": 48484, + "vpnPort": 443, + "apiEndpoint": "http://{}", + "vpnEndpoint": "vpn.balenadev.io", + "registryEndpoint": "registry2.balenadev.io", + "deltaEndpoint": "https://delta2.balenadev.io", + "apiKey": "12345678abcd1234efgh1234567890ab", + "version": "9.99.9+rev1", + "deadbeef": "1234567890", + "mixpanelToken": "12345678abcd1234efgh1234567890ab", + "logsEndpoint": "https://logs.balenadev.io" + }} + "#, + server_address(port) + ), + false, + ); + + serve.stop(); +} + +#[test] +#[timeout(10000)] +fn ignore_unknown_cloud_config_fields() { + let port = 31015; + let tmp_dir = TempDir::new().unwrap(); + + let config_json = format!( + r#" + {{ + "deviceApiKey": "abcdef", + "deviceType": "intel-nuc", + "hostname": "balena", + "persistentLogging": false, + "applicationName": "aaaaaa", + "applicationId": 1234567, + "userId": 654321, + "appUpdatePollInterval": 900000, + "listenPort": 48484, + "vpnPort": 443, + "apiEndpoint": "http://{}", + "vpnEndpoint": "vpn.balenadev.io", + "registryEndpoint": "registry2.balenadev.io", + "deltaEndpoint": "https://delta.balenadev.io", + "apiKey": "12345678abcd1234efgh1234567890ab", + "version": "9.99.9+rev1", + "mixpanelToken": "12345678abcd1234efgh1234567890ab" + }} + "#, + server_address(port) + ); + + let config_json_path = create_tmp_file(&tmp_dir, "config.json", &config_json, None); + + let schema = r#" + { + "services": [ + ], + "keys": ["apiKey", "apiEndpoint", "vpnEndpoint"], + "config": { + "whitelist": [] + } + } + "# + .to_string(); + + let os_config_path = create_tmp_file(&tmp_dir, "os-config.json", &schema, None); + + let configuration = unindent::unindent( + r#" + { + "services": {}, + "config": { + "overrides": {} + }, + "unknown_field": "unknown_value" + } + "#, + ); + + let mut serve = serve_config(configuration, false, port); + + let output = unindent::unindent(&format!( + r#" + Fetching service configuration from http://localhost:{port}/os/v1/config... + Service configuration retrieved + Checking for config.json migrations... + No configuration changes + "#, + )); + + get_base_command() + .args(["update"]) + .timeout(Duration::from_secs(5)) + .envs(os_config_env(&os_config_path, &config_json_path)) + .assert() + .success() + .stdout(output); + + validate_json_file(&config_json_path, &config_json, false); + + serve.stop(); +} /******************************************************************************* * os-config launch */ From 6162eb76e6b709f3165c426a8ac64951c8965690 Mon Sep 17 00:00:00 2001 From: Zahari Petkov Date: Wed, 22 Nov 2023 10:11:06 +0200 Subject: [PATCH 4/5] Modifying `config_json` in-place inside `handle_override_directives` * Renamed `generate_config_json_migration` to `migrate_config_json` * Introduced a `OverridesMap` type alias for `HashMap` * Renamed a `to_update` function argument to `overrides` to match the remote config instructions name. * Renamed `handle_update_directives` to `handle_override_directives` in a similar fashion. * `handle_override_directives` modifies `config_json` in-place instead of operating on top of a clone. It returns a `bool` now to indicate whether modifications were made. All this simplifies a bit the external logic as well. * Processing of `overrides` keys is now done in sorted order, so that logging always produces the same output when the arguments of `handle_override_directives` are the same. This helps with test code as some of it operates on top of logging output. * Inside the `migrate_config_json` integration tests I switched the output checks to use the `.stdout(output)` pattern used in other tests as it provides a diff output when something fails and is easier to navigate when error occurs. Signed-off-by: Zahari Petkov --- src/join.rs | 31 +++++-------- src/migrate.rs | 104 ++++++++++++++++++++++--------------------- src/remote.rs | 4 +- src/update.rs | 4 +- tests/integration.rs | 43 +++++++++--------- 5 files changed, 90 insertions(+), 96 deletions(-) diff --git a/src/join.rs b/src/join.rs index e88fa6b..7e23f55 100644 --- a/src/join.rs +++ b/src/join.rs @@ -6,7 +6,7 @@ use crate::config_json::{ get_api_endpoint, get_root_certificate, merge_config_json, read_config_json, write_config_json, ConfigMap, }; -use crate::migrate::generate_config_json_migration; +use crate::migrate::migrate_config_json; use crate::remote::{config_url, fetch_configuration, RemoteConfiguration}; use crate::schema::{read_os_config_schema, OsConfigSchema}; use crate::systemd; @@ -25,10 +25,10 @@ pub fn join(args: &Args) -> Result<()> { unreachable!() }; - reconfigure(args, &config_json, true) + reconfigure(args, &mut config_json, true) } -pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Result<()> { +pub fn reconfigure(args: &Args, config_json: &mut ConfigMap, joining: bool) -> Result<()> { let schema = read_os_config_schema(&args.os_config_path)?; let api_endpoint = if let Some(api_endpoint) = get_api_endpoint(config_json)? { @@ -48,10 +48,10 @@ pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Resul let has_service_config_changes = has_service_config_changes(&schema, &remote_config)?; - let migrated_config_json = - generate_config_json_migration(&schema, &remote_config.config, &config_json.clone())?; + let has_config_json_migrations = + migrate_config_json(&schema, &remote_config.config, config_json); - if !has_service_config_changes && migrated_config_json == *config_json { + if !has_service_config_changes && !has_config_json_migrations { info!("No configuration changes"); if !joining { @@ -65,18 +65,15 @@ pub fn reconfigure(args: &Args, config_json: &ConfigMap, joining: bool) -> Resul systemd::await_service_exit(SUPERVISOR_SERVICE)?; } + let should_write_config_json = joining || has_config_json_migrations; + let result = reconfigure_core( args, config_json, &schema, &remote_config, has_service_config_changes, - joining, - if migrated_config_json == *config_json { - None - } else { - Some(migrated_config_json) - }, + should_write_config_json, ); if args.supervisor_exists { @@ -92,18 +89,12 @@ fn reconfigure_core( schema: &OsConfigSchema, remote_config: &RemoteConfiguration, has_service_config_changes: bool, - joining: bool, - migrated_config_json: Option, + should_write_config_json: bool, ) -> Result<()> { - if joining { + if should_write_config_json { write_config_json(&args.config_json_path, config_json)?; } - if let Some(map) = migrated_config_json { - info!("Migrating config.json..."); - write_config_json(&args.config_json_path, &map)?; - } - if has_service_config_changes { configure_services(schema, remote_config)?; } diff --git a/src/migrate.rs b/src/migrate.rs index 89da032..87a6f41 100644 --- a/src/migrate.rs +++ b/src/migrate.rs @@ -5,62 +5,64 @@ // whitelist. use crate::config_json::ConfigMap; -use crate::remote::ConfigMigrationInstructions; +use crate::remote::{ConfigMigrationInstructions, OverridesMap}; use crate::schema::OsConfigSchema; -use anyhow::Result; -use std::collections::HashMap; -pub fn generate_config_json_migration( +pub fn migrate_config_json( schema: &OsConfigSchema, - migration_config: &ConfigMigrationInstructions, - config_json: &ConfigMap, -) -> Result { + migration: &ConfigMigrationInstructions, + config_json: &mut ConfigMap, +) -> bool { info!("Checking for config.json migrations..."); - let mut new_config = config_json.clone(); + let overridden = handle_override_directives(schema, &migration.overrides, config_json); - handle_update_directives( - schema, - &migration_config.overrides, - config_json, - &mut new_config, - ); + if overridden { + info!("Done config.json migrations"); + } - Ok(new_config) + overridden } -fn handle_update_directives( +fn handle_override_directives( schema: &OsConfigSchema, - to_update: &HashMap, - config_json: &ConfigMap, - new_config: &mut ConfigMap, -) { - for key in to_update.keys() { + overrides: &OverridesMap, + config_json: &mut ConfigMap, +) -> bool { + let mut overridden = false; + + // Sort overrides by key in order for tests to have predictable order + let mut items = overrides.iter().collect::>(); + items.sort_by_key(|pair| pair.0); + + for (key, new_value) in items { if !schema.config.whitelist.contains(key) { - debug!("Key `{}` not in whitelist, skipping", key); + info!("Key `{}` not in whitelist, skipping", key); continue; } - if let Some(future) = to_update.get(key) { - if !config_json.contains_key(key) { - info!("Key `{}` not found, will insert", key); - new_config.insert(key.to_string(), future.clone()); - } else if let Some(current) = config_json.get(key) { - if current != future { - info!( - "Key `{}` found with current value `{}`, will update to `{}`", - key, current, future - ); - new_config.insert(key.to_string(), future.clone()); - } else { - debug!( - "Key `{}` found with current value `{}` equal to update value `{}`, skipping", - key, current, future - ); - } + if let Some(existing_value) = config_json.get_mut(key) { + if new_value != existing_value { + info!( + "Key `{}` found with existing value `{}`, will override to `{}`", + key, existing_value, new_value + ); + *existing_value = new_value.clone(); + overridden = true; + } else { + debug!( + "Key `{}` found with existing value `{}` equal to override value `{}`, skipping", + key, existing_value, new_value + ); } + } else { + info!("Key `{}` not found, will insert `{}`", key, new_value); + config_json.insert(key.to_string(), new_value.clone()); + overridden = true; } } + + overridden } mod tests { @@ -109,20 +111,20 @@ mod tests { "#, ); - let old_config = serde_json::from_str::(&config_json).unwrap(); + let mut config = serde_json::from_str::(&config_json).unwrap(); - let new_config = super::generate_config_json_migration( + let has_config_json_migrations = super::migrate_config_json( &serde_json::from_str(&schema).unwrap(), &serde_json::from_str(&configuration).unwrap(), - &old_config, - ) - .unwrap(); - - assert_eq!(new_config.get("deadbeef").unwrap(), 2); - assert_eq!(new_config.get("deadca1f").unwrap(), "3"); - assert_eq!(new_config.get("deadca2f").unwrap(), false); - assert_eq!(new_config.get("deadca3f").unwrap(), "string0"); - assert_eq!(new_config.get("deadca4f").unwrap(), "new_field"); - assert!(new_config.get("not_on_whitelist1").is_none()); + &mut config, + ); + + assert_eq!(has_config_json_migrations, true); + assert_eq!(config.get("deadbeef").unwrap(), 2); + assert_eq!(config.get("deadca1f").unwrap(), "3"); + assert_eq!(config.get("deadca2f").unwrap(), false); + assert_eq!(config.get("deadca3f").unwrap(), "string0"); + assert_eq!(config.get("deadca4f").unwrap(), "new_field"); + assert!(config.get("not_on_whitelist1").is_none()); } } diff --git a/src/remote.rs b/src/remote.rs index 2c790e0..cadc9f4 100644 --- a/src/remote.rs +++ b/src/remote.rs @@ -4,6 +4,8 @@ use std::time::Duration; use anyhow::{anyhow, Context, Result}; +pub type OverridesMap = HashMap; + #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct RemoteConfiguration { pub services: HashMap>, @@ -12,7 +14,7 @@ pub struct RemoteConfiguration { #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct ConfigMigrationInstructions { - pub overrides: HashMap, + pub overrides: OverridesMap, } impl RemoteConfiguration { diff --git a/src/update.rs b/src/update.rs index 1f521f1..1e25002 100644 --- a/src/update.rs +++ b/src/update.rs @@ -6,7 +6,7 @@ use crate::config_json::read_config_json; use crate::join::reconfigure; pub fn update(args: &Args) -> Result<()> { - let config_json = read_config_json(&args.config_json_path)?; + let mut config_json = read_config_json(&args.config_json_path)?; - reconfigure(args, &config_json, false) + reconfigure(args, &mut config_json, false) } diff --git a/tests/integration.rs b/tests/integration.rs index 08c9ca4..5af4995 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1911,33 +1911,32 @@ fn migrate_config_json() { let mut serve = serve_config(configuration, false, port); - // This is unfortunately necessary because the output for each line that - // begins with `Key` may vary in order. - let output_patterns = [ - format!("Fetching service configuration from http://localhost:{}/os/v1/config...\nService configuration retrieved\nChecking for config.json migrations...", port), - "Key `logsEndpoint` not found, will insert".into(), - "Key `deltaEndpoint` found with current value `\"https://delta.balenadev.io\"`, will update to `\"https://delta2.balenadev.io\"`".into(), - "Key `deadbeef` found with current value `\"12345678\"`, will update to `\"1234567890\"`".into(), - "Key `persistentLogging` found with current value `false`, will update to `true`".into(), - "Key `applicationId` found with current value `1234567`, will update to `1234568`".into(), - format!("Stopping balena-supervisor.service...\nAwaiting balena-supervisor.service to exit...\nMigrating config.json...\nWriting {}/config.json\nStarting balena-supervisor.service...", tmp_dir_path), - ]; - - let output = get_base_command() + let output = unindent::unindent(&format!( + r#" + Fetching service configuration from http://localhost:{port}/os/v1/config... + Service configuration retrieved + Checking for config.json migrations... + Key `apiEndpoint` not in whitelist, skipping + Key `applicationId` found with existing value `1234567`, will override to `1234568` + Key `deadbeef` found with existing value `"12345678"`, will override to `"1234567890"` + Key `deltaEndpoint` found with existing value `"https://delta.balenadev.io"`, will override to `"https://delta2.balenadev.io"` + Key `logsEndpoint` not found, will insert `"https://logs.balenadev.io"` + Key `persistentLogging` found with existing value `false`, will override to `true` + Done config.json migrations + Stopping balena-supervisor.service... + Awaiting balena-supervisor.service to exit... + Writing {tmp_dir_path}/config.json + Starting balena-supervisor.service... + "# + )); + + get_base_command() .args(["update"]) .timeout(Duration::from_secs(5)) .envs(os_config_env(&os_config_path, &config_json_path)) .assert() .success() - .get_output() - .to_owned(); - - let string_output = String::from_utf8(output.stdout).unwrap(); - - for pattern in output_patterns.iter() { - // write string_output to local file for debugging - assert!(string_output.contains(pattern)); - } + .stdout(output); validate_json_file( &config_json_path, From 6e992f398ad17c6372db49b3d88e9dc74e058617 Mon Sep 17 00:00:00 2001 From: Zahari Petkov Date: Wed, 22 Nov 2023 12:15:19 +0200 Subject: [PATCH 5/5] Fix tiny clippy warning Signed-off-by: Zahari Petkov --- src/migrate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrate.rs b/src/migrate.rs index 87a6f41..8ee5042 100644 --- a/src/migrate.rs +++ b/src/migrate.rs @@ -119,7 +119,7 @@ mod tests { &mut config, ); - assert_eq!(has_config_json_migrations, true); + assert!(has_config_json_migrations); assert_eq!(config.get("deadbeef").unwrap(), 2); assert_eq!(config.get("deadca1f").unwrap(), "3"); assert_eq!(config.get("deadca2f").unwrap(), false);