Skip to content

Commit

Permalink
Make building only require a LocalFSStore
Browse files Browse the repository at this point in the history
This is the crucial first step to allowing client-side building --- i.e.
building locally but using some form of RPC to manipulate metadata
rather than interact with a concrete form of persisting metadata like
the SQLite database.

To facilitate this, a few methods were hastily moved from `LocalStore`
to `LocalFSStore`. We should not just accept that as good enough to
merge, but carefully consider the trust/security implications of this,
and the proper division of labor between the less-trusted "builder" and
the more-trusted "store".
  • Loading branch information
Ericson2314 committed Nov 19, 2023
1 parent 82e23c1 commit 29be795
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 30 deletions.
4 changes: 2 additions & 2 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ void DerivationGoal::tryToBuild()
PathSet lockFiles;
/* FIXME: Should lock something like the drv itself so we don't build same
CA drv concurrently */
if (dynamic_cast<LocalStore *>(&worker.store)) {
if (dynamic_cast<LocalFSStore *>(&worker.store)) {
/* If we aren't a local store, we might need to use the local store as
a build remote, but that would cause a deadlock. */
/* FIXME: Make it so we can use ourselves as a build remote even if we
Expand Down Expand Up @@ -1240,7 +1240,7 @@ Path DerivationGoal::openLogFile()

/* Create a log file. */
Path logDir;
if (auto localStore = dynamic_cast<LocalStore *>(&worker.store))
if (auto localStore = dynamic_cast<LocalFSStore *>(&worker.store))
logDir = localStore->logDir;
else
logDir = settings.nixLogDir;
Expand Down
24 changes: 12 additions & 12 deletions src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ inline bool LocalDerivationGoal::needsHashRewrite()
}


LocalStore & LocalDerivationGoal::getLocalStore()
LocalFSStore & LocalDerivationGoal::getLocalFSStore()
{
auto p = dynamic_cast<LocalStore *>(&worker.store);
auto p = dynamic_cast<LocalFSStore *>(&worker.store);
assert(p);
return *p;
}
Expand Down Expand Up @@ -204,7 +204,7 @@ void LocalDerivationGoal::tryLocalBuild()
useChroot = derivationType->isSandboxed() && !noChroot;
}

auto & localStore = getLocalStore();
auto & localStore = getLocalFSStore();
if (localStore.storeDir != localStore.realStoreDir.get()) {
#if __linux__
useChroot = true;
Expand Down Expand Up @@ -343,7 +343,7 @@ bool LocalDerivationGoal::cleanupDecideWhetherDiskFull()
so, we don't mark this build as a permanent failure. */
#if HAVE_STATVFS
{
auto & localStore = getLocalStore();
auto & localStore = getLocalFSStore();
uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable
struct statvfs st;
if (statvfs(localStore.realStoreDir.get().c_str(), &st) == 0 &&
Expand Down Expand Up @@ -1223,16 +1223,16 @@ struct RestrictedStoreConfig : virtual LocalFSStoreConfig
const std::string name() { return "Restricted Store"; }
};

/* A wrapper around LocalStore that only allows building/querying of
/* A wrapper around LocalFSStore that only allows building/querying of
paths that are in the input closures of the build or were added via
recursive Nix calls. */
struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual IndirectRootStore, public virtual GcStore
{
ref<LocalStore> next;
ref<LocalFSStore> next;

LocalDerivationGoal & goal;

RestrictedStore(const Params & params, ref<LocalStore> next, LocalDerivationGoal & goal)
RestrictedStore(const Params & params, ref<LocalFSStore> next, LocalDerivationGoal & goal)
: StoreConfig(params)
, LocalFSStoreConfig(params)
, RestrictedStoreConfig(params)
Expand Down Expand Up @@ -1459,12 +1459,12 @@ void LocalDerivationGoal::startDaemon()
Store::Params params;
params["path-info-cache-size"] = "0";
params["store"] = worker.store.storeDir;
if (auto & optRoot = getLocalStore().rootDir.get())
if (auto & optRoot = getLocalFSStore().rootDir.get())
params["root"] = *optRoot;
params["state"] = "/no-such-path";
params["log"] = "/no-such-path";
auto store = make_ref<RestrictedStore>(params,
ref<LocalStore>(std::dynamic_pointer_cast<LocalStore>(worker.store.shared_from_this())),
ref<LocalFSStore>(std::dynamic_pointer_cast<LocalFSStore>(worker.store.shared_from_this())),
*this);

addedPaths.clear();
Expand Down Expand Up @@ -2632,7 +2632,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
}
}

auto & localStore = getLocalStore();
auto & localStore = getLocalFSStore();

if (buildMode == bmCheck) {

Expand Down Expand Up @@ -2709,7 +2709,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
paths referenced by each of them. If there are cycles in the
outputs, this will fail. */
{
auto & localStore = getLocalStore();
auto & localStore = getLocalFSStore();

ValidPathInfos infos2;
for (auto & [outputName, newInfo] : infos) {
Expand Down Expand Up @@ -2754,7 +2754,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()

void LocalDerivationGoal::signRealisation(Realisation & realisation)
{
getLocalStore().signRealisation(realisation);
getLocalFSStore().signRealisation(realisation);
}


Expand Down
4 changes: 2 additions & 2 deletions src/libstore/build/local-derivation-goal.hh
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
///@file

#include "derivation-goal.hh"
#include "local-store.hh"
#include "local-fs-store.hh"
#include "processes.hh"

namespace nix {

struct LocalDerivationGoal : public DerivationGoal
{
LocalStore & getLocalStore();
LocalFSStore & getLocalFSStore();

/**
* User selected for running the builder.
Expand Down
6 changes: 4 additions & 2 deletions src/libstore/build/worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include "substitution-goal.hh"
#include "drv-output-substitution-goal.hh"
#include "local-derivation-goal.hh"
// Just for `autoGC`, which should have own interface, don't add more usage!
#include "local-store.hh"
#include "hook-instance.hh"
#include "signals.hh"

Expand Down Expand Up @@ -64,7 +66,7 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv
const OutputsSpec & wantedOutputs, BuildMode buildMode)
{
return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
return !dynamic_cast<LocalStore *>(&store)
return !dynamic_cast<LocalFSStore *>(&store)
? std::make_shared</* */DerivationGoal>(drvPath, wantedOutputs, *this, buildMode)
: std::make_shared<LocalDerivationGoal>(drvPath, wantedOutputs, *this, buildMode);
});
Expand All @@ -75,7 +77,7 @@ std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath
const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode)
{
return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
return !dynamic_cast<LocalStore *>(&store)
return !dynamic_cast<LocalFSStore *>(&store)
? std::make_shared</* */DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode)
: std::make_shared<LocalDerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode);
});
Expand Down
34 changes: 34 additions & 0 deletions src/libstore/local-fs-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,40 @@ public:

std::optional<std::string> getBuildLogExact(const StorePath & path) override;

/**
* @todo this was moved to `LocalFSStore` from `LocalStore` because
* building needed it. Instead of just blindly moving it, we should
* should consider the division of labor and trust between the
* builder and the store.
*/
virtual void registerValidPaths(const ValidPathInfos & infos)
{ unsupported("registerValidPaths"); }

/**
* Optimise a single store path. Optionally, test the encountered
* symlinks for corruption.
*
* @todo this was moved to `LocalFSStore` from `LocalStore` because
* building needed it. Instead of just blindly moving it, we should
* should consider the division of labor and trust between the
* builder and the store.
*/
virtual void optimisePath(const Path & path, RepairFlag repair)
{ unsupported("optimisePath"); }

/**
* Add signatures to a ValidPathInfo or Realisation using the secret keys
* specified by the ‘secret-key-files’ option.
*
* @todo this was moved to `LocalFSStore` from `LocalStore` because
* building needed it. Instead of just blindly moving it, we should
* should consider the division of labor and trust between the
* builder and the store.
*/
virtual void signPathInfo(ValidPathInfo & info)
{ unsupported("signPathInfo"); }
virtual void signRealisation(Realisation &)
{ unsupported("signRealisation"); }
};

}
16 changes: 4 additions & 12 deletions src/libstore/local-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,7 @@ public:

void optimiseStore() override;

/**
* Optimise a single store path. Optionally, test the encountered
* symlinks for corruption.
*/
void optimisePath(const Path & path, RepairFlag repair);
void optimisePath(const Path & path, RepairFlag repair) override;

bool verifyStore(bool checkContents, RepairFlag repair) override;

Expand All @@ -255,7 +251,7 @@ public:
*/
void registerValidPath(const ValidPathInfo & info);

void registerValidPaths(const ValidPathInfos & infos);
void registerValidPaths(const ValidPathInfos & infos) override;

unsigned int getProtocol() override;

Expand Down Expand Up @@ -343,12 +339,8 @@ private:
bool isValidPath_(State & state, const StorePath & path);
void queryReferrers(State & state, const StorePath & path, StorePathSet & referrers);

/**
* Add signatures to a ValidPathInfo or Realisation using the secret keys
* specified by the ‘secret-key-files’ option.
*/
void signPathInfo(ValidPathInfo & info);
void signRealisation(Realisation &);
void signPathInfo(ValidPathInfo & info) override;
void signRealisation(Realisation &) override;

// XXX: Make a generic `Store` method
ContentAddress hashCAPath(
Expand Down

0 comments on commit 29be795

Please sign in to comment.