diff options
author | Marijan Petričević <marijan.petricevic94@gmail.com> | 2024-10-10T14·11-0500 |
---|---|---|
committer | Marijan Petričević <marijan.petricevic94@gmail.com> | 2024-10-11T17·18+0000 |
commit | e8040ec61f2119ece2d396704576973f704607f3 (patch) | |
tree | 94caa469edb4b6c5534eb19a9683d786f9b7e5bf /tvix/store/src/pathinfoservice | |
parent | b4ccaac7ad135249eb0b1866acf4c8e68fd5bdb9 (diff) |
refactor(tvix/store): use strictly typed PathInfo struct r/8787
This switches the PathInfoService trait from using the proto-derived PathInfo struct to a more restrictive struct, and updates all implementations to use it. It removes a lot of the previous conversion and checks, as invalid states became nonrepresentable, and validations are expressed on the type level. PathInfoService implementations consuming protobuf need to convert and do the verification internally, and can only return the strongly typed variant. The nix_compat::narinfo::NarInfo conversions for the proto PathInfo are removed, we only keep a version showing a NarInfo representation for the strong struct. Converting back to a PathInfo requires the root node now, but is otherwise trivial, so left to the users. Co-Authored-By: Florian Klink <flokli@flokli.de> Change-Id: I6fdfdb44063efebb44a8f0097b6b81a828717e03 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12588 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
Diffstat (limited to 'tvix/store/src/pathinfoservice')
-rw-r--r-- | tvix/store/src/pathinfoservice/bigtable.rs | 30 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/combinators.rs | 23 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/fs/mod.rs | 32 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/grpc.rs | 40 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/lru.rs | 52 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/memory.rs | 22 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/mod.rs | 2 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/nix_http.rs | 34 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/redb.rs | 45 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/signing_wrapper.rs | 60 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/tests/mod.rs | 41 |
11 files changed, 146 insertions, 235 deletions
diff --git a/tvix/store/src/pathinfoservice/bigtable.rs b/tvix/store/src/pathinfoservice/bigtable.rs index 15128986ff56..3d8db8e5044a 100644 --- a/tvix/store/src/pathinfoservice/bigtable.rs +++ b/tvix/store/src/pathinfoservice/bigtable.rs @@ -1,6 +1,5 @@ -use super::PathInfoService; +use super::{PathInfo, PathInfoService}; use crate::proto; -use crate::proto::PathInfo; use async_stream::try_stream; use bigtable_rs::{bigtable, google::bigtable::v2 as bigtable_v2}; use bytes::Bytes; @@ -232,14 +231,13 @@ impl PathInfoService for BigtablePathInfoService { } // Try to parse the value into a PathInfo message - let path_info = proto::PathInfo::decode(Bytes::from(cell.value)) + let path_info_proto = proto::PathInfo::decode(Bytes::from(cell.value)) .map_err(|e| Error::StorageError(format!("unable to decode pathinfo proto: {}", e)))?; - let store_path = path_info - .validate() - .map_err(|e| Error::StorageError(format!("invalid PathInfo: {}", e)))?; + let path_info = PathInfo::try_from(path_info_proto) + .map_err(|e| Error::StorageError(format!("Invalid path info: {e}")))?; - if store_path.digest() != &digest { + if path_info.store_path.digest() != &digest { return Err(Error::StorageError("PathInfo has unexpected digest".into())); } @@ -248,14 +246,10 @@ impl PathInfoService for BigtablePathInfoService { #[instrument(level = "trace", skip_all, fields(path_info.root_node = ?path_info.node))] async fn put(&self, path_info: PathInfo) -> Result<PathInfo, Error> { - let store_path = path_info - .validate() - .map_err(|e| Error::InvalidRequest(format!("pathinfo failed validation: {}", e)))?; - let mut client = self.client.clone(); - let path_info_key = derive_pathinfo_key(store_path.digest()); + let path_info_key = derive_pathinfo_key(path_info.store_path.digest()); - let data = path_info.encode_to_vec(); + let data = proto::PathInfo::from(path_info.clone()).encode_to_vec(); if data.len() as u64 > CELL_SIZE_LIMIT { return Err(Error::StorageError( "PathInfo exceeds cell limit on Bigtable".into(), @@ -340,16 +334,12 @@ impl PathInfoService for BigtablePathInfoService { } // Try to parse the value into a PathInfo message. - let path_info = proto::PathInfo::decode(Bytes::from(cell.value)) + let path_info_proto = proto::PathInfo::decode(Bytes::from(cell.value)) .map_err(|e| Error::StorageError(format!("unable to decode pathinfo proto: {}", e)))?; - // Validate the containing PathInfo, ensure its StorePath digest - // matches row key. - let store_path = path_info - .validate() - .map_err(|e| Error::StorageError(format!("invalid PathInfo: {}", e)))?; + let path_info = PathInfo::try_from(path_info_proto).map_err(|e| Error::StorageError(format!("Invalid path info: {e}")))?; - let exp_path_info_key = derive_pathinfo_key(store_path.digest()); + let exp_path_info_key = derive_pathinfo_key(path_info.store_path.digest()); if exp_path_info_key.as_bytes() != row_key.as_slice() { Err(Error::StorageError("PathInfo has unexpected digest".into()))? diff --git a/tvix/store/src/pathinfoservice/combinators.rs b/tvix/store/src/pathinfoservice/combinators.rs index bb5595f72b10..1f413cf310a2 100644 --- a/tvix/store/src/pathinfoservice/combinators.rs +++ b/tvix/store/src/pathinfoservice/combinators.rs @@ -1,6 +1,5 @@ use std::sync::Arc; -use crate::proto::PathInfo; use futures::stream::BoxStream; use nix_compat::nixbase32; use tonic::async_trait; @@ -8,7 +7,7 @@ use tracing::{debug, instrument}; use tvix_castore::composition::{CompositionContext, ServiceBuilder}; use tvix_castore::Error; -use super::PathInfoService; +use super::{PathInfo, PathInfoService}; /// Asks near first, if not found, asks far. /// If found in there, returns it, and *inserts* it into @@ -105,11 +104,9 @@ mod test { use crate::{ pathinfoservice::{LruPathInfoService, MemoryPathInfoService, PathInfoService}, - tests::fixtures::PATH_INFO_WITH_NARINFO, + tests::fixtures::PATH_INFO, }; - const PATH_INFO_DIGEST: [u8; 20] = [0; 20]; - /// Helper function setting up an instance of a "far" and "near" /// PathInfoService. async fn create_pathinfoservice() -> super::Cache<LruPathInfoService, MemoryPathInfoService> { @@ -129,21 +126,25 @@ mod test { let svc = create_pathinfoservice().await; // query the PathInfo, things should not be there. - assert!(svc.get(PATH_INFO_DIGEST).await.unwrap().is_none()); + assert!(svc + .get(*PATH_INFO.store_path.digest()) + .await + .unwrap() + .is_none()); // insert it into the far one. - svc.far.put(PATH_INFO_WITH_NARINFO.clone()).await.unwrap(); + svc.far.put(PATH_INFO.clone()).await.unwrap(); // now try getting it again, it should succeed. assert_eq!( - Some(PATH_INFO_WITH_NARINFO.clone()), - svc.get(PATH_INFO_DIGEST).await.unwrap() + Some(PATH_INFO.clone()), + svc.get(*PATH_INFO.store_path.digest()).await.unwrap() ); // peek near, it should now be there. assert_eq!( - Some(PATH_INFO_WITH_NARINFO.clone()), - svc.near.get(PATH_INFO_DIGEST).await.unwrap() + Some(PATH_INFO.clone()), + svc.near.get(*PATH_INFO.store_path.digest()).await.unwrap() ); } } diff --git a/tvix/store/src/pathinfoservice/fs/mod.rs b/tvix/store/src/pathinfoservice/fs/mod.rs index 1f7fa8a8afce..d996ec9f6f76 100644 --- a/tvix/store/src/pathinfoservice/fs/mod.rs +++ b/tvix/store/src/pathinfoservice/fs/mod.rs @@ -58,32 +58,20 @@ where .as_ref() .get(*store_path.digest()) .await? - .map(|path_info| { - let node = path_info - .node - .as_ref() - .expect("missing root node") - .to_owned(); - - match node.into_name_and_node() { - Ok((_name, node)) => Ok(node), - Err(e) => Err(Error::StorageError(e.to_string())), - } - }) - .transpose()?) + .map(|path_info| path_info.node)) } fn list(&self) -> BoxStream<Result<(PathComponent, Node), Error>> { Box::pin(self.0.as_ref().list().map(|result| { - result.and_then(|path_info| { - let node = path_info - .node - .as_ref() - .expect("missing root node") - .to_owned(); - - node.into_name_and_node() - .map_err(|e| Error::StorageError(e.to_string())) + result.map(|path_info| { + let basename = path_info.store_path.to_string(); + ( + basename + .as_str() + .try_into() + .expect("Tvix bug: StorePath must be PathComponent"), + path_info.node, + ) }) })) } diff --git a/tvix/store/src/pathinfoservice/grpc.rs b/tvix/store/src/pathinfoservice/grpc.rs index 7510ccd911f0..d292b2a784f6 100644 --- a/tvix/store/src/pathinfoservice/grpc.rs +++ b/tvix/store/src/pathinfoservice/grpc.rs @@ -1,7 +1,7 @@ -use super::PathInfoService; +use super::{PathInfo, PathInfoService}; use crate::{ nar::NarCalculationService, - proto::{self, ListPathInfoRequest, PathInfo}, + proto::{self, ListPathInfoRequest}, }; use async_stream::try_stream; use futures::stream::BoxStream; @@ -53,15 +53,10 @@ where .await; match path_info { - Ok(path_info) => { - let path_info = path_info.into_inner(); - - path_info - .validate() - .map_err(|e| Error::StorageError(format!("invalid pathinfo: {}", e)))?; - - Ok(Some(path_info)) - } + Ok(path_info) => Ok(Some( + PathInfo::try_from(path_info.into_inner()) + .map_err(|e| Error::StorageError(format!("Invalid path info: {e}")))?, + )), Err(e) if e.code() == Code::NotFound => Ok(None), Err(e) => Err(Error::StorageError(e.to_string())), } @@ -72,12 +67,12 @@ where let path_info = self .grpc_client .clone() - .put(path_info) + .put(proto::PathInfo::from(path_info)) .await .map_err(|e| Error::StorageError(e.to_string()))? .into_inner(); - - Ok(path_info) + Ok(PathInfo::try_from(path_info) + .map_err(|e| Error::StorageError(format!("Invalid path info: {e}")))?) } #[instrument(level = "trace", skip_all)] @@ -91,21 +86,8 @@ where loop { match stream.message().await { - Ok(o) => match o { - Some(pathinfo) => { - // validate the pathinfo - if let Err(e) = pathinfo.validate() { - Err(Error::StorageError(format!( - "pathinfo {:?} failed validation: {}", - pathinfo, e - )))?; - } - yield pathinfo - } - None => { - return; - }, - }, + Ok(Some(path_info)) => yield PathInfo::try_from(path_info).map_err(|e| Error::StorageError(format!("Invalid path info: {e}")))?, + Ok(None) => return, Err(e) => Err(Error::StorageError(e.to_string()))?, } } diff --git a/tvix/store/src/pathinfoservice/lru.rs b/tvix/store/src/pathinfoservice/lru.rs index 695c04636089..2d8d52e3c9f6 100644 --- a/tvix/store/src/pathinfoservice/lru.rs +++ b/tvix/store/src/pathinfoservice/lru.rs @@ -8,11 +8,10 @@ use tokio::sync::RwLock; use tonic::async_trait; use tracing::instrument; -use crate::proto::PathInfo; use tvix_castore::composition::{CompositionContext, ServiceBuilder}; use tvix_castore::Error; -use super::PathInfoService; +use super::{PathInfo, PathInfoService}; pub struct LruPathInfoService { lru: Arc<RwLock<LruCache<[u8; 20], PathInfo>>>, @@ -35,15 +34,10 @@ impl PathInfoService for LruPathInfoService { #[instrument(level = "trace", skip_all, fields(path_info.root_node = ?path_info.node))] async fn put(&self, path_info: PathInfo) -> Result<PathInfo, Error> { - // call validate - let store_path = path_info - .validate() - .map_err(|e| Error::InvalidRequest(format!("invalid PathInfo: {}", e)))?; - self.lru .write() .await - .put(*store_path.digest(), path_info.clone()); + .put(*path_info.store_path.digest(), path_info.clone()); Ok(path_info) } @@ -91,40 +85,22 @@ impl ServiceBuilder for LruPathInfoServiceConfig { #[cfg(test)] mod test { + use nix_compat::store_path::StorePath; use std::num::NonZeroUsize; use crate::{ - pathinfoservice::{LruPathInfoService, PathInfoService}, - proto::PathInfo, - tests::fixtures::PATH_INFO_WITH_NARINFO, + pathinfoservice::{LruPathInfoService, PathInfo, PathInfoService}, + tests::fixtures::PATH_INFO, }; use lazy_static::lazy_static; - use tvix_castore::proto as castorepb; lazy_static! { - static ref PATHINFO_1: PathInfo = PATH_INFO_WITH_NARINFO.clone(); - static ref PATHINFO_1_DIGEST: [u8; 20] = [0; 20]; static ref PATHINFO_2: PathInfo = { - let mut p = PATHINFO_1.clone(); - let root_node = p.node.as_mut().unwrap(); - if let castorepb::Node { node: Some(node) } = root_node { - match node { - castorepb::node::Node::Directory(n) => { - n.name = "11111111111111111111111111111111-dummy2".into() - } - castorepb::node::Node::File(n) => { - n.name = "11111111111111111111111111111111-dummy2".into() - } - castorepb::node::Node::Symlink(n) => { - n.name = "11111111111111111111111111111111-dummy2".into() - } - } - } else { - unreachable!() - } + let mut p = PATH_INFO.clone(); + p.store_path = StorePath::from_name_and_digest_fixed("dummy", [1; 20]).unwrap(); p }; - static ref PATHINFO_2_DIGEST: [u8; 20] = *(PATHINFO_2.validate().unwrap()).digest(); + static ref PATHINFO_2_DIGEST: [u8; 20] = *PATHINFO_2.store_path.digest(); } #[tokio::test] @@ -133,18 +109,20 @@ mod test { // pathinfo_1 should not be there assert!(svc - .get(*PATHINFO_1_DIGEST) + .get(*PATH_INFO.store_path.digest()) .await .expect("no error") .is_none()); // insert it - svc.put(PATHINFO_1.clone()).await.expect("no error"); + svc.put(PATH_INFO.clone()).await.expect("no error"); // now it should be there. assert_eq!( - Some(PATHINFO_1.clone()), - svc.get(*PATHINFO_1_DIGEST).await.expect("no error") + Some(PATH_INFO.clone()), + svc.get(*PATH_INFO.store_path.digest()) + .await + .expect("no error") ); // insert pathinfo_2. This will evict pathinfo 1 @@ -158,7 +136,7 @@ mod test { // … but pathinfo 1 not anymore. assert!(svc - .get(*PATHINFO_1_DIGEST) + .get(*PATH_INFO.store_path.digest()) .await .expect("no error") .is_none()); diff --git a/tvix/store/src/pathinfoservice/memory.rs b/tvix/store/src/pathinfoservice/memory.rs index 3fabd239c7b1..fd013fe9a573 100644 --- a/tvix/store/src/pathinfoservice/memory.rs +++ b/tvix/store/src/pathinfoservice/memory.rs @@ -1,5 +1,4 @@ -use super::PathInfoService; -use crate::proto::PathInfo; +use super::{PathInfo, PathInfoService}; use async_stream::try_stream; use futures::stream::BoxStream; use nix_compat::nixbase32; @@ -29,22 +28,11 @@ impl PathInfoService for MemoryPathInfoService { #[instrument(level = "trace", skip_all, fields(path_info.root_node = ?path_info.node))] async fn put(&self, path_info: PathInfo) -> Result<PathInfo, Error> { - // Call validate on the received PathInfo message. - match path_info.validate() { - Err(e) => Err(Error::InvalidRequest(format!( - "failed to validate PathInfo: {}", - e - ))), + // This overwrites existing PathInfo objects with the same store path digest. + let mut db = self.db.write().await; + db.insert(*path_info.store_path.digest(), path_info.clone()); - // In case the PathInfo is valid, and we were able to extract a NixPath, store it in the database. - // This overwrites existing PathInfo objects. - Ok(nix_path) => { - let mut db = self.db.write().await; - db.insert(*nix_path.digest(), path_info.clone()); - - Ok(path_info) - } - } + Ok(path_info) } fn list(&self) -> BoxStream<'static, Result<PathInfo, Error>> { diff --git a/tvix/store/src/pathinfoservice/mod.rs b/tvix/store/src/pathinfoservice/mod.rs index 8d60ff79a33b..0a91d6267260 100644 --- a/tvix/store/src/pathinfoservice/mod.rs +++ b/tvix/store/src/pathinfoservice/mod.rs @@ -19,7 +19,7 @@ use tvix_castore::composition::{Registry, ServiceBuilder}; use tvix_castore::Error; use crate::nar::NarCalculationService; -use crate::proto::PathInfo; +pub use crate::path_info::PathInfo; pub use self::combinators::{ Cache as CachePathInfoService, CacheConfig as CachePathInfoServiceConfig, diff --git a/tvix/store/src/pathinfoservice/nix_http.rs b/tvix/store/src/pathinfoservice/nix_http.rs index 2ff094858bc9..ed386f0e9d14 100644 --- a/tvix/store/src/pathinfoservice/nix_http.rs +++ b/tvix/store/src/pathinfoservice/nix_http.rs @@ -1,10 +1,11 @@ -use super::PathInfoService; -use crate::{nar::ingest_nar_and_hash, proto::PathInfo}; +use super::{PathInfo, PathInfoService}; +use crate::nar::ingest_nar_and_hash; use futures::{stream::BoxStream, TryStreamExt}; use nix_compat::{ - narinfo::{self, NarInfo}, + narinfo::{self, NarInfo, Signature}, nixbase32, nixhash::NixHash, + store_path::StorePath, }; use reqwest::StatusCode; use std::sync::Arc; @@ -12,9 +13,7 @@ use tokio::io::{self, AsyncRead}; use tonic::async_trait; use tracing::{debug, instrument, warn}; use tvix_castore::composition::{CompositionContext, ServiceBuilder}; -use tvix_castore::{ - blobservice::BlobService, directoryservice::DirectoryService, proto as castorepb, Error, -}; +use tvix_castore::{blobservice::BlobService, directoryservice::DirectoryService, Error}; use url::Url; /// NixHTTPPathInfoService acts as a bridge in between the Nix HTTP Binary cache @@ -137,12 +136,11 @@ where } } - // Convert to a (sparse) PathInfo. We still need to populate the node field, - // and for this we need to download the NAR file. + // To construct the full PathInfo, we also need to populate the node field, + // and for this we need to download the NAR file and ingest it into castore. // FUTUREWORK: Keep some database around mapping from narsha256 to // (unnamed) rootnode, so we can use that (and the name from the // StorePath) and avoid downloading the same NAR a second time. - let pathinfo: PathInfo = (&narinfo).into(); // create a request for the NAR file itself. let nar_url = self.base_url.join(narinfo.url).map_err(|e| { @@ -228,12 +226,18 @@ where } Ok(Some(PathInfo { - node: Some(castorepb::Node::from_name_and_node( - narinfo.store_path.to_string().into(), - root_node, - )), - references: pathinfo.references, - narinfo: pathinfo.narinfo, + store_path: narinfo.store_path.to_owned(), + node: root_node, + references: narinfo.references.iter().map(StorePath::to_owned).collect(), + nar_size: narinfo.nar_size, + nar_sha256: narinfo.nar_hash, + deriver: narinfo.deriver.as_ref().map(StorePath::to_owned), + signatures: narinfo + .signatures + .into_iter() + .map(|s| Signature::<String>::new(s.name().to_string(), s.bytes().to_owned())) + .collect(), + ca: narinfo.ca, })) } diff --git a/tvix/store/src/pathinfoservice/redb.rs b/tvix/store/src/pathinfoservice/redb.rs index bd0e0fc2b686..6e794e1981f0 100644 --- a/tvix/store/src/pathinfoservice/redb.rs +++ b/tvix/store/src/pathinfoservice/redb.rs @@ -1,5 +1,5 @@ -use super::PathInfoService; -use crate::proto::PathInfo; +use super::{PathInfo, PathInfoService}; +use crate::proto; use data_encoding::BASE64; use futures::{stream::BoxStream, StreamExt}; use prost::Message; @@ -78,10 +78,13 @@ impl PathInfoService for RedbPathInfoService { let table = txn.open_table(PATHINFO_TABLE)?; match table.get(digest)? { Some(pathinfo_bytes) => Ok(Some( - PathInfo::decode(pathinfo_bytes.value().as_slice()).map_err(|e| { - warn!(err=%e, "failed to decode stored PathInfo"); - Error::StorageError("failed to decode stored PathInfo".to_string()) - })?, + proto::PathInfo::decode(pathinfo_bytes.value().as_slice()) + .map_err(|e| { + warn!(err=%e, "failed to decode stored PathInfo"); + Error::StorageError("failed to decode stored PathInfo".to_string()) + })? + .try_into() + .map_err(|e| Error::StorageError(format!("Invalid path info: {e}")))?, )), None => Ok(None), } @@ -92,25 +95,19 @@ impl PathInfoService for RedbPathInfoService { #[instrument(level = "trace", skip_all, fields(path_info.root_node = ?path_info.node))] async fn put(&self, path_info: PathInfo) -> Result<PathInfo, Error> { - // Call validate on the received PathInfo message. - let store_path = path_info - .validate() - .map_err(|e| { - warn!(err=%e, "failed to validate PathInfo"); - Error::StorageError("failed to validate PathInfo".to_string()) - })? - .to_owned(); - - let path_info_encoded = path_info.encode_to_vec(); let db = self.db.clone(); tokio::task::spawn_blocking({ + let path_info = path_info.clone(); move || -> Result<(), Error> { let txn = db.begin_write()?; { let mut table = txn.open_table(PATHINFO_TABLE)?; table - .insert(store_path.digest(), path_info_encoded) + .insert( + *path_info.store_path.digest(), + proto::PathInfo::from(path_info).encode_to_vec(), + ) .map_err(|e| { warn!(err=%e, "failed to insert PathInfo"); Error::StorageError("failed to insert PathInfo".to_string()) @@ -137,12 +134,18 @@ impl PathInfoService for RedbPathInfoService { for elem in table.iter()? { let elem = elem?; tokio::runtime::Handle::current() - .block_on(tx.send(Ok( - PathInfo::decode(elem.1.value().as_slice()).map_err(|e| { + .block_on(tx.send(Ok({ + let path_info_proto = proto::PathInfo::decode( + elem.1.value().as_slice(), + ) + .map_err(|e| { warn!(err=%e, "invalid PathInfo"); Error::StorageError("invalid PathInfo".to_string()) - })?, - ))) + })?; + PathInfo::try_from(path_info_proto).map_err(|e| { + Error::StorageError(format!("Invalid path info: {e}")) + })? + }))) .map_err(|e| Error::StorageError(e.to_string()))?; } diff --git a/tvix/store/src/pathinfoservice/signing_wrapper.rs b/tvix/store/src/pathinfoservice/signing_wrapper.rs index 7f754ec49849..3230e000ab96 100644 --- a/tvix/store/src/pathinfoservice/signing_wrapper.rs +++ b/tvix/store/src/pathinfoservice/signing_wrapper.rs @@ -1,7 +1,6 @@ //! This module provides a [PathInfoService] implementation that signs narinfos -use super::PathInfoService; -use crate::proto::PathInfo; +use super::{PathInfo, PathInfoService}; use futures::stream::BoxStream; use std::path::PathBuf; use std::sync::Arc; @@ -11,9 +10,9 @@ use tvix_castore::composition::{CompositionContext, ServiceBuilder}; use tvix_castore::Error; -use nix_compat::narinfo::{parse_keypair, SigningKey}; +use nix_compat::narinfo::{parse_keypair, Signature, SigningKey}; use nix_compat::nixbase32; -use tracing::{instrument, warn}; +use tracing::instrument; #[cfg(test)] use super::MemoryPathInfoService; @@ -52,22 +51,15 @@ where } async fn put(&self, path_info: PathInfo) -> Result<PathInfo, Error> { - let store_path = path_info.validate().map_err(|e| { - warn!(err=%e, "invalid PathInfo"); - Error::StorageError(e.to_string()) - })?; - let root_node = path_info.node.clone(); - // If we have narinfo then sign it, else passthrough to the upper pathinfoservice - let path_info_to_put = match path_info.to_narinfo(store_path.as_ref()) { - Some(mut nar_info) => { - nar_info.add_signature(self.signing_key.as_ref()); - let mut signed_path_info = PathInfo::from(&nar_info); - signed_path_info.node = root_node; - signed_path_info - } - None => path_info, - }; - self.inner.put(path_info_to_put).await + let mut path_info = path_info.clone(); + let mut nar_info = path_info.to_narinfo(); + nar_info.add_signature(self.signing_key.as_ref()); + path_info.signatures = nar_info + .signatures + .into_iter() + .map(|s| Signature::<String>::new(s.name().to_string(), s.bytes().to_owned())) + .collect(); + self.inner.put(path_info).await } fn list(&self) -> BoxStream<'static, Result<PathInfo, Error>> { @@ -134,51 +126,35 @@ pub const DUMMY_VERIFYING_KEY: &str = "do.not.use:cuXqnuzlWfGTKmfzBPx2kXShjRryZM #[cfg(test)] mod test { - use crate::{ - pathinfoservice::PathInfoService, - proto::PathInfo, - tests::fixtures::{DUMMY_PATH, PATH_INFO_WITH_NARINFO}, - }; + use crate::{pathinfoservice::PathInfoService, tests::fixtures::PATH_INFO}; use nix_compat::narinfo::VerifyingKey; - use lazy_static::lazy_static; - use nix_compat::store_path::StorePath; - - lazy_static! { - static ref PATHINFO_1: PathInfo = PATH_INFO_WITH_NARINFO.clone(); - static ref PATHINFO_1_DIGEST: [u8; 20] = [0; 20]; - } - #[tokio::test] async fn put_and_verify_signature() { let svc = super::test_signing_service(); // pathinfo_1 should not be there ... assert!(svc - .get(*PATHINFO_1_DIGEST) + .get(*PATH_INFO.store_path.digest()) .await .expect("no error") .is_none()); // ... and not be signed - assert!(PATHINFO_1.narinfo.clone().unwrap().signatures.is_empty()); + assert!(PATH_INFO.signatures.is_empty()); // insert it - svc.put(PATHINFO_1.clone()).await.expect("no error"); + svc.put(PATH_INFO.clone()).await.expect("no error"); // now it should be there ... let signed = svc - .get(*PATHINFO_1_DIGEST) + .get(*PATH_INFO.store_path.digest()) .await .expect("no error") .unwrap(); // and signed - let narinfo = signed - .to_narinfo( - StorePath::from_bytes(DUMMY_PATH.as_bytes()).expect("DUMMY_PATH to be parsed"), - ) - .expect("no error"); + let narinfo = signed.to_narinfo(); let fp = narinfo.fingerprint(); // load our keypair from the fixtures diff --git a/tvix/store/src/pathinfoservice/tests/mod.rs b/tvix/store/src/pathinfoservice/tests/mod.rs index 028fa5af57fc..12c685c80fad 100644 --- a/tvix/store/src/pathinfoservice/tests/mod.rs +++ b/tvix/store/src/pathinfoservice/tests/mod.rs @@ -6,12 +6,10 @@ use futures::TryStreamExt; use rstest::*; use rstest_reuse::{self, *}; -use super::PathInfoService; +use super::{PathInfo, PathInfoService}; use crate::pathinfoservice::redb::RedbPathInfoService; use crate::pathinfoservice::MemoryPathInfoService; -use crate::proto::PathInfo; -use crate::tests::fixtures::DUMMY_PATH_DIGEST; -use tvix_castore::proto as castorepb; +use crate::tests::fixtures::{DUMMY_PATH_DIGEST, PATH_INFO}; use crate::pathinfoservice::test_signing_service; @@ -52,32 +50,35 @@ async fn not_found(svc: impl PathInfoService) { #[apply(path_info_services)] #[tokio::test] async fn put_get(svc: impl PathInfoService) { - let path_info = PathInfo { - node: Some(castorepb::Node { - node: Some(castorepb::node::Node::Symlink(castorepb::SymlinkNode { - name: "00000000000000000000000000000000-foo".into(), - target: "doesntmatter".into(), - })), - }), - ..Default::default() - }; - // insert - let resp = svc.put(path_info.clone()).await.expect("must succeed"); + let resp = svc.put(PATH_INFO.clone()).await.expect("must succeed"); - // expect the returned PathInfo to be equal (for now) - // in the future, some stores might add additional fields/signatures. - assert_eq!(path_info, resp); + // expect the returned PathInfo to be equal, + // remove the signatures as the SigningPathInfoService adds them + assert_eq!(*PATH_INFO, strip_signatures(resp)); // get it back let resp = svc.get(DUMMY_PATH_DIGEST).await.expect("must succeed"); - assert_eq!(Some(path_info.clone()), resp); + assert_eq!(Some(PATH_INFO.clone()), resp.map(strip_signatures)); // Ensure the listing endpoint works, and returns the same path_info. // FUTUREWORK: split this, some impls might (rightfully) not support listing let pathinfos: Vec<PathInfo> = svc.list().try_collect().await.expect("must succeed"); // We should get a single pathinfo back, the one we inserted. - assert_eq!(vec![path_info], pathinfos); + assert_eq!( + vec![PATH_INFO.clone()], + pathinfos + .into_iter() + .map(strip_signatures) + .collect::<Vec<_>>() + ); +} + +fn strip_signatures(path_info: PathInfo) -> PathInfo { + PathInfo { + signatures: vec![], + ..path_info + } } |