Skip to content

Commit 8509e1d

Browse files
authored
feat: keyspace version validation (#235)
1 parent 69a1200 commit 8509e1d

8 files changed

Lines changed: 322 additions & 25 deletions

File tree

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
250806.0
1+
250807.0

crates/cluster/src/view.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,21 @@ impl<C: Config> View<C> {
142142
&self.node_operators
143143
}
144144

145-
/// Checks whether the provided [`keyspace`] version is compatible with
146-
/// current state of the cluster.
147-
pub fn validate_keyspace_version(&self, version: u64) -> bool {
145+
/// Returns the highest [`keyspace`] version of this [`Cluster`].
146+
pub fn keyspace_version(&self) -> u64 {
148147
if let Some(migration) = self.migration() {
149-
migration.keyspace().version() == version
148+
migration.keyspace().version()
150149
} else {
151-
self.keyspace().version() == version
150+
self.keyspace().version()
152151
}
153152
}
154153

154+
/// Checks whether the provided [`keyspace`] version is compatible with
155+
/// current state of the cluster.
156+
pub fn validate_keyspace_version(&self, version: u64) -> bool {
157+
self.keyspace_version() == version
158+
}
159+
155160
pub(super) fn require_no_migration(&self) -> Result<(), migration::InProgressError> {
156161
if let Some(migration) = self.migration() {
157162
return Err(migration::InProgressError(migration.id()));

crates/replication2/src/coordinator/mod.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,20 @@ fn hash(key: &[u8]) -> u64 {
9292
impl<C: Config> StorageApi for InboundConnection<C> {
9393
async fn execute_callback<Cb: Callback>(
9494
&self,
95-
operation: Operation<'_>,
95+
mut operation: Operation<'_>,
9696
callback: Cb,
9797
) -> Result<(), Cb::Error> {
98+
let cluster_view = &self.coordinator.cluster.view();
99+
100+
operation.set_keyspace_version(cluster_view.keyspace_version());
98101
let operation = &operation;
99102
let namespace = operation.namespace();
100103

101-
let is_authorized = self.coordinator.cluster.using_view(|view| {
102-
view.node_operators().is_authorized_client(
103-
&self.peer_id,
104-
&namespace.node_operator_id().into(),
105-
namespace.idx(),
106-
)
107-
});
104+
let is_authorized = cluster_view.node_operators().is_authorized_client(
105+
&self.peer_id,
106+
&namespace.node_operator_id().into(),
107+
namespace.idx(),
108+
);
108109

109110
if !is_authorized {
110111
return callback.send_result(&Err(Error::unauthorized())).await;
@@ -113,8 +114,6 @@ impl<C: Config> StorageApi for InboundConnection<C> {
113114
let key = hash(operation.key());
114115
let is_write = operation.is_write();
115116

116-
let cluster_view = &self.coordinator.cluster.view();
117-
118117
let mut primary_quorum = Quorum::new(cluster_view.primary_replica_set(key));
119118
let primary_replicas = primary_quorum.replica_set;
120119
let primary_replica_requests = primary_quorum

crates/replication2/src/coordinator/test.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,6 @@ impl super::Config for Config {
5959
type OutboundReplicaConnection = FakeStorage;
6060
}
6161

62-
impl StorageApi for Replica {
63-
async fn execute_ref(
64-
&self,
65-
operation: &storage_api::Operation<'_>,
66-
) -> storage_api::Result<operation::Output> {
67-
self.storage.execute_ref(operation).await
68-
}
69-
}
70-
7162
struct Context {
7263
config: Config,
7364

@@ -254,6 +245,24 @@ async fn operations_not_authorized_for_unauthorized_namespaces() {
254245
assert_eq!(res, Err(Error::unauthorized()));
255246
}
256247

248+
#[tokio::test]
249+
async fn sets_operation_keyspace_version() {
250+
let ctx = Context::new().await;
251+
252+
ctx.config
253+
.storage_registry
254+
.for_each(|_, storage| storage.expect_keyspace_version(0));
255+
256+
let get = operation::GetBorrowed {
257+
namespace: namespace(0, 0),
258+
key: b"test",
259+
keyspace_version: None,
260+
};
261+
262+
let res = ctx.conn.execute(operation::Borrowed::Get(get).into()).await;
263+
assert_eq!(res, Ok(operation::Output::Record(None)));
264+
}
265+
257266
#[tokio::test]
258267
async fn replicates_only_to_replica_set() {
259268
let ctx = Context::new().await;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ use {
66
storage_api::{operation, DataItem, Error, Operation, Result, StorageApi},
77
};
88

9+
#[cfg(test)]
10+
pub mod test;
11+
912
/// [`Replica`] config.
1013
pub trait Config: cluster::Config {
1114
/// Type of the outbound connection to the WCN Database.
@@ -68,6 +71,13 @@ impl<C: Config> StorageApi for InboundConnection<C> {
6871
async fn execute_ref(&self, operation: &Operation<'_>) -> Result<operation::Output> {
6972
// TODO: once we add signatures to write operations check them here
7073

74+
self.replica.cluster.using_view(|view| {
75+
operation
76+
.keyspace_version()
77+
.filter(|&version| view.validate_keyspace_version(version))
78+
.ok_or_else(Error::keyspace_version_mismatch)
79+
})?;
80+
7181
self.replica
7282
.database
7383
.execute_ref(operation)
@@ -80,6 +90,14 @@ impl<C: Config> StorageApi for InboundConnection<C> {
8090
keyrange: RangeInclusive<u64>,
8191
keyspace_version: u64,
8292
) -> storage_api::Result<impl Stream<Item = storage_api::Result<DataItem>> + Send> {
93+
self.replica.cluster.using_view(|view| {
94+
if !view.validate_keyspace_version(keyspace_version) {
95+
return Err(Error::keyspace_version_mismatch());
96+
}
97+
98+
Ok(())
99+
})?;
100+
83101
self.replica
84102
.database
85103
.read_data(keyrange, keyspace_version)
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
use {
2+
crate::{replica, Replica},
3+
cluster::{
4+
node_operator,
5+
smart_contract::{self, testing::FakeSmartContract},
6+
testing::node_peer_id,
7+
Cluster,
8+
Node,
9+
PeerId,
10+
},
11+
futures::TryStreamExt as _,
12+
std::{sync::Arc, time::Duration},
13+
storage_api::{
14+
operation,
15+
testing::FakeStorage,
16+
Error,
17+
Factory,
18+
Namespace,
19+
Record,
20+
RecordVersion,
21+
StorageApi,
22+
},
23+
};
24+
25+
#[derive(Clone, Default)]
26+
struct Config {
27+
smart_contract_registry: smart_contract::testing::FakeRegistry,
28+
database: FakeStorage,
29+
}
30+
31+
impl cluster::Config for Config {
32+
type SmartContract = FakeSmartContract;
33+
type KeyspaceShards = ();
34+
type Node = Node;
35+
36+
fn new_node(&self, _operator_id: node_operator::Id, node: Node) -> Node {
37+
node
38+
}
39+
}
40+
41+
impl super::Config for Config {
42+
type OutboundDatabaseConnection = FakeStorage;
43+
}
44+
45+
struct Context {
46+
config: Config,
47+
48+
replica: Replica<Config>,
49+
conn: replica::InboundConnection<Config>,
50+
}
51+
52+
impl Context {
53+
async fn new() -> Self {
54+
let cfg = Config::default();
55+
56+
let cluster = Cluster::deploy(
57+
cfg.clone(),
58+
&cfg.smart_contract_registry
59+
.deployer(smart_contract::testing::signer(42)),
60+
cluster::Settings {
61+
max_node_operator_data_bytes: 1024,
62+
},
63+
(0..8)
64+
.map(|idx| cluster::testing::node_operator(idx as u8))
65+
.collect(),
66+
)
67+
.await
68+
.unwrap();
69+
70+
let replica = Replica::new(Arc::new(cfg.clone()), cluster, cfg.database.clone());
71+
72+
Self {
73+
config: cfg,
74+
conn: replica.new_inbound_connection(node_peer_id(0, 0)).unwrap(),
75+
replica,
76+
}
77+
}
78+
}
79+
80+
fn namespace(operator_id: u8, idx: u8) -> Namespace {
81+
let operator_id = cluster::testing::node_operator(operator_id).id;
82+
format!("{operator_id}/{idx}").parse().unwrap()
83+
}
84+
85+
#[tokio::test]
86+
async fn inbound_connections_not_authorized_for_nodes_not_in_cluster() {
87+
let ctx = Context::new().await;
88+
let err = ctx.replica.new_storage_api(PeerId::random()).err().unwrap();
89+
90+
assert_eq!(err, Error::unauthorized())
91+
}
92+
93+
#[tokio::test]
94+
async fn errors_on_keyspace_version_mismatch() {
95+
let ctx = Context::new().await;
96+
97+
let get = operation::GetBorrowed {
98+
namespace: namespace(0, 0),
99+
key: b"test",
100+
keyspace_version: None,
101+
};
102+
103+
let res = ctx.conn.execute(operation::Borrowed::Get(get).into()).await;
104+
assert_eq!(res, Err(Error::keyspace_version_mismatch()));
105+
106+
let get = operation::GetBorrowed {
107+
namespace: namespace(0, 0),
108+
key: b"test",
109+
keyspace_version: Some(1),
110+
};
111+
112+
let res = ctx.conn.execute(operation::Borrowed::Get(get).into()).await;
113+
assert_eq!(res, Err(Error::keyspace_version_mismatch()));
114+
115+
let err = ctx.conn.read_data(0..=42, 1).await.err();
116+
assert_eq!(err, Some(Error::keyspace_version_mismatch()));
117+
}
118+
119+
#[tokio::test]
120+
async fn forwards_calls_to_database() {
121+
let ctx = Context::new().await;
122+
let ns = namespace(0, 0);
123+
124+
let set = operation::Set {
125+
namespace: ns,
126+
key: b"foo".into(),
127+
record: Record {
128+
value: b"bar".into(),
129+
expiration: Duration::from_secs(30).into(),
130+
version: RecordVersion::now(),
131+
},
132+
keyspace_version: None,
133+
};
134+
135+
let res = ctx.config.database.execute(set.clone().into()).await;
136+
assert_eq!(res, Ok(operation::Output::none()));
137+
138+
let get = operation::GetBorrowed {
139+
namespace: namespace(0, 0),
140+
key: b"foo",
141+
keyspace_version: Some(0),
142+
};
143+
144+
let res = ctx.conn.execute(operation::Borrowed::Get(get).into()).await;
145+
assert_eq!(res, Ok(operation::Output::Record(Some(set.record))));
146+
147+
let items: Vec<_> = ctx
148+
.conn
149+
.read_data(0..=u64::MAX, 0)
150+
.await
151+
.unwrap()
152+
.try_collect()
153+
.await
154+
.unwrap();
155+
156+
assert_eq!(items.len(), 2); // +1 `Done` frame
157+
}

0 commit comments

Comments
 (0)