From d2c9c0119d513d7a77d6503ee272b3be78fbf9e0 Mon Sep 17 00:00:00 2001
From: Georgy Moshkin <gmoshkin@picodata.io>
Date: Wed, 28 Feb 2024 21:08:53 +0300
Subject: [PATCH] feat: validate configuration against storage
---
src/config.rs | 75 +++++++++++++++++++++++++++++++++++++++++
src/lib.rs | 3 +-
test/int/test_basics.py | 74 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/src/config.rs b/src/config.rs
index 8a7aebbfd5..04ddfbab08 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -195,6 +195,81 @@ impl PicodataConfig {
todo!()
}
+ /// Does validation of configuration parameters which are persisted in the
+ /// storage.
+ pub fn validate_storage(
+ &self,
+ storage: &storage::Clusterwide,
+ raft_storage: &RaftSpaceAccess,
+ ) -> Result<(), Error> {
+ // Cluster id
+ match (raft_storage.cluster_id()?, self.cluster_id()) {
+ (from_storage, from_config) if from_storage != from_config => {
+ return Err(Error::InvalidConfiguration(format!(
+ "instance restarted with a different `cluster_id`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
+ )));
+ }
+ _ => {}
+ }
+
+ // Instance id
+ let mut instance_id = None;
+ match (raft_storage.instance_id()?, &self.instance.instance_id) {
+ (Some(from_storage), Some(from_config)) if from_storage != from_config => {
+ return Err(Error::InvalidConfiguration(format!(
+ "instance restarted with a different `instance_id`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
+ )));
+ }
+ (Some(from_storage), _) => {
+ instance_id = Some(from_storage);
+ }
+ _ => {}
+ }
+
+ // Replicaset id
+ if let Some(instance_id) = &instance_id {
+ if let Ok(instance_info) = storage.instances.get(instance_id) {
+ match (&instance_info.replicaset_id, &self.instance.replicaset_id) {
+ (from_storage, Some(from_config)) if from_storage != from_config => {
+ return Err(Error::InvalidConfiguration(format!(
+ "instance restarted with a different `replicaset_id`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
+ )));
+ }
+ _ => {}
+ }
+ }
+ }
+
+ // Tier
+ match (&raft_storage.tier()?, &self.instance.tier) {
+ (Some(from_storage), Some(from_config)) if from_storage != from_config => {
+ return Err(Error::InvalidConfiguration(format!(
+ "instance restarted with a different `tier`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
+ )));
+ }
+ _ => {}
+ }
+
+ // Advertise address
+ if let Some(raft_id) = raft_storage.raft_id()? {
+ match (
+ storage.peer_addresses.get(raft_id)?,
+ &self.instance.advertise_address,
+ ) {
+ (Some(from_storage), Some(from_config))
+ if from_storage != from_config.to_host_port() =>
+ {
+ return Err(Error::InvalidConfiguration(format!(
+ "instance restarted with a different `advertise_address`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
+ )));
+ }
+ _ => {}
+ }
+ }
+
+ Ok(())
+ }
+
#[inline]
pub fn cluster_id(&self) -> &str {
match (&self.instance.cluster_id, &self.cluster.cluster_id) {
diff --git a/src/lib.rs b/src/lib.rs
index 1a45d4bb50..2ca5e1c224 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -609,7 +609,6 @@ fn start_discover(
cfg.listen = Some(config.instance.listen().to_host_port());
tarantool::set_cfg(&cfg);
- // TODO assert traft::Storage::instance_id == (null || args.instance_id)
if raft_storage.raft_id().unwrap().is_some() {
tarantool::set_cfg_field("read_only", true).unwrap();
return postjoin(config, storage, raft_storage);
@@ -844,6 +843,8 @@ fn postjoin(
) -> Result<(), Error> {
tlog!(Info, "entering post-join phase");
+ config.validate_storage(&storage, &raft_storage)?;
+
if let Some(config) = &config.instance.audit {
audit::init(config, &raft_storage);
}
diff --git a/test/int/test_basics.py b/test/int/test_basics.py
index e4ef86d7b0..38cde6353a 100644
--- a/test/int/test_basics.py
+++ b/test/int/test_basics.py
@@ -168,6 +168,80 @@ def test_graceful_stop(instance: Instance):
assert f.read()[-4:] == b"\xd5\x10\xad\xed"
+def test_config_storage_conflicts_on_restart(instance: Instance):
+ instance.terminate()
+
+ #
+ # Change cluster_id
+ #
+ was = instance.cluster_id # type: ignore
+ instance.cluster_id = "new-cluster-id"
+ assert instance.cluster_id != was
+ err = f"""\
+invalid configuration: instance restarted with a different `cluster_id`, which is not allowed, was: '{was}' became: 'new-cluster-id'
+""" # noqa: E501
+ crawler = log_crawler(instance, err)
+ instance.fail_to_start()
+ assert crawler.matched
+ instance.cluster_id = was
+
+ #
+ # Change instance_id
+ #
+ was = instance.instance_id # type: ignore
+ instance.instance_id = "new-instance-id"
+ assert instance.instance_id != was
+ err = f"""\
+invalid configuration: instance restarted with a different `instance_id`, which is not allowed, was: '{was}' became: 'new-instance-id'
+""" # noqa: E501
+ crawler = log_crawler(instance, err)
+ instance.fail_to_start()
+ assert crawler.matched
+ instance.instance_id = was
+
+ #
+ # Change tier
+ #
+ was = instance.tier # type: ignore
+ instance.tier = "new-tier"
+ assert instance.tier != was
+ err = """\
+invalid configuration: instance restarted with a different `tier`, which is not allowed, was: 'default' became: 'new-tier'
+""" # noqa: E501
+ crawler = log_crawler(instance, err)
+ instance.fail_to_start()
+ assert crawler.matched
+ instance.tier = was
+
+ #
+ # Change replicaset_id
+ #
+ was = instance.replicaset_id # type: ignore
+ instance.replicaset_id = "new-replicaset-id"
+ assert instance.replicaset_id != was
+ err = """\
+invalid configuration: instance restarted with a different `replicaset_id`, which is not allowed, was: 'r1' became: 'new-replicaset-id'
+""" # noqa: E501
+ crawler = log_crawler(instance, err)
+ instance.fail_to_start()
+ assert crawler.matched
+ instance.replicaset_id = was
+
+ #
+ # Change advertise address
+ #
+ was = instance.listen # type: ignore
+ instance.env["PICODATA_ADVERTISE"] = "example.com:1234"
+ assert instance.env["PICODATA_ADVERTISE"] != was
+ err = f"""\
+invalid configuration: instance restarted with a different `advertise_address`, which is not allowed, was: '{was}' became: 'example.com:1234'
+""" # noqa: E501
+ crawler = log_crawler(instance, err)
+ instance.fail_to_start()
+ assert crawler.matched
+ del instance.env["PICODATA_ADVERTISE"]
+
+
def test_whoami(instance: Instance):
assert instance.call("pico.whoami") == {
"raft_id": 1,
--
GitLab