docs(tvix/TODO): add PathInfo data types and ca reference items

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
This commit is contained in:
Florian Klink 2024-10-01 17:07:06 +03:00 committed by clbot
parent 284c1eb45a
commit f0d5ed7074

View file

@ -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.