various: fix closure narinfo cache#129
Conversation
| Some(format!("nar/{nar_hash}.nar?hash={output_hash}")) | ||
| } | ||
|
|
||
| async fn sign_narinfo_fingerprint( |
There was a problem hiding this comment.
This seems to duplicate sign_fingerprint. Both implement identical Ed25519 signing (fingerprint 1;path;narhash;size;refs) and any potential format changes would have to be applied in two places. Please move shared logic into circus_common or circus_nix.
| project_id: Option<Uuid>, | ||
| ) { | ||
| let key_file = match &signing_config.key_file { | ||
| Some(kf) if signing_config.enabled && kf.exists() => kf, |
There was a problem hiding this comment.
This reads the signing key from disk once per closure path. Read once at the top of persist_closure_narinfos_with_nix and pass the parsed key down.
| publish_succeeded_output_closure( | ||
| &pool, | ||
| &worker_pool, | ||
| build.id, | ||
| output_path, |
There was a problem hiding this comment.
We seem to be passing existing.build_output_path (singular) to publish_succeeded_output_closure. For derivations with multiple outputs, only the first output gets its closure cached.
| if has_signed_persisted_narinfo(pool, &store_path, scope.project_id()).await? | ||
| { | ||
| return Ok(true); | ||
| } |
There was a problem hiding this comment.
If a .drv path ended up in narinfo_cache with a signature, it would bypass the explicit drv check and the CA branch. You can move the check after the drv gate.
NotAShelf
changed the title
| .filter(|hash| !hash.is_empty()) | ||
| } | ||
|
|
||
| fn canonical_nix_sha256_hash(text: &str) -> Option<String> { |
There was a problem hiding this comment.
canonical_nix_sha256_hash returns None silently for unrecognized formats, which means entries are silently dropped from the closure. This is not breaking but for the sake of being explicit we should log with tracing::warn! here.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_persist_closure_narinfos_records_dependency_closure() { |
There was a problem hiding this comment.
Leaves jobset/evaluation/build rows in the test database...
| output_path: &str, | ||
| ) { | ||
| let project_id = match repo::builds::get(pool, build_id).await { | ||
| Ok(build) => { |
There was a problem hiding this comment.
You should log when get_project_for_build returns None instead of only logging the Err branch.
NotAShelf
changed the title
2 participants
Publish signed narinfos for successful build output closures so Circus can serve every recorded dependency from its cache.