From 446cb1cc96665974a5b488df850c6cc82f79a59b Mon Sep 17 00:00:00 2001 From: Christina Ying Wang Date: Mon, 11 Sep 2023 15:11:28 -0700 Subject: [PATCH] Remove ability to delete config.json fields See improvement linked to this PR for more info. Also add test case to ensure os-config behaves normally in the presence of unknown cloud configuration fields. Signed-off-by: Christina Ying Wang --- src/migrate.rs | 71 +++-------------------------- tests/integration.rs | 105 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 95 insertions(+), 81 deletions(-) diff --git a/src/migrate.rs b/src/migrate.rs index 2d2f76f..66ba8a5 100644 --- a/src/migrate.rs +++ b/src/migrate.rs @@ -19,37 +19,14 @@ pub fn generate_config_json_migration( let mut new_config = config_json.clone(); - let (to_update, to_delete) = separate_migration_directives(migration_config); - - handle_update_directives(schema, to_update, &to_delete, config_json, &mut new_config); - - handle_delete_directives(schema, &to_delete, config_json, &mut new_config); + handle_update_directives(schema, &migration_config.overrides, config_json, &mut new_config); Ok(new_config) } -// Separate the migration_config.overrides directives into update -// or delete directives. If a value is null, it is considered a delete directive. -fn separate_migration_directives( - migration_config: &ConfigMigrationInstructions, -) -> (HashMap, Vec) { - let mut to_update = migration_config.overrides.clone(); - let mut to_delete = Vec::new(); - - for (key, value) in migration_config.overrides.iter() { - if value.is_null() { - to_update.remove(key); - to_delete.push(key.to_string()); - } - } - - (to_update, to_delete) -} - fn handle_update_directives( schema: &OsConfigSchema, - to_update: HashMap, - to_delete: &[String], + to_update: &HashMap, config_json: &ConfigMap, new_config: &mut ConfigMap, ) { @@ -59,11 +36,6 @@ fn handle_update_directives( continue; } - // If also deleting, delete takes precedence - if to_delete.contains(key) { - continue; - } - if let Some(future) = to_update.get(key) { if !config_json.contains_key(key) { info!("Key `{}` not found, will insert", key); @@ -86,27 +58,6 @@ fn handle_update_directives( } } -fn handle_delete_directives( - schema: &OsConfigSchema, - to_delete: &[String], - config_json: &ConfigMap, - new_config: &mut ConfigMap, -) { - for key in to_delete.iter() { - if !schema.config.whitelist.contains(key) { - debug!("Key `{}` not in whitelist, skipping", key); - continue; - } - - if config_json.contains_key(key) { - info!("Key `{}` found, will delete", key); - new_config.remove(key); - } else { - debug!("Key `{}` not found for deletion, skipping", key); - } - } -} - mod tests { #[test] fn test_generate_config_json_migration() { @@ -115,9 +66,7 @@ mod tests { "deadbeef": 1, "deadca1f": "2", "deadca2f": true, - "deadca3f": "string1", - "deadca5f": "to_delete", - "on_both_lists": "on_both_lists" + "deadca3f": "string1" } "# .to_string(); @@ -132,9 +81,7 @@ mod tests { "deadca1f", "deadca2f", "deadca3f", - "deadca4f", - "deadca5f", - "on_both_lists" + "deadca4f" ], "leave": [] } @@ -151,11 +98,7 @@ mod tests { "deadca2f": false, "deadca3f": "string0", "deadca4f": "new_field", - "not_on_whitelist1": "not_on_whitelist", - "on_both_lists": "on_both_lists2", - "deadca5f": null, - "not_on_whitelist2": null, - "on_both_lists": null + "not_on_whitelist1": "not_on_whitelist" } } "#, @@ -175,9 +118,5 @@ mod tests { 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("deadca5f").is_none()); - assert!(new_config.get("not_on_whitelist1").is_none()); - assert!(new_config.get("not_on_whitelist2").is_none()); - assert!(new_config.get("on_both_lists").is_none()); } } diff --git a/tests/integration.rs b/tests/integration.rs index b6a55ff..5584c27 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1873,8 +1873,7 @@ fn migrate_config_json() { "deltaEndpoint", "applicationId", "persistentLogging", - "deadbeef", - "vpnEndpoint" + "deadbeef" ], "leave": ["apiKey", "apiEndpoint", "vpnEndpoint"] } @@ -1884,7 +1883,6 @@ fn migrate_config_json() { let os_config_path = create_tmp_file(&tmp_dir, "os-config.json", &schema, None); - // Update: // - apiEndpoint: not on whitelist, should skip // - logsEndpoint: value not present, should insert // - registryEndpoint: value unchanged, should skip @@ -1892,11 +1890,6 @@ fn migrate_config_json() { // - applicationId: value changed, should update (u64) // - persistentLogging: value changed, should update (bool) // - deadbeef: value changed, should update (stringified u64) - // - vpnEndpoint: on whitelist & in update, should delete and not update - // Delete: - // - apiKey: not on whitelist, is null, should skip - // - mixpanelToken: on whitelist, is null, should delete - // - vpnEndpoint: on whitelist, is null AND has value, delete takes precedence let configuration = unindent::unindent( r#" { @@ -1909,11 +1902,7 @@ fn migrate_config_json() { "deltaEndpoint": "https://delta2.balenadev.io", "applicationId": 1234568, "persistentLogging": true, - "deadbeef": "1234567890", - "vpnEndpoint": "vpn2.balenadev.io", - "apiKey": null, - "mixpanelToken": null, - "vpnEndpoint": null + "deadbeef": "1234567890" } } } @@ -1931,8 +1920,6 @@ fn migrate_config_json() { "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(), - "Key `mixpanelToken` found, will delete".into(), - "Key `vpnEndpoint` found, will delete".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), ]; @@ -1968,11 +1955,13 @@ fn migrate_config_json() { "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" }} "#, @@ -1984,6 +1973,92 @@ fn migrate_config_json() { 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": [ + ], + "config": { + "whitelist": [], + "leave": ["apiKey", "apiEndpoint", "vpnEndpoint"] + } + } + "# + .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/v2/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 */