about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2024-10-01T14·07+0300
committerclbot <clbot@tvl.fyi>2024-10-01T14·27+0000
commitf0d5ed7074097ca44a86865ef82cdbd1d1bddff7 (patch)
treeb6c8eb2be12385c64b1cea572d5dfba5618e0dba
parent284c1eb45aa1a7d8436a3e8edb20bb0fbbced5c7 (diff)
docs(tvix/TODO): add PathInfo data types and ca reference items r/8746
With https://cl.tvl.fyi/12533 in, we still need to lookup references to
properly populate `BuildRequest`.

It currently fails as the reference to
h9lc1dpi14z7is86ffhl3ld569138595-audit-tmpdir.sh is not propagated.

We should prevent Frankenbuilds from the go, so let's update our
PathInfo type to accomodate for that.

Change-Id: I26f9215312c258bba222efd390bc135f1a3a3d6d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12560
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
-rw-r--r--tvix/docs/src/TODO.md43
1 files changed, 43 insertions, 0 deletions
diff --git a/tvix/docs/src/TODO.md b/tvix/docs/src/TODO.md
index af558c9580fe..9f8feef9e3ac 100644
--- a/tvix/docs/src/TODO.md
+++ b/tvix/docs/src/TODO.md
@@ -136,6 +136,49 @@ Similarly, we also don't properly populate the build environment for
 `fetchClosure` yet. (Note there already is `ExportedPathInfo`, so once
 `structuredAttrs` is there this should be easy.
 
+### PathInfo Data types
+Similar to the refactors done in tvix-castore, we want a stricter type for
+PathInfo, and use the `tvix_castore::nodes::Node` type we now have as the root
+node.
+
+This allows removing some checks, conversions and handling for invalid data in
+many different places in different store implementations.
+
+Steps:
+
+ - Define the stricter `PathInfo` type
+ - Update the `PathInfoService` trait to use the stricter types
+ - Update the grpc client impl to convert from the proto types to the
+   stricter types (and reject invalid ones)
+ - Update the grpc server wrapper to convert to the proto types
+
+### PathInfo: include references by content
+In the PathInfo struct, we currently only store references by their names and
+store path hash. Getting the castore node for the content at that store path
+requires another lookup to the PathInfoService.
+
+Due to this information missing, this also means we currently cannot preserve
+information necessary to detect/prevent
+[Frankenbuilds](https://tvl.fyi/blog/tvix-update-february-24#builder-protocol-drv-builder).
+
+We should extend/change the `PathInfo` type to maintain references in a
+`BTreeMap` from store path basename to castore node. Should probably be done
+after PathInfo uses its own data type.
+
+The `NixHTTPPathInfoService` needs to get some more logic to populate the ca
+bits of the references:
+
+ - If the referenced store path if not present in our PathInfoService hierarchy,
+   it needs to be ingested too, and persisted.
+ - If the referenced store path is present, we can use the castore root from there.
+   In an optional mode, we should parse the .narinfo file for the reference, and
+   compare the nar hash with our local one, to detect Frankenbuilds in a binary
+cache.
+
+As `NixHTTPPathInfoService` now needs to interact with "the PathInfoService"
+hierarchy, we need to accept a pointer to a PathInfoService (hierarchy) that's
+used to check for presence, and PathInfos are inserted into.
+
 ### Builders
 Once builds are proven to work with real-world builds, and the corner cases
 there are ruled out, adding other types of builders might be interesting.