Skip to content

Commit 31c60c7

Browse files
committed
🐛 fix(config): propagate reconcile errors
`build_state` discarded repository reconciliation errors, so the server could expose a configured route without its persisted identity. Propagate the storage error before `build_state` publishes application state. Nodes without write access keep skipping repository reconciliation.
1 parent 60698d6 commit 31c60c7

4 files changed

Lines changed: 116 additions & 16 deletions

File tree

crates/peryx/src/config/repository_migration.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::time::{SystemTime, UNIX_EPOCH};
22

33
use peryx_identity::UserId;
4-
use peryx_storage::meta::{DesiredRepository, MetaStore};
4+
use peryx_storage::meta::{DesiredRepository, MetaStore, ReconcileRepositoryError};
55

66
use super::IndexConfig;
77

@@ -27,17 +27,19 @@ fn desired(config: &IndexConfig) -> DesiredRepository {
2727
}
2828
}
2929

30-
/// Give every configured index a persisted repository record.
30+
/// Persist repository identities before the server exposes configured routes.
3131
///
32-
/// Each index reconciles by route: a new route mints a record, an existing route reuses its id, so a
33-
/// restart adds nothing and a later rename never re-homes a reference. Unchanged configuration bumps
34-
/// no version. A route the store cannot hold as a repository, an over-long one for example, leaves
35-
/// the batch unwritten and logs rather than failing an otherwise healthy boot.
36-
pub fn reconcile_configured_repositories(meta: &MetaStore, configs: &[IndexConfig]) {
32+
/// Route matching keeps IDs stable and advances versions only when a stored definition changes.
33+
///
34+
/// # Errors
35+
/// Returns an error when validation or persistence prevents the atomic batch.
36+
pub fn reconcile_configured_repositories(
37+
meta: &MetaStore,
38+
configs: &[IndexConfig],
39+
) -> Result<(), ReconcileRepositoryError> {
3740
let desired: Vec<DesiredRepository> = configs.iter().map(desired).collect();
38-
if let Err(error) = meta.reconcile_repositories(&desired, &system_actor(), unix_now()) {
39-
tracing::warn!(%error, "could not assign stable ids to configured repositories");
40-
}
41+
meta.reconcile_repositories(&desired, &system_actor(), unix_now())
42+
.map(drop)
4143
}
4244

4345
#[cfg(test)]

crates/peryx/src/server.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ fn build_state_with_active_backend_and_plugins(
223223
}
224224
attach_policy_decision_recorders(&meta, &mut indexes, read_only)?;
225225
if !read_only {
226-
crate::config::reconcile_configured_repositories(&meta, &configs);
226+
persist_configured_repositories(&meta, &configs)?;
227227
}
228228
let ecosystem_settings = build_index_settings_with_plugins(&configs, plugins)?;
229229
let webhooks = build_webhooks(&configs, plugins)?;
@@ -257,6 +257,19 @@ fn build_state_with_active_backend_and_plugins(
257257
Ok(Arc::new(state))
258258
}
259259

260+
fn persist_configured_repositories(meta: &MetaStore, configs: &[IndexConfig]) -> anyhow::Result<()> {
261+
crate::config::reconcile_configured_repositories(meta, configs).with_context(|| {
262+
format!(
263+
"persist configured repositories [{}]",
264+
configs
265+
.iter()
266+
.map(|config| config.route.as_str())
267+
.collect::<Vec<_>>()
268+
.join(", ")
269+
)
270+
})
271+
}
272+
260273
fn resolve_signing_key(config: &Config) -> anyhow::Result<Option<String>> {
261274
const MIN_BYTES: usize = 32;
262275
let Some(source) = &config.auth.signing_key else {

crates/peryx/tests/unit/config/repository_migration/tests.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ fn test_reconcile_assigns_stable_ids_idempotently_across_boots() {
2424
let dir = tempfile::tempdir().unwrap();
2525
let store = MetaStore::open(dir.path().join("peryx.redb")).unwrap();
2626

27-
reconcile_configured_repositories(&store, &config.indexes);
27+
reconcile_configured_repositories(&store, &config.indexes).unwrap();
2828
let first_boot = routes_to_id_and_version(&store);
2929

3030
assert_eq!(first_boot.len(), config.indexes.len());
3131
assert!(first_boot.values().all(|(id, version)| !id.is_empty() && *version == 1));
3232

33-
reconcile_configured_repositories(&store, &config.indexes);
33+
reconcile_configured_repositories(&store, &config.indexes).unwrap();
3434

3535
assert_eq!(routes_to_id_and_version(&store), first_boot);
3636
}
@@ -45,10 +45,10 @@ fn test_reconcile_renames_default_virtual_indexes_without_changing_ids() {
4545
.name = "root/pypi".to_owned();
4646
let dir = tempfile::tempdir().unwrap();
4747
let store = MetaStore::open(dir.path().join("peryx.redb")).unwrap();
48-
reconcile_configured_repositories(&store, &old.indexes);
48+
reconcile_configured_repositories(&store, &old.indexes).unwrap();
4949
let previous = store.repository_by_route("root/pypi").unwrap().unwrap();
5050

51-
reconcile_configured_repositories(&store, &Config::default().indexes);
51+
reconcile_configured_repositories(&store, &Config::default().indexes).unwrap();
5252
let current = store.repository_by_route("root/pypi").unwrap().unwrap();
5353

5454
assert_eq!(current.id, previous.id);
@@ -63,7 +63,8 @@ fn test_reconcile_writes_nothing_when_a_route_cannot_be_a_repository() {
6363
let dir = tempfile::tempdir().unwrap();
6464
let store = MetaStore::open(dir.path().join("peryx.redb")).unwrap();
6565

66-
reconcile_configured_repositories(&store, &config.indexes);
66+
let error = reconcile_configured_repositories(&store, &config.indexes).unwrap_err();
6767

68+
assert_eq!(error.to_string(), "repository route exceeds 512 bytes");
6869
assert!(routes_to_id_and_version(&store).is_empty());
6970
}

crates/peryx/tests/unit/tests/server_tests.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,90 @@ fn test_build_state_opens_configured_data_dir() {
401401
assert!(dir.path().join("peryx.redb").exists());
402402
}
403403

404+
#[test]
405+
fn test_build_state_reports_configured_repository_persistence_failure() {
406+
let dir = tempfile::tempdir().unwrap();
407+
let config = config_with_corrupt_repository(&dir);
408+
409+
let error = build_state(&config)
410+
.err()
411+
.expect("expected repository persistence error");
412+
413+
assert_eq!(
414+
error.to_string(),
415+
format!("persist configured repositories [{}]", config.indexes[0].route)
416+
);
417+
}
418+
419+
#[test]
420+
fn test_build_state_read_only_skips_repository_reconciliation() {
421+
let dir = tempfile::tempdir().unwrap();
422+
let mut config = config_with_corrupt_repository(&dir);
423+
config.read_only = true;
424+
425+
let state = build_state(&config).unwrap();
426+
427+
assert_eq!(state.serving.indexes[0].route, config.indexes[0].route);
428+
}
429+
430+
#[test]
431+
fn test_build_state_preserves_configured_repository_identity() {
432+
let dir = tempfile::tempdir().unwrap();
433+
let config = Config {
434+
data_dir: dir.path().to_path_buf(),
435+
..neutral_config()
436+
};
437+
let first = build_state(&config).unwrap();
438+
let first_repositories = routes_to_id_and_version(&first.serving.meta);
439+
drop(first);
440+
441+
let second = build_state(&config).unwrap();
442+
443+
assert_eq!(
444+
(routes_to_id_and_version(&second.serving.meta), first_repositories.len()),
445+
(first_repositories, config.indexes.len())
446+
);
447+
}
448+
449+
fn config_with_corrupt_repository(dir: &tempfile::TempDir) -> Config {
450+
let mut config = Config {
451+
data_dir: dir.path().to_path_buf(),
452+
..neutral_config()
453+
};
454+
config.indexes.truncate(1);
455+
let state = build_state(&config).unwrap();
456+
let repository = state
457+
.serving
458+
.meta
459+
.repository_by_route(&config.indexes[0].route)
460+
.unwrap()
461+
.unwrap();
462+
drop(state);
463+
let database = redb::Database::open(dir.path().join("peryx.redb")).unwrap();
464+
let transaction = database.begin_write().unwrap();
465+
{
466+
let mut repositories = transaction
467+
.open_table(redb::TableDefinition::<&str, &[u8]>::new("repository"))
468+
.unwrap();
469+
repositories.insert(repository.id.as_str(), b"{".as_slice()).unwrap();
470+
}
471+
transaction.commit().unwrap();
472+
config
473+
}
474+
475+
fn routes_to_id_and_version(store: &MetaStore) -> std::collections::BTreeMap<String, (String, u64)> {
476+
store
477+
.list_repositories(&peryx_storage::meta::RepositoryQuery {
478+
limit: 100,
479+
..peryx_storage::meta::RepositoryQuery::default()
480+
})
481+
.unwrap()
482+
.repositories
483+
.into_iter()
484+
.map(|record| (record.route, (record.id.as_str().to_owned(), record.version)))
485+
.collect()
486+
}
487+
404488
#[test]
405489
fn test_build_state_repairs_abandoned_quota_before_admission() {
406490
let dir = tempfile::tempdir().unwrap();

0 commit comments

Comments
 (0)