fix: preserve configuration and credentials during v7-to-v8 upgrades
This commit is contained in:
1 parent
670ab9ff95
commit
82089faaf1
5 files changed
+112
-11
No files matched your search
@@ -1223,11 +1223,26 @@ pub(crate) fn set_core_yaml_nested_value(
|
||||
}
|
||||
|
||||
pub(crate) fn core_config_uses_v8(document: &serde_norway::Value) -> bool {
|
||||
document
|
||||
.as_mapping()
|
||||
.and_then(|root| yaml_mapping_value(root, "config-version"))
|
||||
let Some(root) = document.as_mapping() else {
|
||||
return false;
|
||||
};
|
||||
if yaml_mapping_value(root, "config-version")
|
||||
.and_then(serde_norway::Value::as_u64)
|
||||
.is_some_and(|version| version >= 8)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
[
|
||||
"server", "management", "access", "credentials", "requests", "oauth",
|
||||
"multimedia", "observability",
|
||||
]
|
||||
.iter()
|
||||
.any(|section| yaml_mapping_value(root, section).is_some_and(serde_norway::Value::is_mapping))
|
||||
|| ["retry", "cooldown"].iter().any(|section| {
|
||||
nested_yaml_value(root, &["routing", section])
|
||||
.is_some_and(serde_norway::Value::is_mapping)
|
||||
})
|
||||
|| yaml_mapping_value(root, "api-keys").is_some_and(serde_norway::Value::is_mapping)
|
||||
}
|
||||
|
||||
pub(crate) fn set_core_yaml_path_value(
|
||||
|
||||
@@ -1398,11 +1398,10 @@ pub(crate) fn core_start_stdio(log_path: &Path) -> io::Result<(Stdio, Stdio)> {
|
||||
}
|
||||
|
||||
let mut header_file = File::options()
|
||||
.write(true)
|
||||
.append(true)
|
||||
.create(true)
|
||||
.truncate(true)
|
||||
.open(log_path)?;
|
||||
writeln!(header_file, "===== CPA kernel startup {} =====", unix_now())?;
|
||||
writeln!(header_file, "\n===== CPA kernel startup {} =====", unix_now())?;
|
||||
drop(header_file);
|
||||
|
||||
let stdout_file = File::options().append(true).open(log_path)?;
|
||||
@@ -1958,10 +1957,7 @@ pub(crate) fn migrate_core_config_for_update(
|
||||
}
|
||||
let template_document = serde_norway::from_str::<serde_norway::Value>(&template)
|
||||
.map_err(|error| format!("Failed to parse new kernel configuration template: {error}"))?;
|
||||
let migrated = if core_config_uses_v8(&template_document) && !core_config_uses_v8(&old_document) {
|
||||
// v8 accepts legacy-only files and migrates them atomically on the first
|
||||
// successful v8 Management API write. Mixing both layouts here would let
|
||||
// v8 template defaults override the user's legacy values.
|
||||
let migrated = if core_config_uses_v8(&template_document) {
|
||||
old_config
|
||||
} else {
|
||||
merge_core_config_fields(&template, Some(&old_config))?
|
||||
|
||||
@@ -651,6 +651,19 @@ fn portable_update_replacement_preserves_user_data_and_can_roll_back() {
|
||||
fs::write(app_dir.join(GUI_CONFIG_FILE), b"user config").unwrap();
|
||||
fs::create_dir_all(app_dir.join(OAUTH_DIR_NAME)).unwrap();
|
||||
fs::write(app_dir.join(OAUTH_DIR_NAME).join("account.json"), b"oauth").unwrap();
|
||||
let preserved_paths = [
|
||||
"cpa-core/config.yaml",
|
||||
"cpa-core/oauth/account.json",
|
||||
"cpa-core/custom-auth/account.json",
|
||||
"cpa-core/logs/main.log",
|
||||
"cpa-core/logs/archive/requests.log",
|
||||
"oauth/logs/oauth.log",
|
||||
];
|
||||
for path in preserved_paths {
|
||||
let path = app_dir.join(path);
|
||||
fs::create_dir_all(path.parent().unwrap()).unwrap();
|
||||
fs::write(path, b"existing user data").unwrap();
|
||||
}
|
||||
fs::write(staging.join(PORTABLE_APP_BINARY), b"new exe").unwrap();
|
||||
fs::write(staging.join(PORTABLE_APP_MANIFEST_FILE), b"new manifest").unwrap();
|
||||
fs::write(staging.join(CORE_VERSION_FILE), b"7.2.109").unwrap();
|
||||
@@ -685,6 +698,9 @@ fn portable_update_replacement_preserves_user_data_and_can_roll_back() {
|
||||
};
|
||||
|
||||
replace_portable_update_files(&descriptor).unwrap();
|
||||
for path in preserved_paths {
|
||||
assert_eq!(fs::read(app_dir.join(path)).unwrap(), b"existing user data");
|
||||
}
|
||||
assert_eq!(fs::read(&descriptor.current_exe).unwrap(), b"new exe");
|
||||
assert_eq!(
|
||||
fs::read(&descriptor.current_manifest).unwrap(),
|
||||
@@ -708,6 +724,9 @@ fn portable_update_replacement_preserves_user_data_and_can_roll_back() {
|
||||
);
|
||||
|
||||
restore_portable_update_backup(&descriptor).unwrap();
|
||||
for path in preserved_paths {
|
||||
assert_eq!(fs::read(app_dir.join(path)).unwrap(), b"existing user data");
|
||||
}
|
||||
assert_eq!(fs::read(&descriptor.current_exe).unwrap(), b"old exe");
|
||||
assert_eq!(
|
||||
fs::read(&descriptor.current_manifest).unwrap(),
|
||||
|
||||
@@ -1447,6 +1447,26 @@ fn v8_core_config_reads_and_writes_canonical_nested_fields() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v8_startup_without_version_marker_preserves_upstream_credentials() {
|
||||
let input = "server: {port: 9527}\noauth: {auth-dir: ../oauth}\naccess: {api-keys: [old-client-key]}\napi-keys:\n codex:\n - name: custom-provider\n base-url: https://example.invalid/v1\n keys: [{api-key: upstream-secret}]\n";
|
||||
let original = serde_norway::from_str::<serde_norway::Value>(input).unwrap();
|
||||
let config = GuiConfigFile {
|
||||
api_keys: vec![GuiApiKeyEntry { key: "new-client-key".into(), remark: String::new() }],
|
||||
management_secret_key: "management-secret".into(),
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
for keys in [config.api_keys.clone(), Vec::new()] {
|
||||
let config = GuiConfigFile { api_keys: keys, ..config.clone() };
|
||||
let updated = apply_gui_managed_settings(input, &config).unwrap();
|
||||
let updated = serde_norway::from_str::<serde_norway::Value>(&updated).unwrap();
|
||||
assert_eq!(updated["api-keys"], original["api-keys"], "upstream credentials must not be replaced by client keys");
|
||||
let effective = core_config_settings_from_value(&updated).unwrap();
|
||||
assert_eq!(effective.api_keys, gui_api_key_values(&config.api_keys));
|
||||
assert_eq!(effective.auth_dir, "../oauth");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn core_config_reads_proxy_and_session_affinity_fields() {
|
||||
let canonical = serde_norway::from_str::<serde_norway::Value>(
|
||||
|
||||
@@ -611,6 +611,53 @@ fn replacing_v7_with_v8_preserves_the_legacy_config_byte_for_byte() {
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v8_core_updates_preserve_partially_migrated_config_and_credentials() {
|
||||
let root = agent_test_home("core-v8-partial-migration");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let staging_dir = root.join("cpa-core.staging");
|
||||
fs::create_dir_all(&install_dir).unwrap();
|
||||
fs::create_dir_all(root.join("oauth")).unwrap();
|
||||
let credential_path = root.join("oauth/account.json");
|
||||
fs::write(&credential_path, b"existing-credential").unwrap();
|
||||
let legacy = "# User configuration\nhost: 127.0.0.1\nport: 9527\nauth-dir: ../oauth\napi-keys: [client-key]\nremote-management: {secret-key: user-secret}\nproxy-url: direct\nrequest-retry: 9\ncodex-api-key: [{api-key: upstream-key, base-url: 'https://example.invalid/v1'}]\noauth-model-alias: {codex: [{name: model, alias: custom-model}]}\n";
|
||||
let template = "server: {host: '', port: 8317}\noauth: {auth-dir: '~/.cli-proxy-api'}\naccess: {api-keys: [your-api-key-1]}\nmanagement: {secret-key: ''}\nrequests: {proxy-url: ''}\nrouting: {retry: {request-retry: 3}}\n";
|
||||
|
||||
for prefix in ["", "config-version: 8\n", "server: {host: 127.0.0.1}\n", "config-version: 8\nserver: {host: 127.0.0.1}\n"] {
|
||||
let original = format!("{prefix}{legacy}");
|
||||
fs::write(install_dir.join(CORE_CONFIG_FILE), &original).unwrap();
|
||||
for template_prefix in ["config-version: 8\n", ""] {
|
||||
fs::create_dir_all(&staging_dir).unwrap();
|
||||
fs::write(staging_dir.join(CORE_EXAMPLE_CONFIG_FILE), format!("{template_prefix}{template}")).unwrap();
|
||||
migrate_core_config_for_update(&install_dir, &staging_dir).unwrap();
|
||||
overlay_install_dir(&install_dir, &staging_dir).unwrap();
|
||||
|
||||
let migrated = fs::read_to_string(install_dir.join(CORE_CONFIG_FILE)).unwrap();
|
||||
let document = serde_norway::from_str::<serde_norway::Value>(&migrated).unwrap();
|
||||
let settings = core_config_settings_from_value(&document).unwrap();
|
||||
assert_eq!(settings.auth_dir, "../oauth", "credential directory changed during update");
|
||||
assert_eq!(settings.port, 9527);
|
||||
assert_eq!(settings.api_keys, vec!["client-key"]);
|
||||
assert_eq!(settings.management_secret_key.as_deref(), Some("user-secret"));
|
||||
assert_eq!(settings.proxy_url, "direct");
|
||||
assert_eq!(settings.request_retry, 9);
|
||||
assert_eq!(migrated, original, "update must preserve every existing field and comment");
|
||||
|
||||
let mut gui = GuiConfigFile::default();
|
||||
apply_core_settings_to_gui_config(&mut gui, &settings);
|
||||
gui.proxy_url = settings.proxy_url.clone();
|
||||
let startup = merge_core_config_yaml(template, Some(&migrated), &gui).unwrap();
|
||||
let startup = serde_norway::from_str::<serde_norway::Value>(&startup).unwrap();
|
||||
let effective = core_config_settings_from_value(&startup).unwrap();
|
||||
assert_eq!(auth_dir_path_for_core(&effective.auth_dir, &install_dir).unwrap(), root.join("oauth"));
|
||||
assert_eq!(startup["codex-api-key"], document["codex-api-key"]);
|
||||
assert_eq!(startup["oauth-model-alias"], document["oauth-model-alias"]);
|
||||
assert_eq!(fs::read(&credential_path).unwrap(), b"existing-credential");
|
||||
}
|
||||
}
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replacing_a_core_rejects_invalid_config_without_overwriting_files() {
|
||||
let root = agent_test_home("core-config-migrate-invalid");
|
||||
@@ -980,6 +1027,10 @@ fn core_start_log_captures_stdout_and_stderr() {
|
||||
assert!(output.contains("===== CPA kernel startup"));
|
||||
assert!(output.contains("core stdout marker"));
|
||||
assert!(output.contains("core stderr marker"));
|
||||
assert!(!output.contains("stale startup output"));
|
||||
assert!(output.starts_with("stale startup output\n"));
|
||||
drop(core_start_stdio(&log_path).unwrap());
|
||||
let restarted_output = fs::read_to_string(&log_path).unwrap();
|
||||
assert!(restarted_output.starts_with(&output));
|
||||
assert_eq!(restarted_output.matches("===== CPA kernel startup").count(), 2);
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
Reference in new issue
Block a user