Skip to content

Commit

Permalink
Remove ability to delete config.json fields
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@cywang117
cywang117 committed Sep 11, 2023
1 parent c2f39af commit 446cb1c
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 81 deletions.
71 changes: 5 additions & 66 deletions src/migrate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, serde_json::Value>, Vec<String>) {
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<String, serde_json::Value>,
to_delete: &[String],
to_update: &HashMap<String, serde_json::Value>,
config_json: &ConfigMap,
new_config: &mut ConfigMap,
) {
Expand All @@ -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);
Expand All @@ -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() {
Expand All @@ -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();
Expand All @@ -132,9 +81,7 @@ mod tests {
"deadca1f",
"deadca2f",
"deadca3f",
"deadca4f",
"deadca5f",
"on_both_lists"
"deadca4f"
],
"leave": []
}
Expand All @@ -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"
}
}
"#,
Expand All @@ -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());
}
}
105 changes: 90 additions & 15 deletions tests/integration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1873,8 +1873,7 @@ fn migrate_config_json() {
"deltaEndpoint",
"applicationId",
"persistentLogging",
"deadbeef",
"vpnEndpoint"
"deadbeef"
],
"leave": ["apiKey", "apiEndpoint", "vpnEndpoint"]
}
Expand All @@ -1884,19 +1883,13 @@ 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
// - 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)
// - 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#"
{
Expand All @@ -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"
}
}
}
Expand All @@ -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),
];

Expand Down Expand Up @@ -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"
}}
"#,
Expand All @@ -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
*/
Expand Down

0 comments on commit 446cb1c

Please sign in to comment.