puck/lix
0
0
Fork 0
forked from lix-project/lix
Code Pull requests Activity

Merge pull request #9157 from obsidiansystems/protocol-versions

Browse source 
Add protocol versions to `{Worker,Serve}Proto::*Conn`
This commit is contained in:
Robert Hensing 2023-10-20 15:34:26 +02:00 committed by GitHub
parent 8a28ed2e8b e36c9175f4
commit 4d17c59d8d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 207 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/libstore/daemon.cc
View file

@ -45,9 +45,9 @@ struct TunnelLogger : public Logger
Sync<State> state_;
unsigned int clientVersion;
WorkerProto::Version clientVersion;
TunnelLogger(FdSink & to, unsigned int clientVersion)
TunnelLogger(FdSink & to, WorkerProto::Version clientVersion)
: to(to), clientVersion(clientVersion) { }
void enqueueMsg(const std::string & s)
@ -261,7 +261,7 @@ struct ClientSettings
}
};
static std::vector<DerivedPath> readDerivedPaths(Store & store, unsigned int clientVersion, WorkerProto::ReadConn conn)
static std::vector<DerivedPath> readDerivedPaths(Store & store, WorkerProto::Version clientVersion, WorkerProto::ReadConn conn)
{
std::vector<DerivedPath> reqs;
if (GET_PROTOCOL_MINOR(clientVersion) >= 30) {
@ -274,11 +274,17 @@ static std::vector<DerivedPath> readDerivedPaths(Store & store, unsigned int cli
}
static void performOp(TunnelLogger * logger, ref<Store> store,
TrustedFlag trusted, RecursiveFlag recursive, unsigned int clientVersion,
TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion,
Source & from, BufferedSink & to, WorkerProto::Op op)
{
WorkerProto::ReadConn rconn { .from = from };
WorkerProto::WriteConn wconn { .to = to };
WorkerProto::ReadConn rconn {
.from = from,
.version = clientVersion,
};
WorkerProto::WriteConn wconn {
.to = to,
.version = clientVersion,
};
switch (op) {
@ -1017,7 +1023,7 @@ void processConnection(
if (magic != WORKER_MAGIC_1) throw Error("protocol mismatch");
to << WORKER_MAGIC_2 << PROTOCOL_VERSION;
to.flush();
unsigned int clientVersion = readInt(from);
WorkerProto::Version clientVersion = readInt(from);
if (clientVersion < 0x10a)
throw Error("the Nix client version is too old");
@ -1052,7 +1058,10 @@ void processConnection(
auto temp = trusted
? store->isTrustedClient()
: std::optional { NotTrusted };
WorkerProto::WriteConn wconn { .to = to };
WorkerProto::WriteConn wconn {
.to = to,
.version = clientVersion,
};
WorkerProto::write(*store, wconn, temp);
}

4
src/libstore/legacy-ssh-store.cc
View file

@ -45,7 +45,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
std::unique_ptr<SSHMaster::Connection> sshConn;
FdSink to;
FdSource from;
int remoteVersion;
ServeProto::Version remoteVersion;
bool good = true;
/**
@ -60,6 +60,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
{
return ServeProto::ReadConn {
.from = from,
.version = remoteVersion,
};
}
@ -75,6 +76,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
{
return ServeProto::WriteConn {
.to = to,
.version = remoteVersion,
};
}
};

10
src/libstore/path-info.cc
View file

@ -141,7 +141,10 @@ ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned
ValidPathInfo info(path, narHash);
if (deriver != "") info.deriver = store.parseStorePath(deriver);
info.references = WorkerProto::Serialise<StorePathSet>::read(store,
WorkerProto::ReadConn { .from = source });
WorkerProto::ReadConn {
.from = source,
.version = format,
});
source >> info.registrationTime >> info.narSize;
if (format >= 16) {
source >> info.ultimate;
@ -163,7 +166,10 @@ void ValidPathInfo::write(
sink << (deriver ? store.printStorePath(*deriver) : "")
<< narHash.to_string(HashFormat::Base16, false);
WorkerProto::write(store,
WorkerProto::WriteConn { .to = sink },
WorkerProto::WriteConn {
.to = sink,
.version = format,
},
references);
sink << registrationTime << narSize;
if (format >= 16) {

4
src/libstore/remote-store-connection.hh
View file

@ -30,7 +30,7 @@ struct RemoteStore::Connection
* sides support. (If the maximum doesn't exist, we would fail to
* establish a connection and produce a value of this type.)
*/
unsigned int daemonVersion;
WorkerProto::Version daemonVersion;
/**
* Whether the remote side trusts us or not.
@ -70,6 +70,7 @@ struct RemoteStore::Connection
{
return WorkerProto::ReadConn {
.from = from,
.version = daemonVersion,
};
}
@ -85,6 +86,7 @@ struct RemoteStore::Connection
{
return WorkerProto::WriteConn {
.to = to,
.version = daemonVersion,
};
}

15
src/libstore/serve-protocol.hh
View file

@ -30,26 +30,29 @@ struct ServeProto
*/
enum struct Command : uint64_t;
/**
* Version type for the protocol.
*
* @todo Convert to struct with separate major vs minor fields.
*/
using Version = unsigned int;
/**
* A unidirectional read connection, to be used by the read half of the
* canonical serializers below.
*
* This currently is just a `Source &`, but more fields will be added
* later.
*/
struct ReadConn {
Source & from;
Version version;
};
/**
* A unidirectional write connection, to be used by the write half of the
* canonical serializers below.
*
* This currently is just a `Sink &`, but more fields will be added
* later.
*/
struct WriteConn {
Sink & to;
Version version;
};
/**

73
src/libstore/tests/common-protocol.cc
View file

@ -13,10 +13,71 @@ namespace nix {
const char commonProtoDir[] = "common-protocol";
using CommonProtoTest = ProtoTest<CommonProto, commonProtoDir>;
class CommonProtoTest : public ProtoTest<CommonProto, commonProtoDir>
{
public:
/**
* Golden test for `T` reading
*/
template<typename T>
void readTest(PathView testStem, T value)
{
if (testAccept())
{
GTEST_SKIP() << "Cannot read golden master because another test is also updating it";
}
else
{
auto expected = readFile(goldenMaster(testStem));
T got = ({
StringSource from { expected };
CommonProto::Serialise<T>::read(
*store,
CommonProto::ReadConn { .from = from });
});
ASSERT_EQ(got, value);
}
}
/**
* Golden test for `T` write
*/
template<typename T>
void writeTest(PathView testStem, const T & value)
{
auto file = goldenMaster(testStem);
StringSink to;
CommonProto::write(
*store,
CommonProto::WriteConn { .to = to },
value);
if (testAccept())
{
createDirs(dirOf(file));
writeFile(file, to.s);
GTEST_SKIP() << "Updating golden master";
}
else
{
auto expected = readFile(file);
ASSERT_EQ(to.s, expected);
}
}
};
#define CHARACTERIZATION_TEST(NAME, STEM, VALUE) \
TEST_F(CommonProtoTest, NAME ## _read) { \
readTest(STEM, VALUE); \
} \
TEST_F(CommonProtoTest, NAME ## _write) { \
writeTest(STEM, VALUE); \
}
CHARACTERIZATION_TEST(
CommonProtoTest,
string,
"string",
(std::tuple<std::string, std::string, std::string, std::string, std::string> {
@ -28,7 +89,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
storePath,
"store-path",
(std::tuple<StorePath, StorePath> {
@ -37,7 +97,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
contentAddress,
"content-address",
(std::tuple<ContentAddress, ContentAddress, ContentAddress> {
@ -56,7 +115,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
drvOutput,
"drv-output",
(std::tuple<DrvOutput, DrvOutput> {
@ -71,7 +129,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
realisation,
"realisation",
(std::tuple<Realisation, Realisation> {
@ -103,7 +160,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
vector,
"vector",
(std::tuple<std::vector<std::string>, std::vector<std::string>, std::vector<std::string>, std::vector<std::vector<std::string>>> {
@ -114,7 +170,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
set,
"set",
(std::tuple<std::set<std::string>, std::set<std::string>, std::set<std::string>, std::set<std::set<std::string>>> {
@ -125,7 +180,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
optionalStorePath,
"optional-store-path",
(std::tuple<std::optional<StorePath>, std::optional<StorePath>> {
@ -136,7 +190,6 @@ CHARACTERIZATION_TEST(
}))
CHARACTERIZATION_TEST(
CommonProtoTest,
optionalContentAddress,
"optional-content-address",
(std::tuple<std::optional<ContentAddress>, std::optional<ContentAddress>> {

45
src/libstore/tests/protocol.hh
View file

@ -9,26 +9,23 @@ namespace nix {
template<class Proto, const char * protocolDir>
class ProtoTest : public LibStoreTest
{
/**
* Read this as simply `using S = Inner::Serialise;`.
*
* See `LengthPrefixedProtoHelper::S` for the same trick, and its
* rationale.
*/
template<typename U> using S = typename Proto::template Serialise<U>;
public:
protected:
Path unitTestData = getUnitTestData() + "/libstore/" + protocolDir;
Path goldenMaster(std::string_view testStem) {
return unitTestData + "/" + testStem + ".bin";
}
};
template<class Proto, const char * protocolDir>
class VersionedProtoTest : public ProtoTest<Proto, protocolDir>
{
public:
/**
* Golden test for `T` reading
*/
template<typename T>
void readTest(PathView testStem, T value)
void readTest(PathView testStem, typename Proto::Version version, T value)
{
if (testAccept())
{
@ -36,13 +33,16 @@ public:
}
else
{
auto expected = readFile(goldenMaster(testStem));
auto expected = readFile(ProtoTest<Proto, protocolDir>::goldenMaster(testStem));
T got = ({
StringSource from { expected };
S<T>::read(
*store,
typename Proto::ReadConn { .from = from });
Proto::template Serialise<T>::read(
*LibStoreTest::store,
typename Proto::ReadConn {
.from = from,
.version = version,
});
});
ASSERT_EQ(got, value);
@ -53,14 +53,17 @@ public:
* Golden test for `T` write
*/
template<typename T>
void writeTest(PathView testStem, const T & value)
void writeTest(PathView testStem, typename Proto::Version version, const T & value)
{
auto file = goldenMaster(testStem);
auto file = ProtoTest<Proto, protocolDir>::goldenMaster(testStem);
StringSink to;
Proto::write(
*store,
typename Proto::WriteConn { .to = to },
*LibStoreTest::store,
typename Proto::WriteConn {
.to = to,
.version = version,
},
value);
if (testAccept())
@ -77,12 +80,12 @@ public:
}
};
#define CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VALUE) \
#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \
TEST_F(FIXTURE, NAME ## _read) { \
readTest(STEM, VALUE); \
readTest(STEM, VERSION, VALUE); \
} \
TEST_F(FIXTURE, NAME ## _write) { \
writeTest(STEM, VALUE); \
writeTest(STEM, VERSION, VALUE); \
}
}

38
src/libstore/tests/serve-protocol.cc
View file

@ -11,14 +11,22 @@
namespace nix {
const char commonProtoDir[] = "serve-protocol";
const char serveProtoDir[] = "serve-protocol";
using ServeProtoTest = ProtoTest<ServeProto, commonProtoDir>;
struct ServeProtoTest : VersionedProtoTest<ServeProto, serveProtoDir>
{
/**
* For serializers that don't care about the minimum version, we
* used the oldest one: 1.0.
*/
ServeProto::Version defaultVersion = 1 << 8 | 0;
};
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
string,
"string",
defaultVersion,
(std::tuple<std::string, std::string, std::string, std::string, std::string> {
"",
"hi",
@ -27,19 +35,21 @@ CHARACTERIZATION_TEST(
"oh no \0\0\0 what was that!",
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
storePath,
"store-path",
defaultVersion,
(std::tuple<StorePath, StorePath> {
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
contentAddress,
"content-address",
defaultVersion,
(std::tuple<ContentAddress, ContentAddress, ContentAddress> {
ContentAddress {
.method = TextIngestionMethod {},
@ -55,10 +65,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
drvOutput,
"drv-output",
defaultVersion,
(std::tuple<DrvOutput, DrvOutput> {
{
.drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
@ -70,10 +81,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
realisation,
"realisation",
defaultVersion,
(std::tuple<Realisation, Realisation> {
Realisation {
.id = DrvOutput {
@ -102,10 +114,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
vector,
"vector",
defaultVersion,
(std::tuple<std::vector<std::string>, std::vector<std::string>, std::vector<std::string>, std::vector<std::vector<std::string>>> {
{ },
{ "" },
@ -113,10 +126,11 @@ CHARACTERIZATION_TEST(
{ {}, { "" }, { "", "1", "2" } },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
set,
"set",
defaultVersion,
(std::tuple<std::set<std::string>, std::set<std::string>, std::set<std::string>, std::set<std::set<std::string>>> {
{ },
{ "" },
@ -124,10 +138,11 @@ CHARACTERIZATION_TEST(
{ {}, { "" }, { "", "1", "2" } },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
optionalStorePath,
"optional-store-path",
defaultVersion,
(std::tuple<std::optional<StorePath>, std::optional<StorePath>> {
std::nullopt,
std::optional {
@ -135,10 +150,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
ServeProtoTest,
optionalContentAddress,
"optional-content-address",
defaultVersion,
(std::tuple<std::optional<ContentAddress>, std::optional<ContentAddress>> {
std::nullopt,
std::optional {

49
src/libstore/tests/worker-protocol.cc
View file

@ -14,12 +14,21 @@ namespace nix {
const char workerProtoDir[] = "worker-protocol";
using WorkerProtoTest = ProtoTest<WorkerProto, workerProtoDir>;
struct WorkerProtoTest : VersionedProtoTest<WorkerProto, workerProtoDir>
{
/**
* For serializers that don't care about the minimum version, we
* used the oldest one: 1.0.
*/
WorkerProto::Version defaultVersion = 1 << 8 | 0;
};
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
string,
"string",
defaultVersion,
(std::tuple<std::string, std::string, std::string, std::string, std::string> {
"",
"hi",
@ -28,19 +37,21 @@ CHARACTERIZATION_TEST(
"oh no \0\0\0 what was that!",
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
storePath,
"store-path",
defaultVersion,
(std::tuple<StorePath, StorePath> {
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
contentAddress,
"content-address",
defaultVersion,
(std::tuple<ContentAddress, ContentAddress, ContentAddress> {
ContentAddress {
.method = TextIngestionMethod {},
@ -56,10 +67,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
derivedPath,
"derived-path",
defaultVersion,
(std::tuple<DerivedPath, DerivedPath> {
DerivedPath::Opaque {
.path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
@ -72,10 +84,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
drvOutput,
"drv-output",
defaultVersion,
(std::tuple<DrvOutput, DrvOutput> {
{
.drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
@ -87,10 +100,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
realisation,
"realisation",
defaultVersion,
(std::tuple<Realisation, Realisation> {
Realisation {
.id = DrvOutput {
@ -119,10 +133,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
buildResult,
"build-result",
defaultVersion,
({
using namespace std::literals::chrono_literals;
std::tuple<BuildResult, BuildResult, BuildResult> t {
@ -177,10 +192,11 @@ CHARACTERIZATION_TEST(
t;
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
keyedBuildResult,
"keyed-build-result",
defaultVersion,
({
using namespace std::literals::chrono_literals;
std::tuple<KeyedBuildResult, KeyedBuildResult/*, KeyedBuildResult*/> t {
@ -213,20 +229,22 @@ CHARACTERIZATION_TEST(
t;
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
optionalTrustedFlag,
"optional-trusted-flag",
defaultVersion,
(std::tuple<std::optional<TrustedFlag>, std::optional<TrustedFlag>, std::optional<TrustedFlag>> {
std::nullopt,
std::optional { Trusted },
std::optional { NotTrusted },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
vector,
"vector",
defaultVersion,
(std::tuple<std::vector<std::string>, std::vector<std::string>, std::vector<std::string>, std::vector<std::vector<std::string>>> {
{ },
{ "" },
@ -234,10 +252,11 @@ CHARACTERIZATION_TEST(
{ {}, { "" }, { "", "1", "2" } },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
set,
"set",
defaultVersion,
(std::tuple<std::set<std::string>, std::set<std::string>, std::set<std::string>, std::set<std::set<std::string>>> {
{ },
{ "" },
@ -245,10 +264,11 @@ CHARACTERIZATION_TEST(
{ {}, { "" }, { "", "1", "2" } },
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
optionalStorePath,
"optional-store-path",
defaultVersion,
(std::tuple<std::optional<StorePath>, std::optional<StorePath>> {
std::nullopt,
std::optional {
@ -256,10 +276,11 @@ CHARACTERIZATION_TEST(
},
}))
CHARACTERIZATION_TEST(
VERSIONED_CHARACTERIZATION_TEST(
WorkerProtoTest,
optionalContentAddress,
"optional-content-address",
defaultVersion,
(std::tuple<std::optional<ContentAddress>, std::optional<ContentAddress>> {
std::nullopt,
std::optional {

15
src/libstore/worker-protocol.hh
View file

@ -47,26 +47,29 @@ struct WorkerProto
*/
enum struct Op : uint64_t;
/**
* Version type for the protocol.
*
* @todo Convert to struct with separate major vs minor fields.
*/
using Version = unsigned int;
/**
* A unidirectional read connection, to be used by the read half of the
* canonical serializers below.
*
* This currently is just a `Source &`, but more fields will be added
* later.
*/
struct ReadConn {
Source & from;
Version version;
};
/**
* A unidirectional write connection, to be used by the write half of the
* canonical serializers below.
*
* This currently is just a `Sink &`, but more fields will be added
* later.
*/
struct WriteConn {
Sink & to;
Version version;
};
/**

12
src/nix-store/nix-store.cc
View file

@ -818,10 +818,16 @@ static void opServe(Strings opFlags, Strings opArgs)
if (magic != SERVE_MAGIC_1) throw Error("protocol mismatch");
out << SERVE_MAGIC_2 << SERVE_PROTOCOL_VERSION;
out.flush();
unsigned int clientVersion = readInt(in);
ServeProto::Version clientVersion = readInt(in);
ServeProto::ReadConn rconn { .from = in };
ServeProto::WriteConn wconn { .to = out };
ServeProto::ReadConn rconn {
.from = in,
.version = clientVersion,
};
ServeProto::WriteConn wconn {
.to = out,
.version = clientVersion,
};
auto getBuildSettings = [&]() {
// FIXME: changing options here doesn't work if we're