From db8439c32842139cca013078cf3230e6aec2d1be Mon Sep 17 00:00:00 2001 From: Las Date: Mon, 10 Mar 2025 23:21:03 +0000 Subject: [PATCH] Remove `signRealisation` from drv goal We can move this method from `LocalStore` to `Store` --- even if we only want the actual builder to sign things in many cases, there is no reason to try to enforce this policy by spurious moving the method to a subclass. Now, we might technically sign class, but CA derivations is experimental, and @Ericson2314 is going to revisit all this stuff with issue #11896 anyways. --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/derivation-goal.hh | 5 ----- src/libstore/local-store.cc | 13 ------------- src/libstore/local-store.hh | 1 - src/libstore/store-api.cc | 13 +++++++++++++ src/libstore/store-api.hh | 2 ++ src/libstore/unix/build/local-derivation-goal.cc | 7 +------ src/libstore/unix/build/local-derivation-goal.hh | 2 -- 8 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b673153a3..3e7778656 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1047,7 +1047,7 @@ Goal::Co DerivationGoal::resolvedFinished() : worker.store; newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); } - signRealisation(newRealisation); + worker.store.signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } outputPaths.insert(realisation.outPath); diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 84d425b95..2eb36e98a 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -267,11 +267,6 @@ struct DerivationGoal : public Goal */ Path openLogFile(); - /** - * Sign the newly built realisation if the store allows it - */ - virtual void signRealisation(Realisation&) {} - /** * Close the log file. */ diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index b54903432..2fd160dd4 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1585,19 +1585,6 @@ void LocalStore::addSignatures(const StorePath & storePath, const StringSet & si } -void LocalStore::signRealisation(Realisation & realisation) -{ - // FIXME: keep secret keys in memory. - - auto secretKeyFiles = settings.secretKeyFiles; - - for (auto & secretKeyFile : secretKeyFiles.get()) { - SecretKey secretKey(readFile(secretKeyFile)); - LocalSigner signer(std::move(secretKey)); - realisation.sign(signer); - } -} - void LocalStore::signPathInfo(ValidPathInfo & info) { // FIXME: keep secret keys in memory. diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 83154d651..07d9cec0a 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -401,7 +401,6 @@ private: * specified by the ‘secret-key-files’ option. */ void signPathInfo(ValidPathInfo & info); - void signRealisation(Realisation &); void addBuildLog(const StorePath & drvPath, std::string_view log) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index fc3fbcc0f..e99e96aac 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1274,6 +1274,19 @@ Derivation Store::readDerivation(const StorePath & drvPath) Derivation Store::readInvalidDerivation(const StorePath & drvPath) { return readDerivationCommon(*this, drvPath, false); } +void Store::signRealisation(Realisation & realisation) +{ + // FIXME: keep secret keys in memory. + + auto secretKeyFiles = settings.secretKeyFiles; + + for (auto & secretKeyFile : secretKeyFiles.get()) { + SecretKey secretKey(readFile(secretKeyFile)); + LocalSigner signer(std::move(secretKey)); + realisation.sign(signer); + } +} + } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 2eba88ea0..83ca8344b 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -622,6 +622,8 @@ public: virtual void addSignatures(const StorePath & storePath, const StringSet & sigs) { unsupported("addSignatures"); } + void signRealisation(Realisation &); + /* Utility functions. */ /** diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 659ac1b10..7f6bace86 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2872,7 +2872,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv->type().isImpure()) { - signRealisation(thisRealisation); + worker.store.signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } builtOutputs.emplace(outputName, thisRealisation); @@ -2881,11 +2881,6 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() return builtOutputs; } -void LocalDerivationGoal::signRealisation(Realisation & realisation) -{ - getLocalStore().signRealisation(realisation); -} - void LocalDerivationGoal::checkOutputs(const std::map & outputs) { diff --git a/src/libstore/unix/build/local-derivation-goal.hh b/src/libstore/unix/build/local-derivation-goal.hh index 6bd3ce45f..ec8cf2c0b 100644 --- a/src/libstore/unix/build/local-derivation-goal.hh +++ b/src/libstore/unix/build/local-derivation-goal.hh @@ -241,8 +241,6 @@ struct LocalDerivationGoal : public DerivationGoal */ SingleDrvOutputs registerOutputs(); - void signRealisation(Realisation &) override; - /** * Check that an output meets the requirements specified by the * 'outputChecks' attribute (or the legacy