cafkafk/lix
cafkafk/lix
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

Merge pull request #9157 from obsidiansystems/protocol-versions

Browse source 
Add protocol versions to `{Worker,Serve}Proto::*Conn`

(cherry picked from commit 4d17c59d8d059a5b39f1d1da2b58f2ec8da44861)
Change-Id: I497af39deb792e50c157a1305d8c9e722798740b
This commit is contained in:
Robert Hensing 2023-10-20 15:34:26 +02:00 committed by eldritch horrors
parent 5ddd1a9166
commit ab40b2c5d0
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(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;
};
/**

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

73
tests/unit/libstore/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
tests/unit/libstore/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
tests/unit/libstore/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
tests/unit/libstore/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 {