From 85e789a1eefd12d5638cd3c7cfc5afd744a722d9 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Mon, 24 Jan 2022 09:33:50 -0800 Subject: [PATCH 01/24] Upgrade deps --- Cargo.lock | 112 ++++++++++++++++++++++++++++++------------ server/Cargo.toml | 4 +- sky-bench/Cargo.toml | 4 +- sky-macros/Cargo.toml | 4 +- 4 files changed, 87 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c55f539..2b46588a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -112,9 +112,9 @@ dependencies = [ [[package]] name = "clipboard-win" -version = "4.3.0" +version = "4.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1951fb8aa063a2ee18b4b4d217e4aa2ec9cc4f2430482983f607fa10cd36d7aa" +checksum = "2f3e1238132dc01f081e1cbb9dace14e5ef4c3a51ee244bd982275fb514605db" dependencies = [ "error-code", "str-buf", @@ -248,11 +248,32 @@ dependencies = [ "termcolor", ] +[[package]] +name = "errno" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f639046355ee4f37944e44f60642c6f3a7efa3cf6b78c78a0d989a8ce6c396a1" +dependencies = [ + "errno-dragonfly", + "libc", + "winapi", +] + +[[package]] +name = "errno-dragonfly" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa68f1b12764fab894d2755d2518754e71b4fd80ecfb822714a1206c2aab39bf" +dependencies = [ + "cc", + "libc", +] + [[package]] name = "error-code" -version = "2.3.0" +version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5115567ac25674e0043e472be13d14e537f37ea8aa4bdc4aef0c89add1db1ff" +checksum = "64f18991e7bf11e7ffee451b5318b5c1a73c52d0d0ada6e5a3017c8c1ced6a21" dependencies = [ "libc", "str-buf", @@ -260,12 +281,12 @@ dependencies = [ [[package]] name = "fd-lock" -version = "3.0.2" +version = "3.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a16910e685088843d53132b04e0f10a571fdb193224fc589685b3ba1ce4cb03d" +checksum = "fcef756dea9cf3db5ce73759cf0467330427a786b47711b8d6c97620d718ceb9" dependencies = [ "cfg-if", - "libc", + "rustix", "windows-sys", ] @@ -358,6 +379,15 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "io-lifetimes" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6ef6787e7f0faedc040f95716bdd0e62bcfcf4ba93da053b62dea2691c13864" +dependencies = [ + "winapi", +] + [[package]] name = "itoa" version = "1.0.1" @@ -393,9 +423,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.112" +version = "0.2.113" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b03d17f364a3a042d5e5d46b053bbbf82c92c9430c592dd4c064dc6ee997125" +checksum = "eef78b64d87775463c549fbd80e19249ef436ea3bf1de2a1eb7e717ec7fab1e9" [[package]] name = "libsky" @@ -415,6 +445,12 @@ dependencies = [ "rayon", ] +[[package]] +name = "linux-raw-sys" +version = "0.0.37" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95f5690fef754d905294c56f7ac815836f2513af966aa47f2e07ac79be07827f" + [[package]] name = "lock_api" version = "0.4.5" @@ -633,9 +669,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.14" +version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47aa80447ce4daf1717500037052af176af5d38cc3e571d9ec1c7353fc10c87d" +checksum = "864d3e96a899863136fc6e99f3d7cae289dafe43bf2c5ac19b70df7210c0a145" dependencies = [ "proc-macro2", ] @@ -751,6 +787,20 @@ version = "0.6.25" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b" +[[package]] +name = "rustix" +version = "0.32.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cee647393af53c750e15dcbf7781cdd2e550b246bde76e46c326e7ea3c73773" +dependencies = [ + "bitflags", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys", + "winapi", +] + [[package]] name = "rustyline" version = "9.1.2" @@ -789,18 +839,18 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "serde" -version = "1.0.133" +version = "1.0.135" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97565067517b60e2d1ea8b268e59ce036de907ac523ad83a0475da04e818989a" +checksum = "2cf9235533494ea2ddcdb794665461814781c53f19d87b76e571a1c35acbad2b" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.133" +version = "1.0.135" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed201699328568d8d08208fdd080e3ff594e6c422e438b6705905da01005d537" +checksum = "8dcde03d87d4c973c04be249e7d8f0b35db1c848c487bd43032808e59dd8328d" dependencies = [ "proc-macro2", "quote", @@ -809,9 +859,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.75" +version = "1.0.78" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c059c05b48c5c0067d4b4b2b4f0732dd65feb52daf7e0ea09cd87e7dadc1af79" +checksum = "d23c1ba4cf0efd44be32017709280b32d1cea5c3f1275c3b6d9e8bc54f758085" dependencies = [ "itoa", "ryu", @@ -976,9 +1026,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" [[package]] name = "syn" -version = "1.0.85" +version = "1.0.86" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a684ac3dcd8913827e18cd09a68384ee66c1de24157e3c556c9ab16d85695fb7" +checksum = "8a65b3f4ffa0092e9887669db0eae07941f023991ab58ea44da8fe8e2d511c6b" dependencies = [ "proc-macro2", "quote", @@ -1161,9 +1211,9 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "windows-sys" -version = "0.28.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82ca39602d5cbfa692c4b67e3bcbb2751477355141c1ed434c94da4186836ff6" +checksum = "030b7ff91626e57a05ca64a07c481973cbb2db774e4852c9c7ca342408c6a99a" dependencies = [ "windows_aarch64_msvc", "windows_i686_gnu", @@ -1174,33 +1224,33 @@ dependencies = [ [[package]] name = "windows_aarch64_msvc" -version = "0.28.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52695a41e536859d5308cc613b4a022261a274390b25bd29dfff4bf08505f3c2" +checksum = "29277a4435d642f775f63c7d1faeb927adba532886ce0287bd985bffb16b6bca" [[package]] name = "windows_i686_gnu" -version = "0.28.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f54725ac23affef038fecb177de6c9bf065787c2f432f79e3c373da92f3e1d8a" +checksum = "1145e1989da93956c68d1864f32fb97c8f561a8f89a5125f6a2b7ea75524e4b8" [[package]] name = "windows_i686_msvc" -version = "0.28.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51d5158a43cc43623c0729d1ad6647e62fa384a3d135fd15108d37c683461f64" +checksum = "d4a09e3a0d4753b73019db171c1339cd4362c8c44baf1bcea336235e955954a6" [[package]] name = "windows_x86_64_gnu" -version = "0.28.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc31f409f565611535130cfe7ee8e6655d3fa99c1c61013981e491921b5ce954" +checksum = "8ca64fcb0220d58db4c119e050e7af03c69e6f4f415ef69ec1773d9aab422d5a" [[package]] name = "windows_x86_64_msvc" -version = "0.28.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f2b8c7cbd3bfdddd9ab98769f9746a7fad1bca236554cd032b78d768bc0e89f" +checksum = "08cabc9f0066848fef4bc6a1c1668e6efce38b661d2aeec75d18d8617eebb5f1" [[package]] name = "yaml-rust" diff --git a/server/Cargo.toml b/server/Cargo.toml index 9bb8c7e2..5fcdf82f 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -22,7 +22,7 @@ num_cpus = "1.13.1" openssl = { version = "0.10.38", features = ["vendored"] } parking_lot = "0.11.2" regex = "1.5.4" -serde = { version = "1.0.133", features = ["derive"] } +serde = { version = "1.0.135", features = ["derive"] } tokio = { version = "1.15.0", features = ["full"] } tokio-openssl = "0.6.3" toml = "0.5.8" @@ -51,7 +51,7 @@ rand = "0.8.4" tokio = { version = "1.15.0", features = ["test-util"] } [target.'cfg(unix)'.dependencies] # external deps -libc = "0.2.112" +libc = "0.2.113" [features] nightly = [] diff --git a/sky-bench/Cargo.toml b/sky-bench/Cargo.toml index 2eef5b35..1dce6daf 100644 --- a/sky-bench/Cargo.toml +++ b/sky-bench/Cargo.toml @@ -14,5 +14,5 @@ skytable = { git = "https://github.com/skytable/client-rust", branch = "next" } clap = { version = "2.34.0", features = ["yaml"] } devtimer = "4.0.1" rand = "0.8.4" -serde = { version = "1.0.133", features = ["derive"] } -serde_json = "1.0.75" +serde = { version = "1.0.135", features = ["derive"] } +serde_json = "1.0.78" diff --git a/sky-macros/Cargo.toml b/sky-macros/Cargo.toml index 4a44cfe2..6787f136 100644 --- a/sky-macros/Cargo.toml +++ b/sky-macros/Cargo.toml @@ -12,6 +12,6 @@ proc-macro = true [dependencies] # external deps proc-macro2 = "1.0.36" -quote = "1.0.14" +quote = "1.0.15" rand = "0.8.4" -syn = { version = "1.0.85", features = ["full"] } +syn = { version = "1.0.86", features = ["full"] } From 0b1ed6af6ee8284376bbff23876691b2eb2146f8 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Tue, 25 Jan 2022 09:03:03 -0800 Subject: [PATCH 02/24] Add `ErrorStack` definition --- server/src/config/eval.rs | 72 +++++++++++++++++++++++++++++++++++++++ server/src/config/mod.rs | 1 + 2 files changed, 73 insertions(+) create mode 100644 server/src/config/eval.rs diff --git a/server/src/config/eval.rs b/server/src/config/eval.rs new file mode 100644 index 00000000..91664a35 --- /dev/null +++ b/server/src/config/eval.rs @@ -0,0 +1,72 @@ +/* + * Created on Tue Jan 25 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use std::fmt; + +const EMSG_ENV: &str = "Environment"; +const EMSG_FILE: &str = "Configuration file"; +const EMSG_CLI: &str = "Command line arguments"; +const TAB: &str = " "; + +#[derive(Debug, PartialEq)] +pub struct ErrorStack { + stack: Vec, + init: &'static str, +} + +impl ErrorStack { + pub fn new(init: &'static str) -> Self { + Self { + init, + stack: Vec::new(), + } + } + pub fn epush(&mut self, e: impl ToString) { + self.stack.push(e.to_string()) + } +} + +impl fmt::Display for ErrorStack { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} errors:\n", self.init)?; + for err in self.stack.iter() { + write!(f, "{}- {}", TAB, err)?; + } + Ok(()) + } +} + +#[test] +fn errorstact_fmt() { + const EXPECTED: &str = "\ +Environment errors: + - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer +"; + let mut estk = ErrorStack::new(EMSG_ENV); + estk.epush("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); + let fmt = format!("{}\n", estk); + assert_eq!(fmt, EXPECTED); +} diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 8d89363d..1019a45a 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -38,6 +38,7 @@ mod macros; mod cfgenv; mod cfgerr; mod cfgfile; +mod eval; #[cfg(test)] mod tests; // self imports From bd56ee2db5c440555a38223236b261d4f945ea18 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 04:43:18 -0800 Subject: [PATCH 03/24] Add `FeedbackStack`, `ErrorStack` and `WarningStack` --- server/src/config/eval.rs | 85 ++++++++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 14 deletions(-) diff --git a/server/src/config/eval.rs b/server/src/config/eval.rs index 91664a35..fa51d39b 100644 --- a/server/src/config/eval.rs +++ b/server/src/config/eval.rs @@ -31,42 +31,99 @@ const EMSG_FILE: &str = "Configuration file"; const EMSG_CLI: &str = "Command line arguments"; const TAB: &str = " "; -#[derive(Debug, PartialEq)] -pub struct ErrorStack { +#[derive(Debug)] +struct FeedbackStack { stack: Vec, - init: &'static str, + feedback_type: &'static str, + feedback_source: &'static str, } -impl ErrorStack { - pub fn new(init: &'static str) -> Self { +impl FeedbackStack { + fn new(feedback_source: &'static str, feedback_type: &'static str) -> Self { Self { - init, stack: Vec::new(), + feedback_type, + feedback_source, } } - pub fn epush(&mut self, e: impl ToString) { - self.stack.push(e.to_string()) - } } -impl fmt::Display for ErrorStack { +impl fmt::Display for FeedbackStack { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} errors:\n", self.init)?; + write!(f, "{} {}:\n", self.feedback_source, self.feedback_type)?; for err in self.stack.iter() { - write!(f, "{}- {}", TAB, err)?; + writeln!(f, "{}- {}", TAB, err)?; } Ok(()) } } +#[derive(Debug)] +pub struct ErrorStack { + feedback: FeedbackStack, +} + +impl ErrorStack { + pub fn new(err_source: &'static str) -> Self { + Self { + feedback: FeedbackStack::new(err_source, "errors"), + } + } + pub fn epush(&mut self, e: impl ToString) { + self.feedback.stack.push(e.to_string()) + } +} + +impl fmt::Display for ErrorStack { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.feedback) + } +} + #[test] -fn errorstact_fmt() { +fn errorstack_fmt() { const EXPECTED: &str = "\ Environment errors: - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer "; let mut estk = ErrorStack::new(EMSG_ENV); estk.epush("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); - let fmt = format!("{}\n", estk); + let fmt = format!("{}", estk); + assert_eq!(fmt, EXPECTED); +} + +#[derive(Debug)] +pub struct WarningStack { + feedback: FeedbackStack, +} + +impl WarningStack { + pub fn new(warning_source: &'static str) -> Self { + Self { + feedback: FeedbackStack::new(warning_source, "warnings"), + } + } + pub fn wpush(&mut self, w: impl ToString) { + self.feedback.stack.push(w.to_string()) + } +} + +impl fmt::Display for WarningStack { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.feedback) + } +} + +#[test] +fn warningstack_fmt() { + const EXPECTED: &str = "\ +Environment warnings: + - BGSAVE is disabled. You may lose data if the host crashes + - The setting for `maxcon` is too high +"; + let mut wstk = WarningStack::new(EMSG_ENV); + wstk.wpush("BGSAVE is disabled. You may lose data if the host crashes"); + wstk.wpush("The setting for `maxcon` is too high"); + let fmt = format!("{}", wstk); assert_eq!(fmt, EXPECTED); } From 09afcd3e744c41628033608402a351e5b08fce7f Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 07:40:08 -0800 Subject: [PATCH 04/24] Add new config impl for server section --- server/src/config/cfg2.rs | 321 ++++++++++++++++++++++++++++++++++++++ server/src/config/eval.rs | 47 ++++-- server/src/config/mod.rs | 2 + 3 files changed, 360 insertions(+), 10 deletions(-) create mode 100644 server/src/config/cfg2.rs diff --git a/server/src/config/cfg2.rs b/server/src/config/cfg2.rs new file mode 100644 index 00000000..4c9d2f47 --- /dev/null +++ b/server/src/config/cfg2.rs @@ -0,0 +1,321 @@ +/* + * Created on Thu Jan 27 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use super::eval::{ErrorStack, WarningStack}; +use super::{ConfigurationSet, PortConfig}; +use core::str::FromStr; +use std::env::VarError; +use std::net::IpAddr; + +type StaticStr = &'static str; + +#[derive(Debug)] +pub enum ConfigSourceParseResult { + Okay(T), + Absent, + ParseFailure, +} + +pub trait TryFromConfigSource: Sized { + /// Check if the value is present + fn is_present(&self) -> bool; + /// Attempt to mutate the value if present. If any error occurs + /// while parsing the value, return true. Else return false if all went well + fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool; + fn try_parse(self) -> ConfigSourceParseResult; +} + +impl<'a, T: FromStr + 'a> TryFromConfigSource for Option<&'a str> { + fn is_present(&self) -> bool { + self.is_some() + } + fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { + self.map(|slf| { + *trip = true; + match slf.parse() { + Ok(p) => { + *target_value = p; + false + } + Err(_) => true, + } + }) + .unwrap_or(false) + } + fn try_parse(self) -> ConfigSourceParseResult { + self.map(|s| { + s.parse() + .map(|ret| ConfigSourceParseResult::Okay(ret)) + .unwrap_or(ConfigSourceParseResult::ParseFailure) + }) + .unwrap_or(ConfigSourceParseResult::Absent) + } +} + +impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { + fn is_present(&self) -> bool { + !matches!(self, Err(VarError::NotPresent)) + } + fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { + match self { + Ok(s) => s + .parse() + .map(|v| { + *trip = true; + *target_value = v; + false + }) + .unwrap_or(true), + Err(e) => { + if matches!(e, VarError::NotPresent) { + false + } else { + // yes, we failed to get the var but failed to parse it into unicode + *trip = true; + true + } + } + } + } + fn try_parse(self) -> ConfigSourceParseResult { + match self { + Ok(s) => s + .parse() + .map(|v| ConfigSourceParseResult::Okay(v)) + .unwrap_or(ConfigSourceParseResult::ParseFailure), + Err(e) => match e { + VarError::NotPresent => ConfigSourceParseResult::Absent, + VarError::NotUnicode(_) => ConfigSourceParseResult::ParseFailure, + }, + } + } +} + +#[derive(Debug)] +pub struct Configset { + did_mutate: bool, + cfg: ConfigurationSet, + estack: ErrorStack, + wstack: WarningStack, +} + +impl Configset { + const EMSG_ENV: StaticStr = "Environment"; + const EMSG_CLI: StaticStr = "CLI arguments"; + const EMSG_FILE: StaticStr = "Configuration file"; + + fn _new(feedback_source: StaticStr) -> Self { + Self { + did_mutate: false, + cfg: ConfigurationSet::default(), + estack: ErrorStack::new(feedback_source), + wstack: WarningStack::new(feedback_source), + } + } + pub fn new_env() -> Self { + Self::_new(Self::EMSG_ENV) + } + pub fn new_cli() -> Self { + Self::_new(Self::EMSG_CLI) + } + pub fn new_file() -> Self { + Self::_new(Self::EMSG_FILE) + } + fn mutated(&mut self) { + self.did_mutate = true; + } + fn epush(&mut self, field_key: StaticStr, expected: StaticStr) { + self.estack.push(format!( + "Bad value for `${field_key}`. Expected ${expected}", + )) + } + pub fn is_okay(&self) -> bool { + self.estack.is_empty() + } + pub fn is_mutated(&self) -> bool { + self.did_mutate + } + pub fn into_result(self) -> Result, ErrorStack> { + let Self { + wstack, + estack, + cfg, + did_mutate, + } = self; + log::warn!("{}", wstack); + if did_mutate { + if estack.is_empty() { + Ok(Some(cfg)) + } else { + Err(estack) + } + } else { + Ok(None) + } + } + fn try_mutate( + &mut self, + new: impl TryFromConfigSource, + target: &mut T, + field_key: StaticStr, + expected: StaticStr, + ) { + if new.mutate_failed(target, &mut self.did_mutate) { + self.epush(field_key, expected) + } + } + fn try_mutate_with_condcheck( + &mut self, + new: impl TryFromConfigSource, + target: &mut T, + field_key: StaticStr, + expected: StaticStr, + validation_fn: F, + ) where + F: Fn(&T) -> bool, + { + let mut needs_error = false; + match new.try_parse() { + ConfigSourceParseResult::Okay(ok) => { + self.mutated(); + needs_error = !validation_fn(&ok); + *target = ok; + } + ConfigSourceParseResult::ParseFailure => needs_error = true, + ConfigSourceParseResult::Absent => {} + } + if needs_error { + self.epush(field_key, expected) + } + } +} + +// server settings +impl Configset { + pub fn server_tcp( + &mut self, + nhost: impl TryFromConfigSource, + nhost_key: StaticStr, + nport: impl TryFromConfigSource, + nport_key: StaticStr, + ) { + let mut host = super::DEFAULT_IPV4; + let mut port = super::DEFAULT_PORT; + self.try_mutate(nhost, &mut host, nhost_key, "an IPv4/IPv6 address"); + self.try_mutate(nport, &mut port, nport_key, "a 16-bit positive integer"); + self.cfg.ports = PortConfig::new_insecure_only(host, port); + } + pub fn server_noart(&mut self, nart: impl TryFromConfigSource, nart_key: StaticStr) { + let mut noart = false; + self.try_mutate(nart, &mut noart, nart_key, "true/false"); + self.cfg.noart = noart; + } + pub fn server_maxcon( + &mut self, + nmaxcon: impl TryFromConfigSource, + nmaxcon_key: StaticStr, + ) { + let mut maxcon = super::MAXIMUM_CONNECTION_LIMIT; + self.try_mutate_with_condcheck( + nmaxcon, + &mut maxcon, + nmaxcon_key, + "a positive integer greater than one", + |max| *max > 0, + ); + self.cfg.maxcon = maxcon; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::DEFAULT_IPV4; + #[test] + fn server_tcp() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("127.0.0.1"), + "SKY_SERVER_HOST", + Some("2004"), + "SKY_SERVER_PORT", + ); + assert_eq!( + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2004) + ); + assert!(cfgset.is_mutated()); + assert!(cfgset.is_okay()); + } + #[test] + fn server_tcp_fail_host() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("?127.0.0.1"), + "SKY_SERVER_HOST", + Some("2004"), + "SKY_SERVER_PORT", + ); + assert_eq!( + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2004) + ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + } + #[test] + fn server_tcp_fail_port() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("127.0.0.1"), + "SKY_SERVER_HOST", + Some("65537"), + "SKY_SERVER_PORT", + ); + assert_eq!( + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2003) + ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + } + #[test] + fn server_tcp_fail_both() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("?127.0.0.1"), + "SKY_SERVER_HOST", + Some("65537"), + "SKY_SERVER_PORT", + ); + assert_eq!( + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2003) + ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + } +} diff --git a/server/src/config/eval.rs b/server/src/config/eval.rs index fa51d39b..2ec92a2d 100644 --- a/server/src/config/eval.rs +++ b/server/src/config/eval.rs @@ -24,7 +24,8 @@ * */ -use std::fmt; +use core::fmt; +use core::ops; const EMSG_ENV: &str = "Environment"; const EMSG_FILE: &str = "Configuration file"; @@ -32,7 +33,7 @@ const EMSG_CLI: &str = "Command line arguments"; const TAB: &str = " "; #[derive(Debug)] -struct FeedbackStack { +pub struct FeedbackStack { stack: Vec, feedback_type: &'static str, feedback_source: &'static str, @@ -46,6 +47,12 @@ impl FeedbackStack { feedback_source, } } + pub fn push(&mut self, f: impl ToString) { + self.stack.push(f.to_string()) + } + pub fn is_empty(&self) -> bool { + self.stack.is_empty() + } } impl fmt::Display for FeedbackStack { @@ -69,9 +76,6 @@ impl ErrorStack { feedback: FeedbackStack::new(err_source, "errors"), } } - pub fn epush(&mut self, e: impl ToString) { - self.feedback.stack.push(e.to_string()) - } } impl fmt::Display for ErrorStack { @@ -80,6 +84,19 @@ impl fmt::Display for ErrorStack { } } +impl ops::Deref for ErrorStack { + type Target = FeedbackStack; + fn deref(&self) -> &Self::Target { + &self.feedback + } +} + +impl ops::DerefMut for ErrorStack { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.feedback + } +} + #[test] fn errorstack_fmt() { const EXPECTED: &str = "\ @@ -87,7 +104,7 @@ Environment errors: - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer "; let mut estk = ErrorStack::new(EMSG_ENV); - estk.epush("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); + estk.push("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); let fmt = format!("{}", estk); assert_eq!(fmt, EXPECTED); } @@ -103,8 +120,18 @@ impl WarningStack { feedback: FeedbackStack::new(warning_source, "warnings"), } } - pub fn wpush(&mut self, w: impl ToString) { - self.feedback.stack.push(w.to_string()) +} + +impl ops::Deref for WarningStack { + type Target = FeedbackStack; + fn deref(&self) -> &Self::Target { + &self.feedback + } +} + +impl ops::DerefMut for WarningStack { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.feedback } } @@ -122,8 +149,8 @@ Environment warnings: - The setting for `maxcon` is too high "; let mut wstk = WarningStack::new(EMSG_ENV); - wstk.wpush("BGSAVE is disabled. You may lose data if the host crashes"); - wstk.wpush("The setting for `maxcon` is too high"); + wstk.push("BGSAVE is disabled. You may lose data if the host crashes"); + wstk.push("The setting for `maxcon` is too high"); let fmt = format!("{}", wstk); assert_eq!(fmt, EXPECTED); } diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 1019a45a..306d3359 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -38,7 +38,9 @@ mod macros; mod cfgenv; mod cfgerr; mod cfgfile; +// TODO: Upgrade to use these modules mod eval; +mod cfg2; #[cfg(test)] mod tests; // self imports From d46d73301b776db8b43da72ae3e8877d38dbfabf Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 09:15:18 -0800 Subject: [PATCH 05/24] Add config impls for `bgsave`, `snapshot` and `ssl` --- server/src/config/cfg2.rs | 216 +++++++++++++++++++++++++++++++++++++- server/src/config/mod.rs | 23 +++- 2 files changed, 234 insertions(+), 5 deletions(-) diff --git a/server/src/config/cfg2.rs b/server/src/config/cfg2.rs index 4c9d2f47..6ced0f24 100644 --- a/server/src/config/cfg2.rs +++ b/server/src/config/cfg2.rs @@ -25,7 +25,7 @@ */ use super::eval::{ErrorStack, WarningStack}; -use super::{ConfigurationSet, PortConfig}; +use super::{BGSave, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, SslOpts}; use core::str::FromStr; use std::env::VarError; use std::net::IpAddr; @@ -33,18 +33,27 @@ use std::net::IpAddr; type StaticStr = &'static str; #[derive(Debug)] +/// An enum representing the outcome of a parse operation for a specific configuration item from a +/// specific configuration source pub enum ConfigSourceParseResult { Okay(T), Absent, ParseFailure, } +/// A trait for configuration sources. Any type implementing this trait is considered to be a valid +/// source for configuration pub trait TryFromConfigSource: Sized { /// Check if the value is present fn is_present(&self) -> bool; /// Attempt to mutate the value if present. If any error occurs - /// while parsing the value, return true. Else return false if all went well + /// while parsing the value, return true. Else return false if all went well. + /// Here: + /// - `target_value`: is a mutable reference to the target var + /// - `trip`: is a mutable ref to a bool that will be set to true if a value is present + /// (whether parseable or not) fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool; + /// Attempt to parse the value into the target type fn try_parse(self) -> ConfigSourceParseResult; } @@ -93,7 +102,7 @@ impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { if matches!(e, VarError::NotPresent) { false } else { - // yes, we failed to get the var but failed to parse it into unicode + // yes, we got the var but failed to parse it into unicode; so trip *trip = true; true } @@ -115,6 +124,31 @@ impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { } #[derive(Debug)] +pub struct OptString { + base: Option, +} + +impl OptString { + pub const fn new_null() -> Self { + Self { base: None } + } + pub fn finish(self) -> Option { + self.base + } +} + +impl FromStr for OptString { + type Err = (); + fn from_str(st: &str) -> Result { + Ok(Self { + base: Some(st.to_string()), + }) + } +} + +#[derive(Debug)] +/// A high-level configuration set that automatically handles errors, warnings and provides a convenient [`Result`] +/// type that can be used pub struct Configset { did_mutate: bool, cfg: ConfigurationSet, @@ -127,6 +161,8 @@ impl Configset { const EMSG_CLI: StaticStr = "CLI arguments"; const EMSG_FILE: StaticStr = "Configuration file"; + /// Internal ctor for a given feedback source. We do not want to expose this to avoid + /// erroneous feedback source names fn _new(feedback_source: StaticStr) -> Self { Self { did_mutate: false, @@ -135,29 +171,37 @@ impl Configset { wstack: WarningStack::new(feedback_source), } } + /// Create a new configset for environment variables pub fn new_env() -> Self { Self::_new(Self::EMSG_ENV) } + /// Create a new configset for CLI args pub fn new_cli() -> Self { Self::_new(Self::EMSG_CLI) } + /// Create a new configset for a config file pub fn new_file() -> Self { Self::_new(Self::EMSG_FILE) } + /// Mark the configset mutated fn mutated(&mut self) { self.did_mutate = true; } + /// Push an error onto the error stack fn epush(&mut self, field_key: StaticStr, expected: StaticStr) { self.estack.push(format!( "Bad value for `${field_key}`. Expected ${expected}", )) } + /// Check if no errors have occurred pub fn is_okay(&self) -> bool { self.estack.is_empty() } + /// Check if the configset was mutated pub fn is_mutated(&self) -> bool { self.did_mutate } + /// Turns self into a Result pub fn into_result(self) -> Result, ErrorStack> { let Self { wstack, @@ -176,6 +220,8 @@ impl Configset { Ok(None) } } + /// Attempt to mutate with a target `TryFromConfigSource` type, and push in any error that occurs + /// using the given diagnostic info fn try_mutate( &mut self, new: impl TryFromConfigSource, @@ -187,6 +233,9 @@ impl Configset { self.epush(field_key, expected) } } + /// Attempt to mutate with a target `TryFromConfigSource` type, and push in any error that occurs + /// using the given diagnostic info while checking the correctly parsed type using the provided validation + /// closure for any additional validation check that goes beyond type correctness fn try_mutate_with_condcheck( &mut self, new: impl TryFromConfigSource, @@ -243,13 +292,172 @@ impl Configset { nmaxcon, &mut maxcon, nmaxcon_key, - "a positive integer greater than one", + "a positive integer greater than zero", |max| *max > 0, ); self.cfg.maxcon = maxcon; } } +// bgsave settings +impl Configset { + pub fn bgsave_settings( + &mut self, + nenabled: impl TryFromConfigSource, + nenabled_key: StaticStr, + nduration: impl TryFromConfigSource, + nduration_key: StaticStr, + ) { + let mut enabled = true; + let mut duration = 120; + let has_custom_duration = nduration.is_present(); + self.try_mutate(nenabled, &mut enabled, nenabled_key, "true/false"); + self.try_mutate_with_condcheck( + nduration, + &mut duration, + nduration_key, + "a positive integer greater than zero", + |dur| *dur > 0, + ); + if enabled { + self.cfg.bgsave = BGSave::Enabled(duration); + } else { + if has_custom_duration { + self.wstack.push(format!( + "Specifying ${nduration_key} is useless when BGSAVE is disabled" + )); + } + self.wstack + .push("BGSAVE is disabled. You may lose data if the host crashes"); + } + } +} + +// snapshot settings +impl Configset { + pub fn snasphot_settings( + &mut self, + nevery: impl TryFromConfigSource, + nevery_key: StaticStr, + natmost: impl TryFromConfigSource, + natmost_key: StaticStr, + nfailsafe: impl TryFromConfigSource, + nfailsafe_key: StaticStr, + ) { + match (nevery.is_present(), natmost.is_present()) { + (false, false) => { + // noice, disabled + if nfailsafe.is_present() { + // this mutation is pointless, but it is just for the sake of making sure + // that the `failsafe` key has a proper boolean, no matter if it is pointless + let mut _failsafe = true; + self.try_mutate(nfailsafe, &mut _failsafe, nfailsafe_key, "true/false"); + self.wstack.push(format!( + "Specifying ${nfailsafe_key} is usless when snapshots are disabled" + )); + } + } + (true, true) => { + let mut every = 0; + let mut atmost = 0; + let mut failsafe = true; + self.try_mutate_with_condcheck( + nevery, + &mut every, + nevery_key, + "an integer greater than 0", + |dur| *dur > 0, + ); + self.try_mutate( + natmost, + &mut atmost, + natmost_key, + "a positive integer. 0 indicates that all snapshots will be kept", + ); + self.try_mutate(nfailsafe, &mut failsafe, nfailsafe_key, "true/false"); + self.cfg.snapshot = + SnapshotConfig::Enabled(SnapshotPref::new(every, atmost, failsafe)); + } + (false, true) | (true, false) => self.estack.push(format!( + "To use snapshots, pass values for both ${nevery_key} and ${natmost_key}" + )), + } + } +} + +// TLS settings +impl Configset { + pub fn tls_settings( + &mut self, + nkey: impl TryFromConfigSource, + nkey_key: StaticStr, + ncert: impl TryFromConfigSource, + ncert_key: StaticStr, + nport: impl TryFromConfigSource, + nport_key: StaticStr, + nonly: impl TryFromConfigSource, + nonly_key: StaticStr, + npass: impl TryFromConfigSource, + npass_key: StaticStr, + ) { + match (nkey.is_present(), ncert.is_present()) { + (true, true) => { + // get the cert details + let mut key = String::new(); + let mut cert = String::new(); + self.try_mutate(nkey, &mut key, nkey_key, "path to private key file"); + self.try_mutate(ncert, &mut cert, ncert_key, "path to TLS certificate file"); + + // now get port info + let mut port = super::DEFAULT_SSL_PORT; + self.try_mutate(nport, &mut port, nport_key, "a positive 16-bit integer"); + + // now check if TLS only + let mut tls_only = false; + self.try_mutate(nonly, &mut tls_only, nonly_key, "true/false"); + + // check if we have a TLS cert + let mut tls_pass = OptString::new_null(); + self.try_mutate( + npass, + &mut tls_pass, + npass_key, + "path to TLS cert passphrase", + ); + + let sslopts = SslOpts::new(key, cert, port, tls_pass.finish()); + // now check if TLS only + if tls_only { + let host = self.cfg.ports.get_host(); + self.cfg.ports = PortConfig::new_secure_only(host, sslopts) + } else { + // multi. go and upgrade existing + self.cfg.ports.upgrade_to_tls(sslopts); + } + } + (true, false) | (false, true) => { + self.estack.push(format!( + "To use TLS, pass values for both `${nkey_key}` and `${ncert_key}`" + )); + } + (false, false) => { + if nport.is_present() { + self.wstack + .push("Specifying `${nport_key}` is pointless when TLS is disabled"); + } + if nonly.is_present() { + self.wstack + .push("Specifying `${nonly_key}` is pointless when TLS is disabled"); + } + if npass.is_present() { + self.wstack + .push("Specifying `${npass_key}` is pointless when TLS is disabled"); + } + } + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 306d3359..b0699ed8 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -39,8 +39,8 @@ mod cfgenv; mod cfgerr; mod cfgfile; // TODO: Upgrade to use these modules -mod eval; mod cfg2; +mod eval; #[cfg(test)] mod tests; // self imports @@ -129,6 +129,27 @@ impl PortConfig { pub const fn new_multi(host: IpAddr, port: u16, ssl: SslOpts) -> Self { PortConfig::Multi { host, port, ssl } } + pub fn get_host(&self) -> IpAddr { + match self { + Self::InsecureOnly { host, .. } + | Self::SecureOnly { host, .. } + | Self::Multi { host, .. } => *host, + } + } + pub fn upgrade_to_tls(&mut self, ssl: SslOpts) { + match self { + Self::InsecureOnly { host, port } => { + *self = Self::Multi { + host: *host, + port: *port, + ssl, + } + } + Self::SecureOnly { .. } | Self::Multi { .. } => { + panic!("Port config is already upgraded to TLS") + } + } + } } #[derive(Deserialize, Debug, PartialEq)] From 3861a32c2e19a4caf2a03ddb3fda10d7f600a475 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 09:47:16 -0800 Subject: [PATCH 06/24] Rewrite `cfgenv` using new config impl --- server/src/config/cfgenv2.rs | 70 ++++++++++++++++++++++++++++++++++++ server/src/config/mod.rs | 1 + 2 files changed, 71 insertions(+) create mode 100644 server/src/config/cfgenv2.rs diff --git a/server/src/config/cfgenv2.rs b/server/src/config/cfgenv2.rs new file mode 100644 index 00000000..af10ed0a --- /dev/null +++ b/server/src/config/cfgenv2.rs @@ -0,0 +1,70 @@ +/* + * Created on Thu Jan 27 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use super::cfg2::Configset; + +/// Returns the environment configuration +pub(super) fn get_env_config() -> Configset { + let mut defset = Configset::new_env(); + macro_rules! fenv { + ( + $fn:ident, + $( + $field:ident + ),* + ) => { + defset.$fn( + $( + ::std::env::var(stringify!($field)), + stringify!($field), + )* + ); + }; + } + // server settings + fenv!(server_tcp, SKY_SYSTEM_HOST, SKY_SYSTEM_PORT); + fenv!(server_noart, SKY_SYSTEM_NOART); + fenv!(server_maxcon, SKY_SYSTEM_MAXCON); + // bgsave settings + fenv!(bgsave_settings, SKY_BGSAVE_ENABLED, SKY_BGSAVE_DURATION); + // snapshot settings + fenv!( + snasphot_settings, + SKY_SNAPSHOT_DURATION, + SKY_SNAPSHOT_KEEP, + SKY_SNAPSHOT_FAILSAFE + ); + // TLS settings + fenv!( + tls_settings, + SKY_TLS_KEY, + SKY_TLS_CERT, + SKY_TLS_PORT, + SKY_TLS_ONLY, + SKY_TLS_PASSIN + ); + defset +} diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index b0699ed8..c4ef85a7 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -40,6 +40,7 @@ mod cfgerr; mod cfgfile; // TODO: Upgrade to use these modules mod cfg2; +mod cfgenv2; mod eval; #[cfg(test)] mod tests; From a48b3fd4232f3b4be2df860d8c1a5ec81c66972a Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 19:17:57 -0800 Subject: [PATCH 07/24] Rewrite CLI config using new config impl --- server/src/config/cfg2.rs | 8 +-- server/src/config/cfgcli2.rs | 110 +++++++++++++++++++++++++++++++++++ server/src/config/cfgenv2.rs | 2 +- server/src/config/mod.rs | 1 + 4 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 server/src/config/cfgcli2.rs diff --git a/server/src/config/cfg2.rs b/server/src/config/cfg2.rs index 6ced0f24..2c14bcae 100644 --- a/server/src/config/cfg2.rs +++ b/server/src/config/cfg2.rs @@ -324,7 +324,7 @@ impl Configset { } else { if has_custom_duration { self.wstack.push(format!( - "Specifying ${nduration_key} is useless when BGSAVE is disabled" + "Specifying `${nduration_key}` is useless when BGSAVE is disabled" )); } self.wstack @@ -335,7 +335,7 @@ impl Configset { // snapshot settings impl Configset { - pub fn snasphot_settings( + pub fn snapshot_settings( &mut self, nevery: impl TryFromConfigSource, nevery_key: StaticStr, @@ -353,7 +353,7 @@ impl Configset { let mut _failsafe = true; self.try_mutate(nfailsafe, &mut _failsafe, nfailsafe_key, "true/false"); self.wstack.push(format!( - "Specifying ${nfailsafe_key} is usless when snapshots are disabled" + "Specifying `${nfailsafe_key}` is usless when snapshots are disabled" )); } } @@ -379,7 +379,7 @@ impl Configset { SnapshotConfig::Enabled(SnapshotPref::new(every, atmost, failsafe)); } (false, true) | (true, false) => self.estack.push(format!( - "To use snapshots, pass values for both ${nevery_key} and ${natmost_key}" + "To use snapshots, pass values for both `${nevery_key}` and `${natmost_key}`" )), } } diff --git a/server/src/config/cfgcli2.rs b/server/src/config/cfgcli2.rs new file mode 100644 index 00000000..9eb71da0 --- /dev/null +++ b/server/src/config/cfgcli2.rs @@ -0,0 +1,110 @@ +/* + * Created on Fri Jan 28 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use super::cfg2::{ConfigSourceParseResult, Configset, TryFromConfigSource}; +use clap::ArgMatches; + +#[derive(Clone, Copy)] +struct Flag { + flag_set: bool, +} + +impl Flag { + const fn new(flag_set: bool) -> Self { + Self { flag_set } + } +} + +impl TryFromConfigSource for Flag { + fn is_present(&self) -> bool { + self.flag_set + } + fn mutate_failed(self, target_value: &mut bool, trip: &mut bool) -> bool { + if self.is_present() { + *trip = true; + *target_value = true; + } + true + } + fn try_parse(self) -> ConfigSourceParseResult { + ConfigSourceParseResult::Okay(self.flag_set) + } +} + +pub(super) fn parse_cli_args(matches: ArgMatches) -> Configset { + let mut defset = Configset::new_cli(); + macro_rules! fcli { + ($fn:ident, $($source:expr, $key:literal),*) => { + defset.$fn( + $( + $source, + $key, + )* + ) + }; + } + // server settings + fcli!( + server_tcp, + matches.value_of("host"), + "--host", + matches.value_of("port"), + "--port" + ); + // bgsave settings + fcli!( + bgsave_settings, + Flag::new(!matches.is_present("nosave")), + "--nosave", + matches.value_of("saveduration"), + "--saveduration" + ); + // snapshot settings + fcli!( + snapshot_settings, + matches.value_of("snapevery"), + "--snapevery", + matches.value_of("snapkeep"), + "--snapkeep", + matches.value_of("stop-write-on-fail"), + "--stop-write-on-fail" + ); + // TLS settings + fcli!( + tls_settings, + matches.value_of("sslkey"), + "--sslkey", + matches.value_of("sslchain"), + "--sslchain", + matches.value_of("sslport"), + "--sslport", + Flag::new(matches.is_present("sslonly")), + "--sslonly", + matches.value_of("tlspass"), + "--tlspassin" + ); + defset +} diff --git a/server/src/config/cfgenv2.rs b/server/src/config/cfgenv2.rs index af10ed0a..59fa2e14 100644 --- a/server/src/config/cfgenv2.rs +++ b/server/src/config/cfgenv2.rs @@ -52,7 +52,7 @@ pub(super) fn get_env_config() -> Configset { fenv!(bgsave_settings, SKY_BGSAVE_ENABLED, SKY_BGSAVE_DURATION); // snapshot settings fenv!( - snasphot_settings, + snapshot_settings, SKY_SNAPSHOT_DURATION, SKY_SNAPSHOT_KEEP, SKY_SNAPSHOT_FAILSAFE diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index c4ef85a7..feff006c 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -40,6 +40,7 @@ mod cfgerr; mod cfgfile; // TODO: Upgrade to use these modules mod cfg2; +mod cfgcli2; mod cfgenv2; mod eval; #[cfg(test)] From afcd8031c26db4d3daa2639dd0df5a5c58f91f48 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 19:37:58 -0800 Subject: [PATCH 08/24] Fix missing checks for server.noart and server.maxcon for CLI config --- server/src/config/cfgcli2.rs | 8 +++++++- server/src/config/cfgenv2.rs | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/config/cfgcli2.rs b/server/src/config/cfgcli2.rs index 9eb71da0..19930dae 100644 --- a/server/src/config/cfgcli2.rs +++ b/server/src/config/cfgcli2.rs @@ -47,7 +47,7 @@ impl TryFromConfigSource for Flag { *trip = true; *target_value = true; } - true + false } fn try_parse(self) -> ConfigSourceParseResult { ConfigSourceParseResult::Okay(self.flag_set) @@ -74,6 +74,12 @@ pub(super) fn parse_cli_args(matches: ArgMatches) -> Configset { matches.value_of("port"), "--port" ); + fcli!( + server_noart, + Flag::new(matches.is_present("noart")), + "--noart" + ); + fcli!(server_maxcon, matches.value_of("maxcon"), "--maxcon"); // bgsave settings fcli!( bgsave_settings, diff --git a/server/src/config/cfgenv2.rs b/server/src/config/cfgenv2.rs index 59fa2e14..da325752 100644 --- a/server/src/config/cfgenv2.rs +++ b/server/src/config/cfgenv2.rs @@ -27,7 +27,7 @@ use super::cfg2::Configset; /// Returns the environment configuration -pub(super) fn get_env_config() -> Configset { +pub(super) fn parse_env_config() -> Configset { let mut defset = Configset::new_env(); macro_rules! fenv { ( From d820ef910db2f17825ec01144d4fc56438ee8e05 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 21:13:19 -0800 Subject: [PATCH 09/24] Add config file impl using new config impl --- server/src/config/cfg2.rs | 39 ++++++++- server/src/config/cfgfile2.rs | 159 ++++++++++++++++++++++++++++++++++ server/src/config/mod.rs | 1 + 3 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 server/src/config/cfgfile2.rs diff --git a/server/src/config/cfg2.rs b/server/src/config/cfg2.rs index 2c14bcae..01037c63 100644 --- a/server/src/config/cfg2.rs +++ b/server/src/config/cfg2.rs @@ -124,11 +124,15 @@ impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { } #[derive(Debug)] +/// Since we have conflicting trait implementations, we define a custom `Option` type pub struct OptString { base: Option, } impl OptString { + pub const fn new(base: Option) -> Self { + Self { base } + } pub const fn new_null() -> Self { Self { base: None } } @@ -137,6 +141,12 @@ impl OptString { } } +impl From> for OptString { + fn from(base: Option) -> Self { + Self { base } + } +} + impl FromStr for OptString { type Err = (); fn from_str(st: &str) -> Result { @@ -146,6 +156,24 @@ impl FromStr for OptString { } } +impl TryFromConfigSource for OptString { + fn is_present(&self) -> bool { + self.base.is_some() + } + fn mutate_failed(self, target: &mut OptString, trip: &mut bool) -> bool { + if let Some(v) = self.base { + target.base = Some(v); + *trip = true; + } + false + } + fn try_parse(self) -> ConfigSourceParseResult { + self.base + .map(|v| ConfigSourceParseResult::Okay(OptString { base: Some(v) })) + .unwrap_or(ConfigSourceParseResult::Okay(OptString::new_null())) + } +} + #[derive(Debug)] /// A high-level configuration set that automatically handles errors, warnings and provides a convenient [`Result`] /// type that can be used @@ -179,9 +207,14 @@ impl Configset { pub fn new_cli() -> Self { Self::_new(Self::EMSG_CLI) } - /// Create a new configset for a config file + /// Create a new configset for config files pub fn new_file() -> Self { - Self::_new(Self::EMSG_FILE) + Self { + did_mutate: true, + cfg: ConfigurationSet::default(), + estack: ErrorStack::new(Self::EMSG_FILE), + wstack: WarningStack::new(Self::EMSG_FILE), + } } /// Mark the configset mutated fn mutated(&mut self) { @@ -425,7 +458,7 @@ impl Configset { "path to TLS cert passphrase", ); - let sslopts = SslOpts::new(key, cert, port, tls_pass.finish()); + let sslopts = SslOpts::new(key, cert, port, tls_pass.base); // now check if TLS only if tls_only { let host = self.cfg.ports.get_host(); diff --git a/server/src/config/cfgfile2.rs b/server/src/config/cfgfile2.rs new file mode 100644 index 00000000..6046fe6f --- /dev/null +++ b/server/src/config/cfgfile2.rs @@ -0,0 +1,159 @@ +/* + * Created on Fri Jan 28 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use super::cfg2::{ConfigSourceParseResult, Configset, OptString, TryFromConfigSource}; +use super::cfgfile::{Config as ConfigFile, ConfigKeyBGSAVE, ConfigKeySnapshot, KeySslOpts}; + +/// A custom non-null type for config files +pub struct NonNull { + val: T, +} + +impl From for NonNull { + fn from(val: T) -> Self { + Self { val } + } +} + +impl TryFromConfigSource for NonNull { + fn is_present(&self) -> bool { + true + } + fn mutate_failed(self, target: &mut T, trip: &mut bool) -> bool { + *target = self.val; + *trip = true; + false + } + fn try_parse(self) -> ConfigSourceParseResult { + ConfigSourceParseResult::Okay(self.val) + } +} + +pub struct Optional { + base: Option, +} + +impl Optional { + pub const fn some(val: T) -> Self { + Self { base: Some(val) } + } + pub const fn none() -> Self { + Self { base: None } + } +} + +impl From> for Optional { + fn from(base: Option) -> Self { + Self { base } + } +} + +impl TryFromConfigSource for Optional { + fn is_present(&self) -> bool { + self.base.is_some() + } + fn mutate_failed(self, target: &mut T, trip: &mut bool) -> bool { + if let Some(v) = self.base { + *trip = true; + *target = v; + } + false + } + fn try_parse(self) -> ConfigSourceParseResult { + match self.base { + Some(v) => ConfigSourceParseResult::Okay(v), + None => ConfigSourceParseResult::Absent, + } + } +} + +pub fn from_file(file: ConfigFile) -> Configset { + let mut set = Configset::new_file(); + let ConfigFile { + server, + bgsave, + snapshot, + ssl, + } = file; + // server settings + set.server_tcp( + Optional::some(server.host), + "server.host", + Optional::some(server.port), + "server.port", + ); + set.server_maxcon(Optional::from(server.maxclient), "server.maxcon"); + set.server_noart(Optional::from(server.noart), "server.noart"); + // bgsave settings + if let Some(bgsave) = bgsave { + let ConfigKeyBGSAVE { enabled, every } = bgsave; + set.bgsave_settings( + Optional::from(enabled), + "bgsave.enabled", + Optional::from(every), + "bgsave.every", + ); + } + // snapshot settings + if let Some(snapshot) = snapshot { + let ConfigKeySnapshot { + every, + atmost, + failsafe, + } = snapshot; + set.snapshot_settings( + NonNull::from(every), + "snapshot.every", + NonNull::from(atmost), + "snapshot.atmost", + Optional::from(failsafe), + "snapshot.failsafe", + ); + } + // TLS settings + if let Some(tls) = ssl { + let KeySslOpts { + key, + chain, + port, + only, + passin, + } = tls; + set.tls_settings( + NonNull::from(key), + "ssl.key", + NonNull::from(chain), + "ssl.chain", + NonNull::from(port), + "ssl.port", + Optional::from(only), + "ssl.only", + OptString::from(passin), + "ssl.passin", + ); + } + set +} diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index feff006c..356244cb 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -42,6 +42,7 @@ mod cfgfile; mod cfg2; mod cfgcli2; mod cfgenv2; +mod cfgfile2; mod eval; #[cfg(test)] mod tests; From b46a5ac13fe1012815a83f9344fb98e43c713e45 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 21:19:50 -0800 Subject: [PATCH 10/24] Add method for chaining configuration sets --- server/src/config/cfg2.rs | 21 +++++++++++++++++++++ server/src/config/eval.rs | 3 +++ 2 files changed, 24 insertions(+) diff --git a/server/src/config/cfg2.rs b/server/src/config/cfg2.rs index 01037c63..b0f1e560 100644 --- a/server/src/config/cfg2.rs +++ b/server/src/config/cfg2.rs @@ -293,6 +293,27 @@ impl Configset { self.epush(field_key, expected) } } + /// This method can be used to chain configurations to ultimately return the first modified configuration + /// that occurs. For example: `cfg_file.and_then(cfg_cli).and_then(cfg_env)`; it will return the first + /// modified Configset + /// + /// ## Panics + /// This method will panic if both the provided sets are mutated. Hence, you need to check beforehand that + /// there is no conflict + pub fn and_then(self, other: Self) -> Self { + if self.is_mutated() { + if other.is_mutated() { + panic!( + "Double mutation: {env_a} and {env_b}", + env_a = self.estack.source(), + env_b = other.estack.source() + ); + } + self + } else { + other + } + } } // server settings diff --git a/server/src/config/eval.rs b/server/src/config/eval.rs index 2ec92a2d..7c0f6e2a 100644 --- a/server/src/config/eval.rs +++ b/server/src/config/eval.rs @@ -47,6 +47,9 @@ impl FeedbackStack { feedback_source, } } + pub fn source(&self) -> &'static str { + self.feedback_source + } pub fn push(&mut self, f: impl ToString) { self.stack.push(f.to_string()) } From 86b9ac3deeefd161e149850ccd63335a2e471d7d Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Thu, 27 Jan 2022 23:23:34 -0800 Subject: [PATCH 11/24] Switch to using new config framework --- server/src/config/cfg2.rs | 583 ------------ server/src/config/{cfgcli2.rs => cfgcli.rs} | 37 +- server/src/config/cfgenv.rs | 161 +--- server/src/config/cfgenv2.rs | 70 -- server/src/config/cfgerr.rs | 95 -- server/src/config/cfgfile.rs | 131 +++ server/src/config/cfgfile2.rs | 159 ---- server/src/config/definitions.rs | 286 ++++++ server/src/config/{eval.rs => feedback.rs} | 57 +- server/src/config/macros.rs | 59 -- server/src/config/mod.rs | 938 ++++++++++---------- server/src/config/tests.rs | 279 ++---- server/src/main.rs | 26 +- 13 files changed, 1067 insertions(+), 1814 deletions(-) delete mode 100644 server/src/config/cfg2.rs rename server/src/config/{cfgcli2.rs => cfgcli.rs} (74%) delete mode 100644 server/src/config/cfgenv2.rs delete mode 100644 server/src/config/cfgerr.rs delete mode 100644 server/src/config/cfgfile2.rs create mode 100644 server/src/config/definitions.rs rename server/src/config/{eval.rs => feedback.rs} (71%) delete mode 100644 server/src/config/macros.rs diff --git a/server/src/config/cfg2.rs b/server/src/config/cfg2.rs deleted file mode 100644 index b0f1e560..00000000 --- a/server/src/config/cfg2.rs +++ /dev/null @@ -1,583 +0,0 @@ -/* - * Created on Thu Jan 27 2022 - * - * This file is a part of Skytable - * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source - * NoSQL database written by Sayan Nandan ("the Author") with the - * vision to provide flexibility in data modelling without compromising - * on performance, queryability or scalability. - * - * Copyright (c) 2022, Sayan Nandan - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * -*/ - -use super::eval::{ErrorStack, WarningStack}; -use super::{BGSave, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, SslOpts}; -use core::str::FromStr; -use std::env::VarError; -use std::net::IpAddr; - -type StaticStr = &'static str; - -#[derive(Debug)] -/// An enum representing the outcome of a parse operation for a specific configuration item from a -/// specific configuration source -pub enum ConfigSourceParseResult { - Okay(T), - Absent, - ParseFailure, -} - -/// A trait for configuration sources. Any type implementing this trait is considered to be a valid -/// source for configuration -pub trait TryFromConfigSource: Sized { - /// Check if the value is present - fn is_present(&self) -> bool; - /// Attempt to mutate the value if present. If any error occurs - /// while parsing the value, return true. Else return false if all went well. - /// Here: - /// - `target_value`: is a mutable reference to the target var - /// - `trip`: is a mutable ref to a bool that will be set to true if a value is present - /// (whether parseable or not) - fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool; - /// Attempt to parse the value into the target type - fn try_parse(self) -> ConfigSourceParseResult; -} - -impl<'a, T: FromStr + 'a> TryFromConfigSource for Option<&'a str> { - fn is_present(&self) -> bool { - self.is_some() - } - fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { - self.map(|slf| { - *trip = true; - match slf.parse() { - Ok(p) => { - *target_value = p; - false - } - Err(_) => true, - } - }) - .unwrap_or(false) - } - fn try_parse(self) -> ConfigSourceParseResult { - self.map(|s| { - s.parse() - .map(|ret| ConfigSourceParseResult::Okay(ret)) - .unwrap_or(ConfigSourceParseResult::ParseFailure) - }) - .unwrap_or(ConfigSourceParseResult::Absent) - } -} - -impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { - fn is_present(&self) -> bool { - !matches!(self, Err(VarError::NotPresent)) - } - fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { - match self { - Ok(s) => s - .parse() - .map(|v| { - *trip = true; - *target_value = v; - false - }) - .unwrap_or(true), - Err(e) => { - if matches!(e, VarError::NotPresent) { - false - } else { - // yes, we got the var but failed to parse it into unicode; so trip - *trip = true; - true - } - } - } - } - fn try_parse(self) -> ConfigSourceParseResult { - match self { - Ok(s) => s - .parse() - .map(|v| ConfigSourceParseResult::Okay(v)) - .unwrap_or(ConfigSourceParseResult::ParseFailure), - Err(e) => match e { - VarError::NotPresent => ConfigSourceParseResult::Absent, - VarError::NotUnicode(_) => ConfigSourceParseResult::ParseFailure, - }, - } - } -} - -#[derive(Debug)] -/// Since we have conflicting trait implementations, we define a custom `Option` type -pub struct OptString { - base: Option, -} - -impl OptString { - pub const fn new(base: Option) -> Self { - Self { base } - } - pub const fn new_null() -> Self { - Self { base: None } - } - pub fn finish(self) -> Option { - self.base - } -} - -impl From> for OptString { - fn from(base: Option) -> Self { - Self { base } - } -} - -impl FromStr for OptString { - type Err = (); - fn from_str(st: &str) -> Result { - Ok(Self { - base: Some(st.to_string()), - }) - } -} - -impl TryFromConfigSource for OptString { - fn is_present(&self) -> bool { - self.base.is_some() - } - fn mutate_failed(self, target: &mut OptString, trip: &mut bool) -> bool { - if let Some(v) = self.base { - target.base = Some(v); - *trip = true; - } - false - } - fn try_parse(self) -> ConfigSourceParseResult { - self.base - .map(|v| ConfigSourceParseResult::Okay(OptString { base: Some(v) })) - .unwrap_or(ConfigSourceParseResult::Okay(OptString::new_null())) - } -} - -#[derive(Debug)] -/// A high-level configuration set that automatically handles errors, warnings and provides a convenient [`Result`] -/// type that can be used -pub struct Configset { - did_mutate: bool, - cfg: ConfigurationSet, - estack: ErrorStack, - wstack: WarningStack, -} - -impl Configset { - const EMSG_ENV: StaticStr = "Environment"; - const EMSG_CLI: StaticStr = "CLI arguments"; - const EMSG_FILE: StaticStr = "Configuration file"; - - /// Internal ctor for a given feedback source. We do not want to expose this to avoid - /// erroneous feedback source names - fn _new(feedback_source: StaticStr) -> Self { - Self { - did_mutate: false, - cfg: ConfigurationSet::default(), - estack: ErrorStack::new(feedback_source), - wstack: WarningStack::new(feedback_source), - } - } - /// Create a new configset for environment variables - pub fn new_env() -> Self { - Self::_new(Self::EMSG_ENV) - } - /// Create a new configset for CLI args - pub fn new_cli() -> Self { - Self::_new(Self::EMSG_CLI) - } - /// Create a new configset for config files - pub fn new_file() -> Self { - Self { - did_mutate: true, - cfg: ConfigurationSet::default(), - estack: ErrorStack::new(Self::EMSG_FILE), - wstack: WarningStack::new(Self::EMSG_FILE), - } - } - /// Mark the configset mutated - fn mutated(&mut self) { - self.did_mutate = true; - } - /// Push an error onto the error stack - fn epush(&mut self, field_key: StaticStr, expected: StaticStr) { - self.estack.push(format!( - "Bad value for `${field_key}`. Expected ${expected}", - )) - } - /// Check if no errors have occurred - pub fn is_okay(&self) -> bool { - self.estack.is_empty() - } - /// Check if the configset was mutated - pub fn is_mutated(&self) -> bool { - self.did_mutate - } - /// Turns self into a Result - pub fn into_result(self) -> Result, ErrorStack> { - let Self { - wstack, - estack, - cfg, - did_mutate, - } = self; - log::warn!("{}", wstack); - if did_mutate { - if estack.is_empty() { - Ok(Some(cfg)) - } else { - Err(estack) - } - } else { - Ok(None) - } - } - /// Attempt to mutate with a target `TryFromConfigSource` type, and push in any error that occurs - /// using the given diagnostic info - fn try_mutate( - &mut self, - new: impl TryFromConfigSource, - target: &mut T, - field_key: StaticStr, - expected: StaticStr, - ) { - if new.mutate_failed(target, &mut self.did_mutate) { - self.epush(field_key, expected) - } - } - /// Attempt to mutate with a target `TryFromConfigSource` type, and push in any error that occurs - /// using the given diagnostic info while checking the correctly parsed type using the provided validation - /// closure for any additional validation check that goes beyond type correctness - fn try_mutate_with_condcheck( - &mut self, - new: impl TryFromConfigSource, - target: &mut T, - field_key: StaticStr, - expected: StaticStr, - validation_fn: F, - ) where - F: Fn(&T) -> bool, - { - let mut needs_error = false; - match new.try_parse() { - ConfigSourceParseResult::Okay(ok) => { - self.mutated(); - needs_error = !validation_fn(&ok); - *target = ok; - } - ConfigSourceParseResult::ParseFailure => needs_error = true, - ConfigSourceParseResult::Absent => {} - } - if needs_error { - self.epush(field_key, expected) - } - } - /// This method can be used to chain configurations to ultimately return the first modified configuration - /// that occurs. For example: `cfg_file.and_then(cfg_cli).and_then(cfg_env)`; it will return the first - /// modified Configset - /// - /// ## Panics - /// This method will panic if both the provided sets are mutated. Hence, you need to check beforehand that - /// there is no conflict - pub fn and_then(self, other: Self) -> Self { - if self.is_mutated() { - if other.is_mutated() { - panic!( - "Double mutation: {env_a} and {env_b}", - env_a = self.estack.source(), - env_b = other.estack.source() - ); - } - self - } else { - other - } - } -} - -// server settings -impl Configset { - pub fn server_tcp( - &mut self, - nhost: impl TryFromConfigSource, - nhost_key: StaticStr, - nport: impl TryFromConfigSource, - nport_key: StaticStr, - ) { - let mut host = super::DEFAULT_IPV4; - let mut port = super::DEFAULT_PORT; - self.try_mutate(nhost, &mut host, nhost_key, "an IPv4/IPv6 address"); - self.try_mutate(nport, &mut port, nport_key, "a 16-bit positive integer"); - self.cfg.ports = PortConfig::new_insecure_only(host, port); - } - pub fn server_noart(&mut self, nart: impl TryFromConfigSource, nart_key: StaticStr) { - let mut noart = false; - self.try_mutate(nart, &mut noart, nart_key, "true/false"); - self.cfg.noart = noart; - } - pub fn server_maxcon( - &mut self, - nmaxcon: impl TryFromConfigSource, - nmaxcon_key: StaticStr, - ) { - let mut maxcon = super::MAXIMUM_CONNECTION_LIMIT; - self.try_mutate_with_condcheck( - nmaxcon, - &mut maxcon, - nmaxcon_key, - "a positive integer greater than zero", - |max| *max > 0, - ); - self.cfg.maxcon = maxcon; - } -} - -// bgsave settings -impl Configset { - pub fn bgsave_settings( - &mut self, - nenabled: impl TryFromConfigSource, - nenabled_key: StaticStr, - nduration: impl TryFromConfigSource, - nduration_key: StaticStr, - ) { - let mut enabled = true; - let mut duration = 120; - let has_custom_duration = nduration.is_present(); - self.try_mutate(nenabled, &mut enabled, nenabled_key, "true/false"); - self.try_mutate_with_condcheck( - nduration, - &mut duration, - nduration_key, - "a positive integer greater than zero", - |dur| *dur > 0, - ); - if enabled { - self.cfg.bgsave = BGSave::Enabled(duration); - } else { - if has_custom_duration { - self.wstack.push(format!( - "Specifying `${nduration_key}` is useless when BGSAVE is disabled" - )); - } - self.wstack - .push("BGSAVE is disabled. You may lose data if the host crashes"); - } - } -} - -// snapshot settings -impl Configset { - pub fn snapshot_settings( - &mut self, - nevery: impl TryFromConfigSource, - nevery_key: StaticStr, - natmost: impl TryFromConfigSource, - natmost_key: StaticStr, - nfailsafe: impl TryFromConfigSource, - nfailsafe_key: StaticStr, - ) { - match (nevery.is_present(), natmost.is_present()) { - (false, false) => { - // noice, disabled - if nfailsafe.is_present() { - // this mutation is pointless, but it is just for the sake of making sure - // that the `failsafe` key has a proper boolean, no matter if it is pointless - let mut _failsafe = true; - self.try_mutate(nfailsafe, &mut _failsafe, nfailsafe_key, "true/false"); - self.wstack.push(format!( - "Specifying `${nfailsafe_key}` is usless when snapshots are disabled" - )); - } - } - (true, true) => { - let mut every = 0; - let mut atmost = 0; - let mut failsafe = true; - self.try_mutate_with_condcheck( - nevery, - &mut every, - nevery_key, - "an integer greater than 0", - |dur| *dur > 0, - ); - self.try_mutate( - natmost, - &mut atmost, - natmost_key, - "a positive integer. 0 indicates that all snapshots will be kept", - ); - self.try_mutate(nfailsafe, &mut failsafe, nfailsafe_key, "true/false"); - self.cfg.snapshot = - SnapshotConfig::Enabled(SnapshotPref::new(every, atmost, failsafe)); - } - (false, true) | (true, false) => self.estack.push(format!( - "To use snapshots, pass values for both `${nevery_key}` and `${natmost_key}`" - )), - } - } -} - -// TLS settings -impl Configset { - pub fn tls_settings( - &mut self, - nkey: impl TryFromConfigSource, - nkey_key: StaticStr, - ncert: impl TryFromConfigSource, - ncert_key: StaticStr, - nport: impl TryFromConfigSource, - nport_key: StaticStr, - nonly: impl TryFromConfigSource, - nonly_key: StaticStr, - npass: impl TryFromConfigSource, - npass_key: StaticStr, - ) { - match (nkey.is_present(), ncert.is_present()) { - (true, true) => { - // get the cert details - let mut key = String::new(); - let mut cert = String::new(); - self.try_mutate(nkey, &mut key, nkey_key, "path to private key file"); - self.try_mutate(ncert, &mut cert, ncert_key, "path to TLS certificate file"); - - // now get port info - let mut port = super::DEFAULT_SSL_PORT; - self.try_mutate(nport, &mut port, nport_key, "a positive 16-bit integer"); - - // now check if TLS only - let mut tls_only = false; - self.try_mutate(nonly, &mut tls_only, nonly_key, "true/false"); - - // check if we have a TLS cert - let mut tls_pass = OptString::new_null(); - self.try_mutate( - npass, - &mut tls_pass, - npass_key, - "path to TLS cert passphrase", - ); - - let sslopts = SslOpts::new(key, cert, port, tls_pass.base); - // now check if TLS only - if tls_only { - let host = self.cfg.ports.get_host(); - self.cfg.ports = PortConfig::new_secure_only(host, sslopts) - } else { - // multi. go and upgrade existing - self.cfg.ports.upgrade_to_tls(sslopts); - } - } - (true, false) | (false, true) => { - self.estack.push(format!( - "To use TLS, pass values for both `${nkey_key}` and `${ncert_key}`" - )); - } - (false, false) => { - if nport.is_present() { - self.wstack - .push("Specifying `${nport_key}` is pointless when TLS is disabled"); - } - if nonly.is_present() { - self.wstack - .push("Specifying `${nonly_key}` is pointless when TLS is disabled"); - } - if npass.is_present() { - self.wstack - .push("Specifying `${npass_key}` is pointless when TLS is disabled"); - } - } - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::config::DEFAULT_IPV4; - #[test] - fn server_tcp() { - let mut cfgset = Configset::new_env(); - cfgset.server_tcp( - Some("127.0.0.1"), - "SKY_SERVER_HOST", - Some("2004"), - "SKY_SERVER_PORT", - ); - assert_eq!( - cfgset.cfg.ports, - PortConfig::new_insecure_only(DEFAULT_IPV4, 2004) - ); - assert!(cfgset.is_mutated()); - assert!(cfgset.is_okay()); - } - #[test] - fn server_tcp_fail_host() { - let mut cfgset = Configset::new_env(); - cfgset.server_tcp( - Some("?127.0.0.1"), - "SKY_SERVER_HOST", - Some("2004"), - "SKY_SERVER_PORT", - ); - assert_eq!( - cfgset.cfg.ports, - PortConfig::new_insecure_only(DEFAULT_IPV4, 2004) - ); - assert!(cfgset.is_mutated()); - assert!(!cfgset.is_okay()); - } - #[test] - fn server_tcp_fail_port() { - let mut cfgset = Configset::new_env(); - cfgset.server_tcp( - Some("127.0.0.1"), - "SKY_SERVER_HOST", - Some("65537"), - "SKY_SERVER_PORT", - ); - assert_eq!( - cfgset.cfg.ports, - PortConfig::new_insecure_only(DEFAULT_IPV4, 2003) - ); - assert!(cfgset.is_mutated()); - assert!(!cfgset.is_okay()); - } - #[test] - fn server_tcp_fail_both() { - let mut cfgset = Configset::new_env(); - cfgset.server_tcp( - Some("?127.0.0.1"), - "SKY_SERVER_HOST", - Some("65537"), - "SKY_SERVER_PORT", - ); - assert_eq!( - cfgset.cfg.ports, - PortConfig::new_insecure_only(DEFAULT_IPV4, 2003) - ); - assert!(cfgset.is_mutated()); - assert!(!cfgset.is_okay()); - } -} diff --git a/server/src/config/cfgcli2.rs b/server/src/config/cfgcli.rs similarity index 74% rename from server/src/config/cfgcli2.rs rename to server/src/config/cfgcli.rs index 19930dae..e41323e6 100644 --- a/server/src/config/cfgcli2.rs +++ b/server/src/config/cfgcli.rs @@ -24,33 +24,38 @@ * */ -use super::cfg2::{ConfigSourceParseResult, Configset, TryFromConfigSource}; +use super::{ConfigSourceParseResult, Configset, TryFromConfigSource}; use clap::ArgMatches; -#[derive(Clone, Copy)] -struct Flag { - flag_set: bool, +/// A flag. The flag is said to be set if `self.set` is true and unset if `self.set` is false. However, +/// if the flag is set, the value of SWITCH determines what value it is set to +struct Flag { + set: bool, } -impl Flag { - const fn new(flag_set: bool) -> Self { - Self { flag_set } +impl Flag { + fn new(set: bool) -> Self { + Self { set } } } -impl TryFromConfigSource for Flag { +impl TryFromConfigSource for Flag { fn is_present(&self) -> bool { - self.flag_set + self.set } - fn mutate_failed(self, target_value: &mut bool, trip: &mut bool) -> bool { - if self.is_present() { + fn mutate_failed(self, target: &mut bool, trip: &mut bool) -> bool { + if self.set { *trip = true; - *target_value = true; + *target = SWITCH; } false } fn try_parse(self) -> ConfigSourceParseResult { - ConfigSourceParseResult::Okay(self.flag_set) + if self.set { + ConfigSourceParseResult::Okay(SWITCH) + } else { + ConfigSourceParseResult::Absent + } } } @@ -76,14 +81,14 @@ pub(super) fn parse_cli_args(matches: ArgMatches) -> Configset { ); fcli!( server_noart, - Flag::new(matches.is_present("noart")), + Flag::::new(matches.is_present("noart")), "--noart" ); fcli!(server_maxcon, matches.value_of("maxcon"), "--maxcon"); // bgsave settings fcli!( bgsave_settings, - Flag::new(!matches.is_present("nosave")), + Flag::::new(matches.is_present("nosave")), "--nosave", matches.value_of("saveduration"), "--saveduration" @@ -107,7 +112,7 @@ pub(super) fn parse_cli_args(matches: ArgMatches) -> Configset { "--sslchain", matches.value_of("sslport"), "--sslport", - Flag::new(matches.is_present("sslonly")), + Flag::::new(matches.is_present("sslonly")), "--sslonly", matches.value_of("tlspass"), "--tlspassin" diff --git a/server/src/config/cfgenv.rs b/server/src/config/cfgenv.rs index 76822dfa..53615843 100644 --- a/server/src/config/cfgenv.rs +++ b/server/src/config/cfgenv.rs @@ -1,5 +1,5 @@ /* - * Created on Sat Oct 02 2021 + * Created on Thu Jan 27 2022 * * This file is a part of Skytable * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source @@ -7,7 +7,7 @@ * vision to provide flexibility in data modelling without compromising * on performance, queryability or scalability. * - * Copyright (c) 2021, Sayan Nandan + * Copyright (c) 2022, Sayan Nandan * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by @@ -24,122 +24,47 @@ * */ -use super::{ - BGSave, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, SslOpts, DEFAULT_IPV4, - DEFAULT_PORT, DEFAULT_SSL_PORT, -}; -use std::env::{self, VarError}; -use std::net::IpAddr; +use super::Configset; -pub(super) enum EnvError { - CfgError(&'static str), - ParseError(String), -} - -pub(super) fn get_env_config() -> Result, EnvError> { - let mut defset = ConfigurationSet::default(); - let mut is_set = false; - macro_rules! getenv { - ($var:ident) => {{ - let var = stringify!($var); - match env::var(var) { - Ok(v) => { - // set flag to true - is_set = true; - Some(v) - } - Err(e) => match e { - VarError::NotPresent => None, - VarError::NotUnicode(..) => { - return Err(EnvError::ParseError(format!( - "Bad value for {var}. The value is not unicode", - var = var - ))); - } - }, - } - }}; - ($var:ident, $ty:ty) => {{ - match getenv!($var).map(|v| v.parse::<$ty>()) { - Some(Ok(v)) => Some(v), - Some(Err(e)) => { - return Err(EnvError::ParseError(format!( - "Bad value for {var}. {e}", - var = stringify!($var), - e = e - ))) - } - None => None, - } - }}; - } - // get system settings - let noart = getenv!(SKY_SYSTEM_NOART, bool); - let maxcon = getenv!(SKY_SYSTEM_MAXCON, usize); - set_if_exists!(noart, defset.noart); - set_if_exists!(maxcon, defset.maxcon); - - // now get port config - let port = getenv!(SKY_SYSTEM_PORT, u16).unwrap_or(DEFAULT_PORT); - let host = getenv!(SKY_SYSTEM_HOST, IpAddr).unwrap_or(DEFAULT_IPV4); - let tlsport = getenv!(SKY_TLS_PORT, u16); - let tlsonly = getenv!(SKY_TLS_ONLY, bool).unwrap_or_default(); - let tlscert = getenv!(SKY_TLS_CERT, String); - let tlskey = getenv!(SKY_TLS_KEY, String); - let tls_passin = getenv!(SKY_TLS_PASSIN, String); - let portcfg = match (tlscert, tlskey) { - (Some(cert), Some(key)) => { - let sslopts = SslOpts::new(key, cert, tlsport.unwrap_or(DEFAULT_SSL_PORT), tls_passin); - if tlsonly { - PortConfig::new_secure_only(host, sslopts) - } else { - PortConfig::new_multi(host, port, sslopts) - } - } - (None, None) => { - // no TLS - if tlsonly { - log::warn!("Ignoring value for SKY_TLS_ONLY because TLS was disabled"); - } - if tlsport.is_some() { - log::warn!("Ignoring value for SKY_TLS_PORT because TLS was disabled"); - } - PortConfig::new_insecure_only(host, port) - } - _ => { - return Err(EnvError::CfgError( - "To use TLS, pass values for both SKY_TLS_CERT and SKY_TLS_KEY", - )) - } - }; - defset.ports = portcfg; - - // now get bgsave - let bgsave_enabled = getenv!(SKY_BGSAVE_ENABLED, bool).unwrap_or(true); - let bgsave_duration = getenv!(SKY_BGSAVE_DURATION, u64).unwrap_or(120); - let bgsave = BGSave::new(bgsave_enabled, bgsave_duration); - defset.bgsave = bgsave; - // now get snapshot config - let snapshot_enabled = getenv!(SKY_SNAPSHOT_ENABLED, bool).unwrap_or_default(); - let snapshot_duration = getenv!(SKY_SNAPSHOT_DURATION, u64); - let snapshot_keep = getenv!(SKY_SNAPSHOT_KEEP, usize); - let snapshot_failsafe = getenv!(SKY_SNAPSHOT_FAILSAFE, bool).unwrap_or(true); - let snapcfg = { - if snapshot_enabled { - match (snapshot_duration, snapshot_keep) { - (Some(duration), Some(keep)) => { - SnapshotConfig::Enabled(SnapshotPref::new(duration, keep, snapshot_failsafe)) - }, - _ => return Err(EnvError::CfgError("To use snapshots, you must pass values for both SKY_SNAPSHOT_DURATION and SKY_SNAPSHOT_KEEP")), - } - } else { - SnapshotConfig::default() - } - }; - defset.snapshot = snapcfg; - if is_set { - Ok(Some(defset)) - } else { - Ok(None) +/// Returns the environment configuration +pub(super) fn parse_env_config() -> Configset { + let mut defset = Configset::new_env(); + macro_rules! fenv { + ( + $fn:ident, + $( + $field:ident + ),* + ) => { + defset.$fn( + $( + ::std::env::var(stringify!($field)), + stringify!($field), + )* + ); + }; } + // server settings + fenv!(server_tcp, SKY_SYSTEM_HOST, SKY_SYSTEM_PORT); + fenv!(server_noart, SKY_SYSTEM_NOART); + fenv!(server_maxcon, SKY_SYSTEM_MAXCON); + // bgsave settings + fenv!(bgsave_settings, SKY_BGSAVE_ENABLED, SKY_BGSAVE_DURATION); + // snapshot settings + fenv!( + snapshot_settings, + SKY_SNAPSHOT_DURATION, + SKY_SNAPSHOT_KEEP, + SKY_SNAPSHOT_FAILSAFE + ); + // TLS settings + fenv!( + tls_settings, + SKY_TLS_KEY, + SKY_TLS_CERT, + SKY_TLS_PORT, + SKY_TLS_ONLY, + SKY_TLS_PASSIN + ); + defset } diff --git a/server/src/config/cfgenv2.rs b/server/src/config/cfgenv2.rs deleted file mode 100644 index da325752..00000000 --- a/server/src/config/cfgenv2.rs +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Created on Thu Jan 27 2022 - * - * This file is a part of Skytable - * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source - * NoSQL database written by Sayan Nandan ("the Author") with the - * vision to provide flexibility in data modelling without compromising - * on performance, queryability or scalability. - * - * Copyright (c) 2022, Sayan Nandan - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * -*/ - -use super::cfg2::Configset; - -/// Returns the environment configuration -pub(super) fn parse_env_config() -> Configset { - let mut defset = Configset::new_env(); - macro_rules! fenv { - ( - $fn:ident, - $( - $field:ident - ),* - ) => { - defset.$fn( - $( - ::std::env::var(stringify!($field)), - stringify!($field), - )* - ); - }; - } - // server settings - fenv!(server_tcp, SKY_SYSTEM_HOST, SKY_SYSTEM_PORT); - fenv!(server_noart, SKY_SYSTEM_NOART); - fenv!(server_maxcon, SKY_SYSTEM_MAXCON); - // bgsave settings - fenv!(bgsave_settings, SKY_BGSAVE_ENABLED, SKY_BGSAVE_DURATION); - // snapshot settings - fenv!( - snapshot_settings, - SKY_SNAPSHOT_DURATION, - SKY_SNAPSHOT_KEEP, - SKY_SNAPSHOT_FAILSAFE - ); - // TLS settings - fenv!( - tls_settings, - SKY_TLS_KEY, - SKY_TLS_CERT, - SKY_TLS_PORT, - SKY_TLS_ONLY, - SKY_TLS_PASSIN - ); - defset -} diff --git a/server/src/config/cfgerr.rs b/server/src/config/cfgerr.rs deleted file mode 100644 index d0d67aa9..00000000 --- a/server/src/config/cfgerr.rs +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Created on Sat Oct 02 2021 - * - * This file is a part of Skytable - * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source - * NoSQL database written by Sayan Nandan ("the Author") with the - * vision to provide flexibility in data modelling without compromising - * on performance, queryability or scalability. - * - * Copyright (c) 2021, Sayan Nandan - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * -*/ - -use super::cfgenv::EnvError; -use std::fmt; - -pub(super) const ERR_CONFLICT: &str = - "Either use command line arguments, environment variables or a configuration file"; - -#[derive(Debug)] -/// Type of configuration error: -/// - The config file was not found (`OSError`) -/// - The config file was invalid (`SyntaxError`) -/// - The config file has an invalid value, which is syntatically correct -/// but logically incorrect (`CfgError`) -/// - The command line arguments have an invalid value/invalid values (`CliArgError`) -pub enum ConfigError { - OSError(std::io::Error), - SyntaxError(toml::de::Error), - CfgError(&'static str), - CliArgErr(&'static str), - EnvArgParseFailure(String), -} - -impl PartialEq for ConfigError { - fn eq(&self, oth: &Self) -> bool { - use ConfigError::*; - match (self, oth) { - (OSError(a), OSError(b)) => a.kind() == b.kind(), - (SyntaxError(a), SyntaxError(b)) => a == b, - (CfgError(a), CfgError(b)) => a == b, - (CliArgErr(a), CliArgErr(b)) => a == b, - (EnvArgParseFailure(a), EnvArgParseFailure(b)) => a == b, - _ => false, - } - } -} - -impl fmt::Display for ConfigError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ConfigError::OSError(e) => write!(f, "error: {}", e), - ConfigError::SyntaxError(e) => { - write!(f, "syntax error in configuration file: {}", e) - } - ConfigError::CfgError(e) => write!(f, "Configuration error: {}", e), - ConfigError::CliArgErr(e) => write!(f, "Argument error: {}", e), - ConfigError::EnvArgParseFailure(e) => write!(f, "Environment variable error: {}", e), - } - } -} - -impl From for ConfigError { - fn from(derr: toml::de::Error) -> Self { - Self::SyntaxError(derr) - } -} - -impl From for ConfigError { - fn from(derr: std::io::Error) -> Self { - Self::OSError(derr) - } -} - -impl From for ConfigError { - fn from(err: EnvError) -> Self { - match err { - EnvError::CfgError(e) => Self::CfgError(e), - EnvError::ParseError(e) => Self::EnvArgParseFailure(e), - } - } -} diff --git a/server/src/config/cfgfile.rs b/server/src/config/cfgfile.rs index 2a22cab0..0331fff7 100644 --- a/server/src/config/cfgfile.rs +++ b/server/src/config/cfgfile.rs @@ -24,6 +24,7 @@ * */ +use super::{ConfigSourceParseResult, Configset, OptString, TryFromConfigSource}; use serde::Deserialize; use std::net::IpAddr; @@ -89,3 +90,133 @@ pub struct KeySslOpts { pub(super) only: Option, pub(super) passin: Option, } + +/// A custom non-null type for config files +pub struct NonNull { + val: T, +} + +impl From for NonNull { + fn from(val: T) -> Self { + Self { val } + } +} + +impl TryFromConfigSource for NonNull { + fn is_present(&self) -> bool { + true + } + fn mutate_failed(self, target: &mut T, trip: &mut bool) -> bool { + *target = self.val; + *trip = true; + false + } + fn try_parse(self) -> ConfigSourceParseResult { + ConfigSourceParseResult::Okay(self.val) + } +} + +pub struct Optional { + base: Option, +} + +impl Optional { + pub const fn some(val: T) -> Self { + Self { base: Some(val) } + } +} + +impl From> for Optional { + fn from(base: Option) -> Self { + Self { base } + } +} + +impl TryFromConfigSource for Optional { + fn is_present(&self) -> bool { + self.base.is_some() + } + fn mutate_failed(self, target: &mut T, trip: &mut bool) -> bool { + if let Some(v) = self.base { + *trip = true; + *target = v; + } + false + } + fn try_parse(self) -> ConfigSourceParseResult { + match self.base { + Some(v) => ConfigSourceParseResult::Okay(v), + None => ConfigSourceParseResult::Absent, + } + } +} + +type ConfigFile = Config; + +pub fn from_file(file: ConfigFile) -> Configset { + let mut set = Configset::new_file(); + let ConfigFile { + server, + bgsave, + snapshot, + ssl, + } = file; + // server settings + set.server_tcp( + Optional::some(server.host), + "server.host", + Optional::some(server.port), + "server.port", + ); + set.server_maxcon(Optional::from(server.maxclient), "server.maxcon"); + set.server_noart(Optional::from(server.noart), "server.noart"); + // bgsave settings + if let Some(bgsave) = bgsave { + let ConfigKeyBGSAVE { enabled, every } = bgsave; + set.bgsave_settings( + Optional::from(enabled), + "bgsave.enabled", + Optional::from(every), + "bgsave.every", + ); + } + // snapshot settings + if let Some(snapshot) = snapshot { + let ConfigKeySnapshot { + every, + atmost, + failsafe, + } = snapshot; + set.snapshot_settings( + NonNull::from(every), + "snapshot.every", + NonNull::from(atmost), + "snapshot.atmost", + Optional::from(failsafe), + "snapshot.failsafe", + ); + } + // TLS settings + if let Some(tls) = ssl { + let KeySslOpts { + key, + chain, + port, + only, + passin, + } = tls; + set.tls_settings( + NonNull::from(key), + "ssl.key", + NonNull::from(chain), + "ssl.chain", + NonNull::from(port), + "ssl.port", + Optional::from(only), + "ssl.only", + OptString::from(passin), + "ssl.passin", + ); + } + set +} diff --git a/server/src/config/cfgfile2.rs b/server/src/config/cfgfile2.rs deleted file mode 100644 index 6046fe6f..00000000 --- a/server/src/config/cfgfile2.rs +++ /dev/null @@ -1,159 +0,0 @@ -/* - * Created on Fri Jan 28 2022 - * - * This file is a part of Skytable - * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source - * NoSQL database written by Sayan Nandan ("the Author") with the - * vision to provide flexibility in data modelling without compromising - * on performance, queryability or scalability. - * - * Copyright (c) 2022, Sayan Nandan - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * -*/ - -use super::cfg2::{ConfigSourceParseResult, Configset, OptString, TryFromConfigSource}; -use super::cfgfile::{Config as ConfigFile, ConfigKeyBGSAVE, ConfigKeySnapshot, KeySslOpts}; - -/// A custom non-null type for config files -pub struct NonNull { - val: T, -} - -impl From for NonNull { - fn from(val: T) -> Self { - Self { val } - } -} - -impl TryFromConfigSource for NonNull { - fn is_present(&self) -> bool { - true - } - fn mutate_failed(self, target: &mut T, trip: &mut bool) -> bool { - *target = self.val; - *trip = true; - false - } - fn try_parse(self) -> ConfigSourceParseResult { - ConfigSourceParseResult::Okay(self.val) - } -} - -pub struct Optional { - base: Option, -} - -impl Optional { - pub const fn some(val: T) -> Self { - Self { base: Some(val) } - } - pub const fn none() -> Self { - Self { base: None } - } -} - -impl From> for Optional { - fn from(base: Option) -> Self { - Self { base } - } -} - -impl TryFromConfigSource for Optional { - fn is_present(&self) -> bool { - self.base.is_some() - } - fn mutate_failed(self, target: &mut T, trip: &mut bool) -> bool { - if let Some(v) = self.base { - *trip = true; - *target = v; - } - false - } - fn try_parse(self) -> ConfigSourceParseResult { - match self.base { - Some(v) => ConfigSourceParseResult::Okay(v), - None => ConfigSourceParseResult::Absent, - } - } -} - -pub fn from_file(file: ConfigFile) -> Configset { - let mut set = Configset::new_file(); - let ConfigFile { - server, - bgsave, - snapshot, - ssl, - } = file; - // server settings - set.server_tcp( - Optional::some(server.host), - "server.host", - Optional::some(server.port), - "server.port", - ); - set.server_maxcon(Optional::from(server.maxclient), "server.maxcon"); - set.server_noart(Optional::from(server.noart), "server.noart"); - // bgsave settings - if let Some(bgsave) = bgsave { - let ConfigKeyBGSAVE { enabled, every } = bgsave; - set.bgsave_settings( - Optional::from(enabled), - "bgsave.enabled", - Optional::from(every), - "bgsave.every", - ); - } - // snapshot settings - if let Some(snapshot) = snapshot { - let ConfigKeySnapshot { - every, - atmost, - failsafe, - } = snapshot; - set.snapshot_settings( - NonNull::from(every), - "snapshot.every", - NonNull::from(atmost), - "snapshot.atmost", - Optional::from(failsafe), - "snapshot.failsafe", - ); - } - // TLS settings - if let Some(tls) = ssl { - let KeySslOpts { - key, - chain, - port, - only, - passin, - } = tls; - set.tls_settings( - NonNull::from(key), - "ssl.key", - NonNull::from(chain), - "ssl.chain", - NonNull::from(port), - "ssl.port", - Optional::from(only), - "ssl.only", - OptString::from(passin), - "ssl.passin", - ); - } - set -} diff --git a/server/src/config/definitions.rs b/server/src/config/definitions.rs new file mode 100644 index 00000000..f60ca9ab --- /dev/null +++ b/server/src/config/definitions.rs @@ -0,0 +1,286 @@ +/* + * Created on Fri Jan 28 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use super::feedback::WarningStack; +use super::{DEFAULT_IPV4, DEFAULT_PORT}; +use crate::dbnet::MAXIMUM_CONNECTION_LIMIT; +use serde::Deserialize; +use std::net::IpAddr; + +/// The BGSAVE configuration +/// +/// If BGSAVE is enabled, then the duration (corresponding to `every`) is wrapped in the `Enabled` +/// variant. Otherwise, the `Disabled` variant is to be used +#[derive(PartialEq, Debug)] +pub enum BGSave { + Enabled(u64), + Disabled, +} + +impl BGSave { + /// Create a new BGSAVE configuration with all the fields + pub const fn new(enabled: bool, every: u64) -> Self { + if enabled { + BGSave::Enabled(every) + } else { + BGSave::Disabled + } + } + /// The default BGSAVE configuration + /// + /// Defaults: + /// - `enabled`: true + /// - `every`: 120 + pub const fn default() -> Self { + BGSave::new(true, 120) + } +} + +/// A `ConfigurationSet` which can be used by main::check_args_or_connect() to bind +/// to a `TcpListener` and show the corresponding terminal output for the given +/// configuration +#[derive(Debug, PartialEq)] +pub struct ConfigurationSet { + /// If `noart` is set to true, no terminal artwork should be displayed + pub noart: bool, + /// The BGSAVE configuration + pub bgsave: BGSave, + /// The snapshot configuration + pub snapshot: SnapshotConfig, + /// Port configuration + pub ports: PortConfig, + /// The maximum number of connections + pub maxcon: usize, +} + +impl ConfigurationSet { + /// Create a default `ConfigurationSet` with the following setup defaults: + /// - `host`: 127.0.0.1 + /// - `port` : 2003 + /// - `noart` : false + /// - `bgsave_enabled` : true + /// - `bgsave_duration` : 120 + /// - `ssl` : disabled + pub const fn default() -> Self { + ConfigurationSet { + noart: false, + bgsave: BGSave::default(), + snapshot: SnapshotConfig::default(), + ports: PortConfig::new_insecure_only(DEFAULT_IPV4, 2003), + maxcon: MAXIMUM_CONNECTION_LIMIT, + } + } + /// Returns `false` if `noart` is enabled. Otherwise it returns `true` + pub const fn is_artful(&self) -> bool { + !self.noart + } +} + +/// Port configuration +/// +/// This enumeration determines whether the ports are: +/// - `Multi`: This means that the database server will be listening to both +/// SSL **and** non-SSL requests +/// - `SecureOnly` : This means that the database server will only accept SSL requests +/// and will not even activate the non-SSL socket +/// - `InsecureOnly` : This indicates that the server would only accept non-SSL connections +/// and will not even activate the SSL socket +#[derive(Debug, PartialEq)] +pub enum PortConfig { + SecureOnly { + host: IpAddr, + ssl: SslOpts, + }, + Multi { + host: IpAddr, + port: u16, + ssl: SslOpts, + }, + InsecureOnly { + host: IpAddr, + port: u16, + }, +} + +impl Default for PortConfig { + fn default() -> PortConfig { + PortConfig::InsecureOnly { + host: DEFAULT_IPV4, + port: DEFAULT_PORT, + } + } +} + +impl PortConfig { + pub const fn new_secure_only(host: IpAddr, ssl: SslOpts) -> Self { + PortConfig::SecureOnly { host, ssl } + } + pub const fn new_insecure_only(host: IpAddr, port: u16) -> Self { + PortConfig::InsecureOnly { host, port } + } + pub fn get_host(&self) -> IpAddr { + match self { + Self::InsecureOnly { host, .. } + | Self::SecureOnly { host, .. } + | Self::Multi { host, .. } => *host, + } + } + pub fn upgrade_to_tls(&mut self, ssl: SslOpts) { + match self { + Self::InsecureOnly { host, port } => { + *self = Self::Multi { + host: *host, + port: *port, + ssl, + } + } + Self::SecureOnly { .. } | Self::Multi { .. } => { + panic!("Port config is already upgraded to TLS") + } + } + } +} + +#[derive(Deserialize, Debug, PartialEq)] +pub struct SslOpts { + pub key: String, + pub chain: String, + pub port: u16, + pub passfile: Option, +} + +impl SslOpts { + pub const fn new(key: String, chain: String, port: u16, passfile: Option) -> Self { + SslOpts { + key, + chain, + port, + passfile, + } + } +} + +#[derive(Debug, PartialEq)] +/// The snapshot configuration +/// +pub struct SnapshotPref { + /// Capture a snapshot `every` seconds + pub every: u64, + /// The maximum numeber of snapshots to be kept + pub atmost: usize, + /// Lock writes if snapshotting fails + pub poison: bool, +} + +impl SnapshotPref { + /// Create a new a new `SnapshotPref` instance + pub const fn new(every: u64, atmost: usize, poison: bool) -> Self { + SnapshotPref { + every, + atmost, + poison, + } + } + /// Returns `every,almost` as a tuple for pattern matching + pub const fn decompose(self) -> (u64, usize, bool) { + (self.every, self.atmost, self.poison) + } +} + +#[derive(Debug, PartialEq)] +/// Snapshotting configuration +/// +/// The variant `Enabled` directly carries a `ConfigKeySnapshot` object that +/// is parsed from the configuration file, The variant `Disabled` is a ZST, and doesn't +/// hold any data +pub enum SnapshotConfig { + /// Snapshotting is enabled: this variant wraps around a `SnapshotPref` + /// object + Enabled(SnapshotPref), + /// Snapshotting is disabled + Disabled, +} + +impl SnapshotConfig { + /// Snapshots are disabled by default, so `SnapshotConfig::Disabled` is the + /// default configuration + pub const fn default() -> Self { + SnapshotConfig::Disabled + } +} + +type RestoreFile = Option; + +#[derive(Debug, PartialEq)] +/// The type of configuration: +/// - The default configuration +/// - A custom supplied configuration +pub struct ConfigType { + config: ConfigurationSet, + restore: RestoreFile, + is_custom: bool, + warnings: Option, +} + +impl ConfigType { + fn _new( + config: ConfigurationSet, + restore: RestoreFile, + is_custom: bool, + warnings: Option, + ) -> Self { + Self { + config, + restore, + is_custom, + warnings, + } + } + pub fn print_warnings(&self) { + if let Some(warnings) = self.warnings.as_ref() { + warnings.print_warnings() + } + } + pub fn finish(self) -> (ConfigurationSet, Option) { + (self.config, self.restore) + } + pub fn is_custom(&self) -> bool { + self.is_custom + } + pub fn is_artful(&self) -> bool { + self.config.is_artful() + } + pub fn new_custom( + config: ConfigurationSet, + restore: RestoreFile, + warnings: WarningStack, + ) -> Self { + Self::_new(config, restore, true, Some(warnings)) + } + pub fn new_default(restore: RestoreFile) -> Self { + Self::_new(ConfigurationSet::default(), restore, false, None) + } +} diff --git a/server/src/config/eval.rs b/server/src/config/feedback.rs similarity index 71% rename from server/src/config/eval.rs rename to server/src/config/feedback.rs index 7c0f6e2a..73ae4dcb 100644 --- a/server/src/config/eval.rs +++ b/server/src/config/feedback.rs @@ -24,15 +24,17 @@ * */ +// external imports +use toml::de::Error as TomlError; +// std imports use core::fmt; use core::ops; +#[cfg(test)] const EMSG_ENV: &str = "Environment"; -const EMSG_FILE: &str = "Configuration file"; -const EMSG_CLI: &str = "Command line arguments"; const TAB: &str = " "; -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct FeedbackStack { stack: Vec, feedback_type: &'static str, @@ -60,15 +62,17 @@ impl FeedbackStack { impl fmt::Display for FeedbackStack { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} {}:\n", self.feedback_source, self.feedback_type)?; - for err in self.stack.iter() { - writeln!(f, "{}- {}", TAB, err)?; + if !self.is_empty() { + write!(f, "{} {}:", self.feedback_source, self.feedback_type)?; + for err in self.stack.iter() { + write!(f, "\n{}- {}", TAB, err)?; + } } Ok(()) } } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct ErrorStack { feedback: FeedbackStack, } @@ -112,7 +116,7 @@ Environment errors: assert_eq!(fmt, EXPECTED); } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct WarningStack { feedback: FeedbackStack, } @@ -123,6 +127,9 @@ impl WarningStack { feedback: FeedbackStack::new(warning_source, "warnings"), } } + pub fn print_warnings(&self) { + log::warn!("{}", self); + } } impl ops::Deref for WarningStack { @@ -157,3 +164,37 @@ Environment warnings: let fmt = format!("{}", wstk); assert_eq!(fmt, EXPECTED); } + +#[derive(Debug)] +pub enum ConfigError { + OSError(std::io::Error), + CfgError(ErrorStack), + ConfigFileParseError(TomlError), + Conflict, +} + +impl PartialEq for ConfigError { + fn eq(&self, oth: &Self) -> bool { + match (self, oth) { + (Self::OSError(lhs), Self::OSError(rhs)) => lhs.to_string() == rhs.to_string(), + (Self::CfgError(lhs), Self::CfgError(rhs)) => lhs == rhs, + (Self::ConfigFileParseError(lhs), Self::ConfigFileParseError(rhs)) => lhs == rhs, + (Self::Conflict, Self::Conflict) => true, + _ => false, + } + } +} + +impl fmt::Display for ConfigError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::ConfigFileParseError(e) => write!(f, "Configuration file parse failed: {}", e), + Self::OSError(e) => write!(f, "OS Error: {}", e), + Self::CfgError(e) => write!(f, "{}", e), + Self::Conflict => write!( + f, + "Conflict error: Either provide CLI args, environment variables or a config file for configuration" + ), + } + } +} diff --git a/server/src/config/macros.rs b/server/src/config/macros.rs deleted file mode 100644 index f8a83d0a..00000000 --- a/server/src/config/macros.rs +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Created on Sat Oct 02 2021 - * - * This file is a part of Skytable - * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source - * NoSQL database written by Sayan Nandan ("the Author") with the - * vision to provide flexibility in data modelling without compromising - * on performance, queryability or scalability. - * - * Copyright (c) 2021, Sayan Nandan - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * -*/ - -macro_rules! cli_parse_or_default_or_err { - ($parsewhat:expr, $default:expr, $except:expr $(,)?) => { - match $parsewhat.map(|v| v.parse()) { - Some(Ok(v)) => v, - Some(Err(_)) => return Err(self::cfgerr::ConfigError::CliArgErr($except)), - None => $default, - } - }; -} - -macro_rules! cli_setparse_or_err { - ($setwhat:expr, $parsewhat:expr, $except:expr $(,)?) => { - match $parsewhat.map(|v| v.parse()) { - Some(Ok(v)) => $setwhat = v, - Some(Err(_)) => return Err(self::cfgerr::ConfigError::CliArgErr($except)), - _ => {} - } - }; -} - -macro_rules! set_if_exists { - ($testwhat:expr, $trywhat:expr) => { - if let Some(testwhat) = $testwhat { - $trywhat = testwhat; - } - }; -} - -macro_rules! ret_cli_err { - ($what:expr) => { - return Err(self::cfgerr::ConfigError::CliArgErr($what)) - }; -} diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 356244cb..bbb219b0 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -1,5 +1,5 @@ /* - * Created on Tue Sep 01 2020 + * Created on Thu Jan 27 2022 * * This file is a part of Skytable * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source @@ -7,7 +7,7 @@ * vision to provide flexibility in data modelling without compromising * on performance, queryability or scalability. * - * Copyright (c) 2020, Sayan Nandan + * Copyright (c) 2022, Sayan Nandan * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by @@ -24,544 +24,542 @@ * */ -//! This module provides tools to handle configuration files and settings - -use crate::dbnet::MAXIMUM_CONNECTION_LIMIT; -use clap::ArgMatches; +// external imports use clap::{load_yaml, App}; -use serde::Deserialize; +// std imports +use core::str::FromStr; +use std::env::VarError; use std::fs; use std::net::{IpAddr, Ipv4Addr}; -// modules -#[macro_use] -mod macros; +// internal modules +mod cfgcli; mod cfgenv; -mod cfgerr; mod cfgfile; -// TODO: Upgrade to use these modules -mod cfg2; -mod cfgcli2; -mod cfgenv2; -mod cfgfile2; -mod eval; +mod definitions; +mod feedback; #[cfg(test)] mod tests; -// self imports -use self::cfgerr::{ConfigError, ERR_CONFLICT}; +// internal imports +use self::cfgfile::Config as ConfigFile; +pub use self::definitions::*; +use self::feedback::{ConfigError, ErrorStack, WarningStack}; +use crate::dbnet::MAXIMUM_CONNECTION_LIMIT; +// server defaults const DEFAULT_IPV4: IpAddr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); -const DEFAULT_SSL_PORT: u16 = 2004; const DEFAULT_PORT: u16 = 2003; +// bgsave defaults +const DEFAULT_BGSAVE_DURATION: u64 = 120; +// snapshot defaults +const DEFAULT_SNAPSHOT_FAILSAFE: bool = true; +// TLS defaults +const DEFAULT_SSL_PORT: u16 = 2004; -/// The BGSAVE configuration -/// -/// If BGSAVE is enabled, then the duration (corresponding to `every`) is wrapped in the `Enabled` -/// variant. Otherwise, the `Disabled` variant is to be used -#[derive(PartialEq, Debug)] -pub enum BGSave { - Enabled(u64), - Disabled, -} +type StaticStr = &'static str; -impl BGSave { - /// Create a new BGSAVE configuration with all the fields - pub const fn new(enabled: bool, every: u64) -> Self { - if enabled { - BGSave::Enabled(every) - } else { - BGSave::Disabled - } - } - /// The default BGSAVE configuration - /// - /// Defaults: - /// - `enabled`: true - /// - `every`: 120 - pub const fn default() -> Self { - BGSave::new(true, 120) - } - /// If `self` is a `Disabled` variant, then BGSAVE has been disabled by the user - pub const fn is_disabled(&self) -> bool { - matches!(self, BGSave::Disabled) - } +#[derive(Debug)] +/// An enum representing the outcome of a parse operation for a specific configuration item from a +/// specific configuration source +pub enum ConfigSourceParseResult { + Okay(T), + Absent, + ParseFailure, } -/// Port configuration -/// -/// This enumeration determines whether the ports are: -/// - `Multi`: This means that the database server will be listening to both -/// SSL **and** non-SSL requests -/// - `SecureOnly` : This means that the database server will only accept SSL requests -/// and will not even activate the non-SSL socket -/// - `InsecureOnly` : This indicates that the server would only accept non-SSL connections -/// and will not even activate the SSL socket -#[derive(Debug, PartialEq)] -pub enum PortConfig { - SecureOnly { - host: IpAddr, - ssl: SslOpts, - }, - Multi { - host: IpAddr, - port: u16, - ssl: SslOpts, - }, - InsecureOnly { - host: IpAddr, - port: u16, - }, +/// A trait for configuration sources. Any type implementing this trait is considered to be a valid +/// source for configuration +pub trait TryFromConfigSource: Sized { + /// Check if the value is present + fn is_present(&self) -> bool; + /// Attempt to mutate the value if present. If any error occurs + /// while parsing the value, return true. Else return false if all went well. + /// Here: + /// - `target_value`: is a mutable reference to the target var + /// - `trip`: is a mutable ref to a bool that will be set to true if a value is present + /// (whether parseable or not) + fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool; + /// Attempt to parse the value into the target type + fn try_parse(self) -> ConfigSourceParseResult; } -#[cfg(test)] -impl PortConfig { - pub const fn default() -> PortConfig { - PortConfig::InsecureOnly { - host: DEFAULT_IPV4, - port: DEFAULT_PORT, - } +impl<'a, T: FromStr + 'a> TryFromConfigSource for Option<&'a str> { + fn is_present(&self) -> bool { + self.is_some() } -} - -impl PortConfig { - pub const fn new_secure_only(host: IpAddr, ssl: SslOpts) -> Self { - PortConfig::SecureOnly { host, ssl } - } - pub const fn new_insecure_only(host: IpAddr, port: u16) -> Self { - PortConfig::InsecureOnly { host, port } + fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { + self.map(|slf| { + *trip = true; + match slf.parse() { + Ok(p) => { + *target_value = p; + false + } + Err(_) => true, + } + }) + .unwrap_or(false) } - pub const fn new_multi(host: IpAddr, port: u16, ssl: SslOpts) -> Self { - PortConfig::Multi { host, port, ssl } + fn try_parse(self) -> ConfigSourceParseResult { + self.map(|s| { + s.parse() + .map(|ret| ConfigSourceParseResult::Okay(ret)) + .unwrap_or(ConfigSourceParseResult::ParseFailure) + }) + .unwrap_or(ConfigSourceParseResult::Absent) } - pub fn get_host(&self) -> IpAddr { - match self { - Self::InsecureOnly { host, .. } - | Self::SecureOnly { host, .. } - | Self::Multi { host, .. } => *host, - } +} + +impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { + fn is_present(&self) -> bool { + !matches!(self, Err(VarError::NotPresent)) } - pub fn upgrade_to_tls(&mut self, ssl: SslOpts) { + fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { match self { - Self::InsecureOnly { host, port } => { - *self = Self::Multi { - host: *host, - port: *port, - ssl, + Ok(s) => s + .parse() + .map(|v| { + *trip = true; + *target_value = v; + false + }) + .unwrap_or(true), + Err(e) => { + if matches!(e, VarError::NotPresent) { + false + } else { + // yes, we got the var but failed to parse it into unicode; so trip + *trip = true; + true } } - Self::SecureOnly { .. } | Self::Multi { .. } => { - panic!("Port config is already upgraded to TLS") - } } } -} - -#[derive(Deserialize, Debug, PartialEq)] -pub struct SslOpts { - pub key: String, - pub chain: String, - pub port: u16, - pub passfile: Option, -} - -impl SslOpts { - pub const fn new(key: String, chain: String, port: u16, passfile: Option) -> Self { - SslOpts { - key, - chain, - port, - passfile, + fn try_parse(self) -> ConfigSourceParseResult { + match self { + Ok(s) => s + .parse() + .map(|v| ConfigSourceParseResult::Okay(v)) + .unwrap_or(ConfigSourceParseResult::ParseFailure), + Err(e) => match e { + VarError::NotPresent => ConfigSourceParseResult::Absent, + VarError::NotUnicode(_) => ConfigSourceParseResult::ParseFailure, + }, } } } -#[derive(Debug, PartialEq)] -/// The snapshot configuration -/// -pub struct SnapshotPref { - /// Capture a snapshot `every` seconds - pub every: u64, - /// The maximum numeber of snapshots to be kept - pub atmost: usize, - /// Lock writes if snapshotting fails - pub poison: bool, +#[derive(Debug)] +/// Since we have conflicting trait implementations, we define a custom `Option` type +pub struct OptString { + base: Option, } -impl SnapshotPref { - /// Create a new a new `SnapshotPref` instance - pub const fn new(every: u64, atmost: usize, poison: bool) -> Self { - SnapshotPref { - every, - atmost, - poison, - } +impl OptString { + pub const fn new_null() -> Self { + Self { base: None } } - /// Returns `every,almost` as a tuple for pattern matching - pub const fn decompose(self) -> (u64, usize, bool) { - (self.every, self.atmost, self.poison) - } -} - -#[derive(Debug, PartialEq)] -/// Snapshotting configuration -/// -/// The variant `Enabled` directly carries a `ConfigKeySnapshot` object that -/// is parsed from the configuration file, The variant `Disabled` is a ZST, and doesn't -/// hold any data -pub enum SnapshotConfig { - /// Snapshotting is enabled: this variant wraps around a `SnapshotPref` - /// object - Enabled(SnapshotPref), - /// Snapshotting is disabled - Disabled, } -impl SnapshotConfig { - /// Snapshots are disabled by default, so `SnapshotConfig::Disabled` is the - /// default configuration - pub const fn default() -> Self { - SnapshotConfig::Disabled +impl From> for OptString { + fn from(base: Option) -> Self { + Self { base } } } -/// A `ConfigurationSet` which can be used by main::check_args_or_connect() to bind -/// to a `TcpListener` and show the corresponding terminal output for the given -/// configuration -#[derive(Debug, PartialEq)] -pub struct ConfigurationSet { - /// If `noart` is set to true, no terminal artwork should be displayed - noart: bool, - /// The BGSAVE configuration - pub bgsave: BGSave, - /// The snapshot configuration - pub snapshot: SnapshotConfig, - /// Port configuration - pub ports: PortConfig, - /// The maximum number of connections - pub maxcon: usize, +impl FromStr for OptString { + type Err = (); + fn from_str(st: &str) -> Result { + Ok(Self { + base: Some(st.to_string()), + }) + } } -impl ConfigurationSet { - /// Create a new `ConfigurationSet` from a given file in `location` - pub fn new_from_file(location: String) -> Result { - let file = fs::read_to_string(location)?; - let r = toml::from_str(&file).map(ConfigurationSet::from_config)?; - Ok(r) - } - /// Create a `ConfigurationSet` instance from a `Config` object, which is a parsed - /// TOML file (represented as an object) - fn from_config(cfg_info: cfgfile::Config) -> Self { - let mut cfg = Self::default(); - set_if_exists!(cfg_info.server.noart, cfg.noart); - if let Some(bgsave) = cfg_info.bgsave { - let bgsave_ret = match (bgsave.enabled, bgsave.every) { - // TODO: Show a warning that there are unused keys - (Some(enabled), Some(every)) => BGSave::new(enabled, every), - (Some(enabled), None) => BGSave::new(enabled, 120), - (None, Some(every)) => BGSave::new(true, every), - (None, None) => BGSave::default(), - }; - cfg.bgsave = bgsave_ret; - } - if let Some(snapshot) = cfg_info.snapshot { - cfg.snapshot = SnapshotConfig::Enabled(SnapshotPref::new( - snapshot.every, - snapshot.atmost, - option_unwrap_or!(snapshot.failsafe, true), - )); - } - if let Some(sslopts) = cfg_info.ssl { - let portcfg = if sslopts.only.unwrap_or_default() { - PortConfig::SecureOnly { - ssl: SslOpts { - key: sslopts.key, - chain: sslopts.chain, - port: sslopts.port, - passfile: sslopts.passin, - }, - host: cfg_info.server.host, - } - } else { - PortConfig::Multi { - ssl: SslOpts { - key: sslopts.key, - chain: sslopts.chain, - port: sslopts.port, - passfile: sslopts.passin, - }, - host: cfg_info.server.host, - port: cfg_info.server.port, - } - }; - cfg.ports = portcfg; - } else { - // make sure we check for portcfg for non-TLS connections - cfg.ports = PortConfig::new_insecure_only(cfg_info.server.host, cfg_info.server.port); - } - set_if_exists!(cfg_info.server.maxclient, cfg.maxcon); - cfg - } - #[cfg(test)] - /// Create a new `ConfigurationSet` from a `TOML` string - pub fn new_from_toml_str(tomlstr: String) -> tests::TResult { - Ok(ConfigurationSet::from_config(toml::from_str(&tomlstr)?)) - } - #[cfg(test)] - /// Create a new `ConfigurationSet` with all the fields - pub const fn new( - noart: bool, - bgsave: BGSave, - snapshot: SnapshotConfig, - ports: PortConfig, - maxcon: usize, - ) -> Self { - ConfigurationSet { - noart, - bgsave, - snapshot, - ports, - maxcon, - } +impl TryFromConfigSource for OptString { + fn is_present(&self) -> bool { + self.base.is_some() } - /// Create a default `ConfigurationSet` with the following setup defaults: - /// - `host`: 127.0.0.1 - /// - `port` : 2003 - /// - `noart` : false - /// - `bgsave_enabled` : true - /// - `bgsave_duration` : 120 - /// - `ssl` : disabled - pub const fn default() -> Self { - ConfigurationSet { - noart: false, - bgsave: BGSave::default(), - snapshot: SnapshotConfig::default(), - ports: PortConfig::new_insecure_only(DEFAULT_IPV4, 2003), - maxcon: MAXIMUM_CONNECTION_LIMIT, + fn mutate_failed(self, target: &mut OptString, trip: &mut bool) -> bool { + if let Some(v) = self.base { + target.base = Some(v); + *trip = true; } + false } - /// Returns `false` if `noart` is enabled. Otherwise it returns `true` - pub const fn is_artful(&self) -> bool { - !self.noart + fn try_parse(self) -> ConfigSourceParseResult { + self.base + .map(|v| ConfigSourceParseResult::Okay(OptString { base: Some(v) })) + .unwrap_or(ConfigSourceParseResult::Okay(OptString::new_null())) } } -/// The type of configuration: -/// - We either used a custom configuration file given to us by the user (`Custom`) OR -/// - We used the default configuration (`Def`) -/// -/// The second field in the tuple is for the restore file, if there was any #[derive(Debug)] -pub enum ConfigType { - Def(ConfigurationSet, Option), - Custom(ConfigurationSet, Option), +/// A high-level configuration set that automatically handles errors, warnings and provides a convenient [`Result`] +/// type that can be used +pub struct Configset { + did_mutate: bool, + cfg: ConfigurationSet, + estack: ErrorStack, + wstack: WarningStack, } -/// This function returns a `ConfigType` -/// -/// This parses a configuration file if it is supplied as a command line argument -/// or it returns the default configuration. **If** the configuration file -/// contains an error, then this returns it as an `Err` variant -pub fn get_config_file_or_return_cfg() -> Result { - let cfg_layout = load_yaml!("../cli.yml"); - let matches = App::from_yaml(cfg_layout).get_matches(); - self::get_config_file_or_return_cfg_from_matches(matches) -} - -// this method simply allows us to simplify tests for conflicts -fn get_config_file_or_return_cfg_from_matches( - matches: ArgMatches, -) -> Result { - let no_cli_args = matches.args.is_empty(); // check cli args - let env_args = cfgenv::get_env_config()?; - if no_cli_args && env_args.is_none() { - // that means we need to use the default config - return Ok(ConfigType::Def(ConfigurationSet::default(), None)); - } +impl Configset { + const EMSG_ENV: StaticStr = "Environment"; + const EMSG_CLI: StaticStr = "CLI arguments"; + const EMSG_FILE: StaticStr = "Configuration file"; - // Check if there is a config file - let filename = matches.value_of("config"); - let restorefile = matches.value_of("restore").map(|v| v.to_string()); - let cfg = if let Some(filename) = filename { - // so we have a config file; let's confirm that we don't have any other arguments - // either no restore file and len greater than 1; or restore file is some, and args greater - // than 2 - let is_conflict = (restorefile.is_none() - && (matches.args.len() > 1 || matches.subcommand.is_some())) - || (restorefile.is_some() && (matches.args.len() > 2 || matches.subcommand.is_some())); - let is_conflict = is_conflict || env_args.is_some(); - if is_conflict { - // nope, more args were passed; error - return Err(ConfigError::CfgError(ERR_CONFLICT)); + /// Internal ctor for a given feedback source. We do not want to expose this to avoid + /// erroneous feedback source names + fn _new(feedback_source: StaticStr) -> Self { + Self { + did_mutate: false, + cfg: ConfigurationSet::default(), + estack: ErrorStack::new(feedback_source), + wstack: WarningStack::new(feedback_source), } - ConfigurationSet::new_from_file(filename.to_owned()) - } else { - if env_args.is_some() && !matches.args.is_empty() { - // so we have env args and some CLI args? that's a conflict - return Err(ConfigError::CfgError(ERR_CONFLICT)); + } + /// Create a new configset for environment variables + pub fn new_env() -> Self { + Self::_new(Self::EMSG_ENV) + } + /// Create a new configset for CLI args + pub fn new_cli() -> Self { + Self::_new(Self::EMSG_CLI) + } + /// Create a new configset for config files + pub fn new_file() -> Self { + Self { + did_mutate: true, + cfg: ConfigurationSet::default(), + estack: ErrorStack::new(Self::EMSG_FILE), + wstack: WarningStack::new(Self::EMSG_FILE), } - if let Some(env_args) = env_args { - // we are sure that we just have env args - Ok(env_args) - } else { - // we are sure that we just have CLI args - parse_cli_args(matches) + } + /// Mark the configset mutated + fn mutated(&mut self) { + self.did_mutate = true; + } + /// Push an error onto the error stack + fn epush(&mut self, field_key: StaticStr, expected: StaticStr) { + self.estack.push(format!( + "Bad value for `${field_key}`. Expected ${expected}", + )) + } + /// Check if no errors have occurred + pub fn is_okay(&self) -> bool { + self.estack.is_empty() + } + /// Check if the configset was mutated + pub fn is_mutated(&self) -> bool { + self.did_mutate + } + /// Attempt to mutate with a target `TryFromConfigSource` type, and push in any error that occurs + /// using the given diagnostic info + fn try_mutate( + &mut self, + new: impl TryFromConfigSource, + target: &mut T, + field_key: StaticStr, + expected: StaticStr, + ) { + if new.mutate_failed(target, &mut self.did_mutate) { + self.epush(field_key, expected) } - }?; - // now validate - if cfg.bgsave.is_disabled() { - log::warn!("BGSAVE is disabled: If this system crashes unexpectedly, it may lead to the loss of data"); } - if let SnapshotConfig::Enabled(e) = &cfg.snapshot { - if e.every == 0 { - return Err(ConfigError::CfgError( - "The snapshot duration has to be greater than 0!", - )); + /// Attempt to mutate with a target `TryFromConfigSource` type, and push in any error that occurs + /// using the given diagnostic info while checking the correctly parsed type using the provided validation + /// closure for any additional validation check that goes beyond type correctness + fn try_mutate_with_condcheck( + &mut self, + new: impl TryFromConfigSource, + target: &mut T, + field_key: StaticStr, + expected: StaticStr, + validation_fn: F, + ) where + F: Fn(&T) -> bool, + { + let mut needs_error = false; + match new.try_parse() { + ConfigSourceParseResult::Okay(ok) => { + self.mutated(); + needs_error = !validation_fn(&ok); + *target = ok; + } + ConfigSourceParseResult::ParseFailure => needs_error = true, + ConfigSourceParseResult::Absent => {} + } + if needs_error { + self.epush(field_key, expected) } } - if let BGSave::Enabled(dur) = &cfg.bgsave { - if *dur == 0 { - return Err(ConfigError::CfgError( - "The BGSAVE duration has to be greater than 0!", - )); + /// This method can be used to chain configurations to ultimately return the first modified configuration + /// that occurs. For example: `cfg_file.and_then(cfg_cli).and_then(cfg_env)`; it will return the first + /// modified Configset + /// + /// ## Panics + /// This method will panic if both the provided sets are mutated. Hence, you need to check beforehand that + /// there is no conflict + pub fn and_then(self, other: Self) -> Self { + if self.is_mutated() { + if other.is_mutated() { + panic!( + "Double mutation: {env_a} and {env_b}", + env_a = self.estack.source(), + env_b = other.estack.source() + ); + } + self + } else { + other } } - Ok(ConfigType::Custom(cfg, restorefile)) } -fn parse_cli_args(matches: ArgMatches) -> Result { - let mut cfg = ConfigurationSet::default(); - // Check flags - let sslonly = matches.is_present("sslonly"); - let noart = matches.is_present("noart"); - let nosave = matches.is_present("nosave"); - // check options - let host = matches.value_of("host"); - let port = matches.value_of("port"); - let sslport = matches.value_of("sslport"); - let custom_ssl_port = sslport.is_some(); - let snapevery = matches.value_of("snapevery"); - let snapkeep = matches.value_of("snapkeep"); - let saveduration = matches.value_of("saveduration"); - let sslkey = matches.value_of("sslkey"); - let sslchain = matches.value_of("sslchain"); - let maxcon = matches.value_of("maxcon"); - let passfile = matches.value_of("tlspassin"); +// server settings +impl Configset { + pub fn server_tcp( + &mut self, + nhost: impl TryFromConfigSource, + nhost_key: StaticStr, + nport: impl TryFromConfigSource, + nport_key: StaticStr, + ) { + let mut host = DEFAULT_IPV4; + let mut port = DEFAULT_PORT; + self.try_mutate(nhost, &mut host, nhost_key, "an IPv4/IPv6 address"); + self.try_mutate(nport, &mut port, nport_key, "a 16-bit positive integer"); + self.cfg.ports = PortConfig::new_insecure_only(host, port); + } + pub fn server_noart(&mut self, nart: impl TryFromConfigSource, nart_key: StaticStr) { + let mut noart = false; + self.try_mutate(nart, &mut noart, nart_key, "true/false"); + self.cfg.noart = noart; + } + pub fn server_maxcon( + &mut self, + nmaxcon: impl TryFromConfigSource, + nmaxcon_key: StaticStr, + ) { + let mut maxcon = MAXIMUM_CONNECTION_LIMIT; + self.try_mutate_with_condcheck( + nmaxcon, + &mut maxcon, + nmaxcon_key, + "a positive integer greater than zero", + |max| *max > 0, + ); + self.cfg.maxcon = maxcon; + } +} - cfg.noart = noart; - let port: u16 = cli_parse_or_default_or_err!( - port, - 2003, - "Invalid value for `--port`. Expected an unsigned 16-bit integer" - ); - let host: IpAddr = cli_parse_or_default_or_err!( - host, - DEFAULT_IPV4, - "Invalid value for `--host`. Expected a valid IPv4 or IPv6 address" - ); - let sslport: u16 = cli_parse_or_default_or_err!( - sslport, - DEFAULT_SSL_PORT, - "Invalid value for `--sslport`. Expected a valid unsigned 16-bit integer" - ); - cli_setparse_or_err!( - cfg.maxcon, - maxcon, - "Invalid value for `--maxcon`. Expected a valid positive integer" - ); - if nosave { - if saveduration.is_some() { - // If there is both `nosave` and `saveduration` - the arguments aren't logically correct! - // How would we run BGSAVE in a given `saveduration` if it is disabled? Return an error - ret_cli_err!("Invalid options for BGSAVE. Either supply `--nosave` or `--saveduration` or nothing"); - } - // It is our responsibility to keep the user aware of bad settings, so we'll send a warning - log::warn!("BGSAVE is disabled. You might lose data if the host crashes"); - cfg.bgsave = BGSave::Disabled; - } else if let Some(dur) = saveduration { - // A duration is specified for BGSAVE, so use it - cfg.bgsave = match dur.parse() { - Ok(duration) => BGSave::new(true, duration), - Err(_) => { - ret_cli_err!( - "Invalid value for `--saveduration`. Expected an unsigned 64-bit integer" - ) +// bgsave settings +impl Configset { + pub fn bgsave_settings( + &mut self, + nenabled: impl TryFromConfigSource, + nenabled_key: StaticStr, + nduration: impl TryFromConfigSource, + nduration_key: StaticStr, + ) { + let mut enabled = true; + let mut duration = DEFAULT_BGSAVE_DURATION; + let has_custom_duration = nduration.is_present(); + self.try_mutate(nenabled, &mut enabled, nenabled_key, "true/false"); + self.try_mutate_with_condcheck( + nduration, + &mut duration, + nduration_key, + "a positive integer greater than zero", + |dur| *dur > 0, + ); + if enabled { + self.cfg.bgsave = BGSave::Enabled(duration); + } else { + if has_custom_duration { + self.wstack.push(format!( + "Specifying `${nduration_key}` is useless when BGSAVE is disabled" + )); } - }; + self.wstack + .push("BGSAVE is disabled. You may lose data if the host crashes"); + } } - // check snapshot configuration - let snapevery: Option = match snapevery { - Some(dur) => match dur.parse() { - Ok(dur) => Some(dur), - Err(_) => { - ret_cli_err!("Invalid value for `--snapevery`. Expected an unsigned 64-bit integer") +} + +// snapshot settings +impl Configset { + pub fn snapshot_settings( + &mut self, + nevery: impl TryFromConfigSource, + nevery_key: StaticStr, + natmost: impl TryFromConfigSource, + natmost_key: StaticStr, + nfailsafe: impl TryFromConfigSource, + nfailsafe_key: StaticStr, + ) { + match (nevery.is_present(), natmost.is_present()) { + (false, false) => { + // noice, disabled + if nfailsafe.is_present() { + // this mutation is pointless, but it is just for the sake of making sure + // that the `failsafe` key has a proper boolean, no matter if it is pointless + let mut _failsafe = DEFAULT_SNAPSHOT_FAILSAFE; + self.try_mutate(nfailsafe, &mut _failsafe, nfailsafe_key, "true/false"); + self.wstack.push(format!( + "Specifying `${nfailsafe_key}` is usless when snapshots are disabled" + )); + } } - }, - None => None, - }; - let snapkeep: Option = match snapkeep { - Some(maxtop) => match maxtop.parse() { - Ok(maxtop) => Some(maxtop), - Err(_) => { - ret_cli_err!("Invalid value for `--snapkeep`. Expected an unsigned 64-bit integer") + (true, true) => { + let mut every = 0; + let mut atmost = 0; + let mut failsafe = DEFAULT_SNAPSHOT_FAILSAFE; + self.try_mutate_with_condcheck( + nevery, + &mut every, + nevery_key, + "an integer greater than 0", + |dur| *dur > 0, + ); + self.try_mutate( + natmost, + &mut atmost, + natmost_key, + "a positive integer. 0 indicates that all snapshots will be kept", + ); + self.try_mutate(nfailsafe, &mut failsafe, nfailsafe_key, "true/false"); + self.cfg.snapshot = + SnapshotConfig::Enabled(SnapshotPref::new(every, atmost, failsafe)); } - }, - None => None, - }; - let failsafe = if let Ok(failsafe) = option_unwrap_or!( - matches - .value_of("stop-write-on-fail") - .map(|val| val.parse::()), - Ok(true) - ) { - failsafe - } else { - ret_cli_err!("Please provide a boolean `true` or `false` value to --stop-write-on-fail"); - }; - cfg.snapshot = match (snapevery, snapkeep) { - (Some(every), Some(keep)) => { - SnapshotConfig::Enabled(SnapshotPref::new(every, keep, failsafe)) - } - (None, None) => SnapshotConfig::Disabled, - _ => { - ret_cli_err!( - "No value supplied for `--snapevery`. When you supply `--snapkeep`, you also need to specify `--snapevery`" - ) + (false, true) | (true, false) => self.estack.push(format!( + "To use snapshots, pass values for both `${nevery_key}` and `${natmost_key}`" + )), } - }; - // check port config - let portcfg = match ( - sslkey.map(|val| val.to_owned()), - sslchain.map(|val| val.to_owned()), + } +} + +// TLS settings +impl Configset { + pub fn tls_settings( + &mut self, + nkey: impl TryFromConfigSource, + nkey_key: StaticStr, + ncert: impl TryFromConfigSource, + ncert_key: StaticStr, + nport: impl TryFromConfigSource, + nport_key: StaticStr, + nonly: impl TryFromConfigSource, + nonly_key: StaticStr, + npass: impl TryFromConfigSource, + npass_key: StaticStr, ) { - (None, None) => { - if sslonly { - ret_cli_err!( - "You mast pass values for both --sslkey and --sslchain to use the --sslonly flag" + match (nkey.is_present(), ncert.is_present()) { + (true, true) => { + // get the cert details + let mut key = String::new(); + let mut cert = String::new(); + self.try_mutate(nkey, &mut key, nkey_key, "path to private key file"); + self.try_mutate(ncert, &mut cert, ncert_key, "path to TLS certificate file"); + + // now get port info + let mut port = DEFAULT_SSL_PORT; + self.try_mutate(nport, &mut port, nport_key, "a positive 16-bit integer"); + + // now check if TLS only + let mut tls_only = false; + self.try_mutate(nonly, &mut tls_only, nonly_key, "true/false"); + + // check if we have a TLS cert + let mut tls_pass = OptString::new_null(); + self.try_mutate( + npass, + &mut tls_pass, + npass_key, + "path to TLS cert passphrase", ); - } else { - if custom_ssl_port { - log::warn!("Ignoring value for `--sslport` as TLS was not enabled"); + + let sslopts = SslOpts::new(key, cert, port, tls_pass.base); + // now check if TLS only + if tls_only { + let host = self.cfg.ports.get_host(); + self.cfg.ports = PortConfig::new_secure_only(host, sslopts) + } else { + // multi. go and upgrade existing + self.cfg.ports.upgrade_to_tls(sslopts); } - PortConfig::new_insecure_only(host, port) } - } - (Some(key), Some(chain)) => { - if sslonly { - PortConfig::new_secure_only( - host, - SslOpts::new(key, chain, sslport, passfile.map(|v| v.to_string())), - ) - } else { - PortConfig::new_multi( - host, - port, - SslOpts::new(key, chain, sslport, passfile.map(|v| v.to_string())), - ) + (true, false) | (false, true) => { + self.estack.push(format!( + "To use TLS, pass values for both `${nkey_key}` and `${ncert_key}`" + )); + } + (false, false) => { + if nport.is_present() { + self.wstack + .push("Specifying `${nport_key}` is pointless when TLS is disabled"); + } + if nonly.is_present() { + self.wstack + .push("Specifying `${nonly_key}` is pointless when TLS is disabled"); + } + if npass.is_present() { + self.wstack + .push("Specifying `${npass_key}` is pointless when TLS is disabled"); + } } } - _ => { - ret_cli_err!("To use TLS, pass values for both --sslkey and --sslchain"); - } + } +} + +pub fn get_config() -> Result { + // initialize clap because that will let us check for CLI/file configs + let cfg_layout = load_yaml!("../cli.yml"); + let matches = App::from_yaml(cfg_layout).get_matches(); + let restore_file = matches.value_of("restore").map(|v| v.to_string()); + + // get config from file + let cfg_from_file = if let Some(file) = matches.value_of("config") { + let file = match fs::read(file) { + Ok(f) => f, + Err(e) => return Err(ConfigError::OSError(e)), + }; + let cfg_file: ConfigFile = match toml::from_slice(&file) { + Ok(cfg) => cfg, + Err(e) => return Err(ConfigError::ConfigFileParseError(e)), + }; + Some(cfgfile::from_file(cfg_file)) + } else { + None }; - cfg.ports = portcfg; - Ok(cfg) + + // get config from CLI + let cfg_from_cli = cfgcli::parse_cli_args(matches); + // get config from env + let cfg_from_env = cfgenv::parse_env_config(); + // calculate the number of config sources + let cfg_degree = cfg_from_cli.is_mutated() as u8 + + cfg_from_env.is_mutated() as u8 + + cfg_from_file.is_some() as u8; + // if degree is more than 1, there is a conflict + let has_conflict = cfg_degree > 1; + if has_conflict { + return Err(ConfigError::Conflict); + } + if cfg_degree == 0 { + // no configuration, use default + Ok(ConfigType::new_default(restore_file)) + } else { + let final_config = if let Some(cfg) = cfg_from_file { + cfg + } else { + cfg_from_env.and_then(cfg_from_cli) + }; + if final_config.is_okay() { + let Configset { cfg, wstack, .. } = final_config; + return Ok(ConfigType::new_custom(cfg, restore_file, wstack)); + } else { + return Err(ConfigError::CfgError(final_config.estack)); + } + } } diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 4d55847a..4dc68e49 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -24,248 +24,83 @@ * */ -use super::cfgerr::{ConfigError, ERR_CONFLICT}; -use super::{ - BGSave, ConfigurationSet, IpAddr, PortConfig, SnapshotConfig, SnapshotPref, SslOpts, - DEFAULT_IPV4, DEFAULT_PORT, MAXIMUM_CONNECTION_LIMIT, -}; -use clap::{load_yaml, App}; +use super::{Configset, PortConfig, DEFAULT_IPV4}; pub(super) use libsky::TResult; use std::fs; -use std::net::Ipv6Addr; #[test] -fn test_config_toml_okayport() { - let file = r#" - [server] - host = "127.0.0.1" - port = 2003 -"# - .to_owned(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); - assert_eq!(cfg, ConfigurationSet::default(),); -} -/// Gets a `toml` file from `WORKSPACEROOT/examples/config-files` -fn get_toml_from_examples_dir(filename: String) -> TResult { - use std::path; - let curdir = std::env::current_dir().unwrap(); - let workspaceroot = curdir.ancestors().nth(1).unwrap(); - let mut fileloc = path::PathBuf::from(workspaceroot); - fileloc.push("examples"); - fileloc.push("config-files"); - fileloc.push(filename); - Ok(fs::read_to_string(fileloc)?) -} - -#[test] -fn test_config_toml_badport() { - let file = r#" - [server] - port = 20033002 -"# - .to_owned(); - let cfg = ConfigurationSet::new_from_toml_str(file); - assert!(cfg.is_err()); -} - -#[test] -fn test_config_file_ok() { - let file = get_toml_from_examples_dir("skyd.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); - assert_eq!(cfg, ConfigurationSet::default()); -} - -#[test] -fn test_config_file_err() { - let file = get_toml_from_examples_dir("skyd.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_file(file); - assert!(cfg.is_err()); -} -#[test] -fn test_args() { - let cmdlineargs = vec!["skyd", "--withconfig", "../examples/config-files/skyd.toml"]; - let cfg_layout = load_yaml!("../cli.yml"); - let matches = App::from_yaml(cfg_layout).get_matches_from(cmdlineargs); - let filename = matches.value_of("config").unwrap(); - assert_eq!("../examples/config-files/skyd.toml", filename); - let cfg = - ConfigurationSet::new_from_toml_str(std::fs::read_to_string(filename).unwrap()).unwrap(); - assert_eq!(cfg, ConfigurationSet::default()); -} - -#[test] -fn test_config_file_noart() { - let file = get_toml_from_examples_dir("secure-noart.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); - assert_eq!( - cfg, - ConfigurationSet { - noart: true, - bgsave: BGSave::default(), - snapshot: SnapshotConfig::default(), - ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT - } +fn server_tcp() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("127.0.0.1"), + "SKY_SERVER_HOST", + Some("2004"), + "SKY_SERVER_PORT", ); -} - -#[test] -fn test_config_file_ipv6() { - let file = get_toml_from_examples_dir("ipv6.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); assert_eq!( - cfg, - ConfigurationSet { - noart: false, - bgsave: BGSave::default(), - snapshot: SnapshotConfig::default(), - ports: PortConfig::new_insecure_only( - IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0x1)), - DEFAULT_PORT - ), - maxcon: MAXIMUM_CONNECTION_LIMIT - } + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2004) ); + assert!(cfgset.is_mutated()); + assert!(cfgset.is_okay()); } - #[test] -fn test_config_file_template() { - let file = get_toml_from_examples_dir("template.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); - assert_eq!( - cfg, - ConfigurationSet::new( - false, - BGSave::default(), - SnapshotConfig::Enabled(SnapshotPref::new(3600, 4, true)), - PortConfig::new_secure_only( - DEFAULT_IPV4, - SslOpts::new( - "/path/to/keyfile.pem".into(), - "/path/to/chain.pem".into(), - 2004, - Some("/path/to/cert/passphrase.txt".to_owned()) - ) - ), - MAXIMUM_CONNECTION_LIMIT - ) +fn server_tcp_fail_host() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("?127.0.0.1"), + "SKY_SERVER_HOST", + Some("2004"), + "SKY_SERVER_PORT", ); -} - -#[test] -fn test_config_file_bad_bgsave_section() { - let file = get_toml_from_examples_dir("badcfg2.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file); - assert!(cfg.is_err()); -} - -#[test] -fn test_config_file_custom_bgsave() { - let file = get_toml_from_examples_dir("withcustombgsave.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); assert_eq!( - cfg, - ConfigurationSet { - noart: false, - bgsave: BGSave::new(true, 600), - snapshot: SnapshotConfig::default(), - ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT - } + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2004) ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); } - -#[test] -fn test_config_file_bgsave_enabled_only() { - /* - * This test demonstrates a case where the user just said that BGSAVE is enabled. - * In that case, we will default to the 120 second duration - */ - let file = get_toml_from_examples_dir("bgsave-justenabled.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); - assert_eq!( - cfg, - ConfigurationSet { - noart: false, - bgsave: BGSave::default(), - snapshot: SnapshotConfig::default(), - ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT - } - ) -} - -#[test] -fn test_config_file_bgsave_every_only() { - /* - * This test demonstrates a case where the user just gave the value for every - * In that case, it means BGSAVE is enabled and set to `every` seconds - */ - let file = get_toml_from_examples_dir("bgsave-justevery.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); - assert_eq!( - cfg, - ConfigurationSet { - noart: false, - bgsave: BGSave::new(true, 600), - snapshot: SnapshotConfig::default(), - ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT - } - ) -} - #[test] -fn test_config_file_snapshot() { - let file = get_toml_from_examples_dir("snapshot.toml".to_owned()).unwrap(); - let cfg = ConfigurationSet::new_from_toml_str(file).unwrap(); +fn server_tcp_fail_port() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("127.0.0.1"), + "SKY_SERVER_HOST", + Some("65537"), + "SKY_SERVER_PORT", + ); assert_eq!( - cfg, - ConfigurationSet { - snapshot: SnapshotConfig::Enabled(SnapshotPref::new(3600, 4, true)), - bgsave: BGSave::default(), - noart: false, - ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT - } + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2003) ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); } - #[test] -fn test_cli_args_conflict() { - let cfg_layout = load_yaml!("../cli.yml"); - let cli_args = ["skyd", "--nosave", "-c config.toml"]; - let matches = App::from_yaml(cfg_layout).get_matches_from(&cli_args); - let err = super::get_config_file_or_return_cfg_from_matches(matches).unwrap_err(); - assert_eq!(err, ConfigError::CfgError(ERR_CONFLICT)); -} - -#[test] -fn test_cli_args_conflict_with_restore_file_okay() { - let cfg_layout = load_yaml!("../cli.yml"); - let cli_args = ["skyd", "--restore", "somedir", "-c", "config.toml"]; - let matches = App::from_yaml(cfg_layout).get_matches_from(&cli_args); - let ret = super::get_config_file_or_return_cfg_from_matches(matches).unwrap_err(); - // this should only compain about the missing dir but not about conflict +fn server_tcp_fail_both() { + let mut cfgset = Configset::new_env(); + cfgset.server_tcp( + Some("?127.0.0.1"), + "SKY_SERVER_HOST", + Some("65537"), + "SKY_SERVER_PORT", + ); assert_eq!( - ret, - ConfigError::OSError(std::io::Error::from(std::io::ErrorKind::NotFound)) + cfgset.cfg.ports, + PortConfig::new_insecure_only(DEFAULT_IPV4, 2003) ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); } -#[test] -fn test_cli_args_conflict_with_restore_file_fail() { - let cfg_layout = load_yaml!("../cli.yml"); - let cli_args = [ - "skyd", - "--restore", - "somedir", - "-c", - "config.toml", - "--nosave", - ]; - let matches = App::from_yaml(cfg_layout).get_matches_from(&cli_args); - let ret = super::get_config_file_or_return_cfg_from_matches(matches).unwrap_err(); - // this should only compain about the missing dir but not about conflict - assert_eq!(ret, ConfigError::CfgError(ERR_CONFLICT)); +/// Gets a `toml` file from `WORKSPACEROOT/examples/config-files` +fn get_toml_from_examples_dir(filename: String) -> TResult { + use std::path; + let curdir = std::env::current_dir().unwrap(); + let workspaceroot = curdir.ancestors().nth(1).unwrap(); + let mut fileloc = path::PathBuf::from(workspaceroot); + fileloc.push("examples"); + fileloc.push("config-files"); + fileloc.push(filename); + Ok(fs::read_to_string(fileloc)?) } diff --git a/server/src/main.rs b/server/src/main.rs index c36d4e44..2867074a 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -46,8 +46,6 @@ use std::thread; use std::time; #[macro_use] mod util; -#[macro_use] -extern crate libsky; mod actions; mod admin; mod arbiter; @@ -169,28 +167,28 @@ use self::config::{BGSave, PortConfig, SnapshotConfig}; /// This function checks the command line arguments and either returns a config object /// or prints an error to `stderr` and terminates the server fn check_args_and_get_cfg() -> (PortConfig, BGSave, SnapshotConfig, Option, usize) { - let cfg = config::get_config_file_or_return_cfg(); - let binding_and_cfg = match cfg { - Ok(config::ConfigType::Custom(cfg, file)) => { + match config::get_config() { + Ok(cfg) => { if cfg.is_artful() { println!("Skytable v{} | {}\n{}", VERSION, URL, TEXT); } else { println!("Skytable v{} | {}", VERSION, URL); } - log::info!("Using settings from supplied configuration"); - (cfg.ports, cfg.bgsave, cfg.snapshot, file, cfg.maxcon) - } - Ok(config::ConfigType::Def(cfg, file)) => { - println!("Skytable v{} | {}\n{}", VERSION, URL, TEXT); - log::warn!("No configuration file supplied. Using default settings"); - (cfg.ports, cfg.bgsave, cfg.snapshot, file, cfg.maxcon) + if cfg.is_custom() { + log::info!("Using settings from supplied configuration"); + } else { + log::warn!("No configuration file supplied. Using default settings"); + } + // print warnings if any + cfg.print_warnings(); + let (cfg, restore) = cfg.finish(); + (cfg.ports, cfg.bgsave, cfg.snapshot, restore, cfg.maxcon) } Err(e) => { log::error!("{}", e); std::process::exit(0x01); } - }; - binding_and_cfg + } } /// On startup, we attempt to check if a `.sky_pid` file exists. If it does, then From ce7d7bef257517575bf006f664d621a719096e8a Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 17:06:06 +0530 Subject: [PATCH 12/24] Fix config change checks and error messages --- server/src/config/feedback.rs | 8 +++--- server/src/config/mod.rs | 48 ++++++++++++++++++++--------------- server/src/config/tests.rs | 2 +- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index 73ae4dcb..fa2c6c2b 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -108,7 +108,7 @@ impl ops::DerefMut for ErrorStack { fn errorstack_fmt() { const EXPECTED: &str = "\ Environment errors: - - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer + - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer\ "; let mut estk = ErrorStack::new(EMSG_ENV); estk.push("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); @@ -128,7 +128,9 @@ impl WarningStack { } } pub fn print_warnings(&self) { - log::warn!("{}", self); + if !self.feedback.is_empty() { + log::warn!("{}", self); + } } } @@ -156,7 +158,7 @@ fn warningstack_fmt() { const EXPECTED: &str = "\ Environment warnings: - BGSAVE is disabled. You may lose data if the host crashes - - The setting for `maxcon` is too high + - The setting for `maxcon` is too high\ "; let mut wstk = WarningStack::new(EMSG_ENV); wstk.push("BGSAVE is disabled. You may lose data if the host crashes"); diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index bbb219b0..36a4a5fd 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -115,14 +115,15 @@ impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { } fn mutate_failed(self, target_value: &mut T, trip: &mut bool) -> bool { match self { - Ok(s) => s - .parse() - .map(|v| { - *trip = true; - *target_value = v; - false - }) - .unwrap_or(true), + Ok(s) => { + *trip = true; + s.parse() + .map(|v| { + *target_value = v; + false + }) + .unwrap_or(true) + } Err(e) => { if matches!(e, VarError::NotPresent) { false @@ -241,9 +242,8 @@ impl Configset { } /// Push an error onto the error stack fn epush(&mut self, field_key: StaticStr, expected: StaticStr) { - self.estack.push(format!( - "Bad value for `${field_key}`. Expected ${expected}", - )) + self.estack + .push(format!("Bad value for `{field_key}`. Expected ${expected}",)) } /// Check if no errors have occurred pub fn is_okay(&self) -> bool { @@ -378,7 +378,7 @@ impl Configset { } else { if has_custom_duration { self.wstack.push(format!( - "Specifying `${nduration_key}` is useless when BGSAVE is disabled" + "Specifying `{nduration_key}` is useless when BGSAVE is disabled" )); } self.wstack @@ -407,7 +407,7 @@ impl Configset { let mut _failsafe = DEFAULT_SNAPSHOT_FAILSAFE; self.try_mutate(nfailsafe, &mut _failsafe, nfailsafe_key, "true/false"); self.wstack.push(format!( - "Specifying `${nfailsafe_key}` is usless when snapshots are disabled" + "Specifying `{nfailsafe_key}` is usless when snapshots are disabled" )); } } @@ -432,9 +432,13 @@ impl Configset { self.cfg.snapshot = SnapshotConfig::Enabled(SnapshotPref::new(every, atmost, failsafe)); } - (false, true) | (true, false) => self.estack.push(format!( - "To use snapshots, pass values for both `${nevery_key}` and `${natmost_key}`" - )), + (false, true) | (true, false) => { + // no changes, but still attempted to change + self.mutated(); + self.estack.push(format!( + "To use snapshots, pass values for both `{nevery_key}` and `{natmost_key}`" + )) + } } } } @@ -490,22 +494,26 @@ impl Configset { } } (true, false) | (false, true) => { + self.mutated(); self.estack.push(format!( - "To use TLS, pass values for both `${nkey_key}` and `${ncert_key}`" + "To use TLS, pass values for both `{nkey_key}` and `{ncert_key}`" )); } (false, false) => { if nport.is_present() { + self.mutated(); self.wstack - .push("Specifying `${nport_key}` is pointless when TLS is disabled"); + .push("Specifying `{nport_key}` is pointless when TLS is disabled"); } if nonly.is_present() { + self.mutated(); self.wstack - .push("Specifying `${nonly_key}` is pointless when TLS is disabled"); + .push("Specifying `{nonly_key}` is pointless when TLS is disabled"); } if npass.is_present() { + self.mutated(); self.wstack - .push("Specifying `${npass_key}` is pointless when TLS is disabled"); + .push("Specifying `{npass_key}` is pointless when TLS is disabled"); } } } diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 4dc68e49..2e45e8c4 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -96,7 +96,7 @@ fn server_tcp_fail_both() { /// Gets a `toml` file from `WORKSPACEROOT/examples/config-files` fn get_toml_from_examples_dir(filename: String) -> TResult { use std::path; - let curdir = std::env::current_dir().unwrap(); + let curdir = path::Path::new(env!("CARGO_MANIFEST_DIR")); let workspaceroot = curdir.ancestors().nth(1).unwrap(); let mut fileloc = path::PathBuf::from(workspaceroot); fileloc.push("examples"); From 647d6ce05c54ecac05724d112c0c60863234ec10 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 04:05:34 -0800 Subject: [PATCH 13/24] Add tests for config impl --- server/src/config/mod.rs | 5 +- server/src/config/tests.rs | 195 ++++++++++++++++++++++++++++++++++++- 2 files changed, 198 insertions(+), 2 deletions(-) diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 36a4a5fd..35881e65 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -286,7 +286,10 @@ impl Configset { needs_error = !validation_fn(&ok); *target = ok; } - ConfigSourceParseResult::ParseFailure => needs_error = true, + ConfigSourceParseResult::ParseFailure => { + self.mutated(); + needs_error = true + } ConfigSourceParseResult::Absent => {} } if needs_error { diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 2e45e8c4..08fa3300 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -24,10 +24,12 @@ * */ -use super::{Configset, PortConfig, DEFAULT_IPV4}; +use super::{BGSave, Configset, PortConfig, SnapshotConfig, SnapshotPref, SslOpts, DEFAULT_IPV4}; pub(super) use libsky::TResult; use std::fs; +// server tests +// TCP #[test] fn server_tcp() { let mut cfgset = Configset::new_env(); @@ -92,6 +94,197 @@ fn server_tcp_fail_both() { assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); } +// noart +#[test] +fn server_noart_okay() { + let mut cfgset = Configset::new_env(); + cfgset.server_noart(Some("true"), "SKY_SYSTEM_NOART"); + assert!(!cfgset.cfg.is_artful()); + assert!(cfgset.is_okay()); + assert!(cfgset.is_mutated()); +} +#[test] +fn server_noart_fail() { + let mut cfgset = Configset::new_env(); + cfgset.server_noart(Some("truee"), "SKY_SYSTEM_NOART"); + assert!(cfgset.cfg.is_artful()); + assert!(!cfgset.is_okay()); + assert!(cfgset.is_mutated()); +} +#[test] +fn server_maxcon_okay() { + let mut cfgset = Configset::new_env(); + cfgset.server_maxcon(Some("12345"), "SKY_SYSTEM_MAXCON"); + assert!(cfgset.is_mutated()); + assert!(cfgset.is_okay()); + assert_eq!(cfgset.cfg.maxcon, 12345); +} +#[test] +fn server_maxcon_fail() { + let mut cfgset = Configset::new_env(); + cfgset.server_maxcon(Some("12345A"), "SKY_SYSTEM_MAXCON"); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + assert_eq!(cfgset.cfg.maxcon, 50000); +} + +// bgsave settings +#[test] +fn bgsave_okay() { + let mut cfgset = Configset::new_env(); + cfgset.bgsave_settings( + Some("true"), + "SKY_BGSAVE_ENABLED", + Some("128"), + "SKY_BGSAVE_DURATION", + ); + assert!(cfgset.is_mutated()); + assert!(cfgset.is_okay()); + assert_eq!(cfgset.cfg.bgsave, BGSave::Enabled(128)); +} +#[test] +fn bgsave_fail() { + let mut cfgset = Configset::new_env(); + cfgset.bgsave_settings( + Some("truee"), + "SKY_BGSAVE_ENABLED", + Some("128"), + "SKY_BGSAVE_DURATION", + ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + assert_eq!(cfgset.cfg.bgsave, BGSave::Enabled(128)); +} + +// snapshot settings +#[test] +fn snapshot_okay() { + let mut cfgset = Configset::new_env(); + cfgset.snapshot_settings( + Some("3600"), + "SKY_SNAPSHOT_EVERY", + Some("0"), + "SKY_SNAPSHOT_ATMOST", + Some("false"), + "SKY_SNAPSHOT_FAILSAFE", + ); + assert!(cfgset.is_mutated()); + assert!(cfgset.is_okay()); + assert_eq!( + cfgset.cfg.snapshot, + SnapshotConfig::Enabled(SnapshotPref::new(3600, 0, false)) + ); +} +#[test] +fn snapshot_fail() { + let mut cfgset = Configset::new_env(); + cfgset.snapshot_settings( + Some("3600"), + "SKY_SNAPSHOT_EVERY", + Some("0"), + "SKY_SNAPSHOT_ATMOST", + Some("falsee"), + "SKY_SNAPSHOT_FAILSAFE", + ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.cfg.snapshot, + SnapshotConfig::Enabled(SnapshotPref::new(3600, 0, true)) + ); +} +#[test] +fn snapshot_fail_with_missing_required_values() { + let mut cfgset = Configset::new_env(); + cfgset.snapshot_settings( + Some("3600"), + "SKY_SNAPSHOT_EVERY", + None, + "SKY_SNAPSHOT_ATMOST", + None, + "SKY_SNAPSHOT_FAILSAFE", + ); + assert!(cfgset.is_mutated()); + assert!(!cfgset.is_okay()); + assert_eq!(cfgset.cfg.snapshot, SnapshotConfig::Disabled); +} + +// TLS settings +#[test] +fn tls_settings_okay() { + let mut cfg = Configset::new_env(); + cfg.tls_settings( + Some("key.pem"), + "SKY_TLS_KEY", + Some("cert.pem"), + "SKY_TLS_CERT", + Some("2005"), + "SKY_TLS_PORT", + Some("false"), + "SKY_TLS_ONLY", + None, + "SKY_TLS_PASSIN", + ); + assert!(cfg.is_mutated()); + assert!(cfg.is_okay()); + assert_eq!(cfg.cfg.ports, { + let mut pf = PortConfig::default(); + pf.upgrade_to_tls(SslOpts::new( + "key.pem".to_owned(), + "cert.pem".to_owned(), + 2005, + None, + )); + pf + }); +} +#[test] +fn tls_settings_fail() { + let mut cfg = Configset::new_env(); + cfg.tls_settings( + Some("key.pem"), + "SKY_TLS_KEY", + Some("cert.pem"), + "SKY_TLS_CERT", + Some("A2005"), + "SKY_TLS_PORT", + Some("false"), + "SKY_TLS_ONLY", + None, + "SKY_TLS_PASSIN", + ); + assert!(cfg.is_mutated()); + assert!(!cfg.is_okay()); + assert_eq!(cfg.cfg.ports, { + let mut pf = PortConfig::default(); + pf.upgrade_to_tls(SslOpts::new( + "key.pem".to_owned(), + "cert.pem".to_owned(), + 2004, + None, + )); + pf + }); +} +#[test] +fn tls_settings_fail_with_missing_required_values() { + let mut cfg = Configset::new_env(); + cfg.tls_settings( + Some("key.pem"), + "SKY_TLS_KEY", + None, + "SKY_TLS_CERT", + Some("2005"), + "SKY_TLS_PORT", + Some("false"), + "SKY_TLS_ONLY", + None, + "SKY_TLS_PASSIN", + ); + assert!(cfg.is_mutated()); + assert!(!cfg.is_okay()); + assert_eq!(cfg.cfg.ports, PortConfig::default()); +} /// Gets a `toml` file from `WORKSPACEROOT/examples/config-files` fn get_toml_from_examples_dir(filename: String) -> TResult { From 53218ee98a8ea857a9ef83aeaaf78c2d251e3178 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 06:44:26 -0800 Subject: [PATCH 14/24] Add tests for types that implement `TryFromConfigSource` --- server/src/config/cfgcli.rs | 5 +- server/src/config/mod.rs | 10 +- server/src/config/tests.rs | 253 ++++++++++++++++++++++++++++++++- server/src/protocol/element.rs | 2 +- 4 files changed, 265 insertions(+), 5 deletions(-) diff --git a/server/src/config/cfgcli.rs b/server/src/config/cfgcli.rs index e41323e6..3bfa64e9 100644 --- a/server/src/config/cfgcli.rs +++ b/server/src/config/cfgcli.rs @@ -29,12 +29,13 @@ use clap::ArgMatches; /// A flag. The flag is said to be set if `self.set` is true and unset if `self.set` is false. However, /// if the flag is set, the value of SWITCH determines what value it is set to -struct Flag { +#[derive(Copy, Clone)] +pub(super) struct Flag { set: bool, } impl Flag { - fn new(set: bool) -> Self { + pub(super) fn new(set: bool) -> Self { Self { set } } } diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 35881e65..305e3d6a 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -31,6 +31,7 @@ use core::str::FromStr; use std::env::VarError; use std::fs; use std::net::{IpAddr, Ipv4Addr}; + // internal modules mod cfgcli; mod cfgenv; @@ -39,6 +40,7 @@ mod definitions; mod feedback; #[cfg(test)] mod tests; + // internal imports use self::cfgfile::Config as ConfigFile; pub use self::definitions::*; @@ -149,7 +151,7 @@ impl<'a, T: FromStr + 'a> TryFromConfigSource for Result { } } -#[derive(Debug)] +#[derive(Debug, PartialEq)] /// Since we have conflicting trait implementations, we define a custom `Option` type pub struct OptString { base: Option, @@ -161,6 +163,12 @@ impl OptString { } } +impl Default for OptString { + fn default() -> Self { + Self { base: None } + } +} + impl From> for OptString { fn from(base: Option) -> Self { Self { base } diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 08fa3300..76ffc8cb 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -46,6 +46,7 @@ fn server_tcp() { assert!(cfgset.is_mutated()); assert!(cfgset.is_okay()); } + #[test] fn server_tcp_fail_host() { let mut cfgset = Configset::new_env(); @@ -62,6 +63,7 @@ fn server_tcp_fail_host() { assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); } + #[test] fn server_tcp_fail_port() { let mut cfgset = Configset::new_env(); @@ -78,6 +80,7 @@ fn server_tcp_fail_port() { assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); } + #[test] fn server_tcp_fail_both() { let mut cfgset = Configset::new_env(); @@ -94,6 +97,7 @@ fn server_tcp_fail_both() { assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); } + // noart #[test] fn server_noart_okay() { @@ -103,6 +107,7 @@ fn server_noart_okay() { assert!(cfgset.is_okay()); assert!(cfgset.is_mutated()); } + #[test] fn server_noart_fail() { let mut cfgset = Configset::new_env(); @@ -111,6 +116,7 @@ fn server_noart_fail() { assert!(!cfgset.is_okay()); assert!(cfgset.is_mutated()); } + #[test] fn server_maxcon_okay() { let mut cfgset = Configset::new_env(); @@ -119,6 +125,7 @@ fn server_maxcon_okay() { assert!(cfgset.is_okay()); assert_eq!(cfgset.cfg.maxcon, 12345); } + #[test] fn server_maxcon_fail() { let mut cfgset = Configset::new_env(); @@ -142,6 +149,7 @@ fn bgsave_okay() { assert!(cfgset.is_okay()); assert_eq!(cfgset.cfg.bgsave, BGSave::Enabled(128)); } + #[test] fn bgsave_fail() { let mut cfgset = Configset::new_env(); @@ -175,6 +183,7 @@ fn snapshot_okay() { SnapshotConfig::Enabled(SnapshotPref::new(3600, 0, false)) ); } + #[test] fn snapshot_fail() { let mut cfgset = Configset::new_env(); @@ -193,6 +202,7 @@ fn snapshot_fail() { SnapshotConfig::Enabled(SnapshotPref::new(3600, 0, true)) ); } + #[test] fn snapshot_fail_with_missing_required_values() { let mut cfgset = Configset::new_env(); @@ -238,6 +248,7 @@ fn tls_settings_okay() { pf }); } + #[test] fn tls_settings_fail() { let mut cfg = Configset::new_env(); @@ -266,6 +277,7 @@ fn tls_settings_fail() { pf }); } + #[test] fn tls_settings_fail_with_missing_required_values() { let mut cfg = Configset::new_env(); @@ -287,7 +299,7 @@ fn tls_settings_fail_with_missing_required_values() { } /// Gets a `toml` file from `WORKSPACEROOT/examples/config-files` -fn get_toml_from_examples_dir(filename: String) -> TResult { +fn get_toml_from_examples_dir(filename: &str) -> TResult { use std::path; let curdir = path::Path::new(env!("CARGO_MANIFEST_DIR")); let workspaceroot = curdir.ancestors().nth(1).unwrap(); @@ -297,3 +309,242 @@ fn get_toml_from_examples_dir(filename: String) -> TResult { fileloc.push(filename); Ok(fs::read_to_string(fileloc)?) } + +mod cfg_file_tests { + use super::get_toml_from_examples_dir; + use crate::config::{ + cfgfile, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, SslOpts, + }; + + #[test] + fn config_file_okay() { + let file = get_toml_from_examples_dir("template.toml").unwrap(); + let toml = toml::from_str(&file).unwrap(); + let cfg_from_file = cfgfile::from_file(toml); + assert!(cfg_from_file.is_mutated()); + assert!(cfg_from_file.is_okay()); + // expected + let mut expected = ConfigurationSet::default(); + expected.snapshot = SnapshotConfig::Enabled(SnapshotPref::new(3600, 4, true)); + expected.ports = PortConfig::new_secure_only( + crate::config::DEFAULT_IPV4, + SslOpts::new( + "/path/to/keyfile.pem".to_owned(), + "/path/to/chain.pem".to_owned(), + 2004, + Some("/path/to/cert/passphrase.txt".to_owned()), + ), + ); + // check + assert_eq!(cfg_from_file.cfg, expected); + } +} + +mod try_from_config_source_impls { + use crate::config::{cfgcli::Flag, cfgfile::Optional, TryFromConfigSource, DEFAULT_IPV4}; + use std::env::{set_var, var}; + use std::fmt::Debug; + + const EXPECT_TRUE: bool = true; + const EXPECT_FALSE: bool = false; + const MUTATED: bool = true; + const NOT_MUTATED: bool = false; + const IS_PRESENT: bool = true; + const IS_ABSENT: bool = false; + const MUTATION_FAILURE: bool = true; + const NO_MUTATION_FAILURE: bool = false; + + fn _mut_base_test_expected( + new: impl TryFromConfigSource, + expected: T, + is_present: bool, + mutate_failed: bool, + has_mutated: bool, + ) { + let mut default = Default::default(); + let mut mutated = false; + assert_eq!(new.is_present(), is_present); + assert_eq!(new.mutate_failed(&mut default, &mut mutated), mutate_failed); + assert_eq!(mutated, has_mutated); + assert_eq!(default, expected); + } + + fn _mut_base_test( + new: impl TryFromConfigSource, + mut default: T, + is_present: bool, + mutate_failed: bool, + has_mutated: bool, + ) { + let mut mutated = false; + dbg!(new.is_present(), is_present); + assert_eq!(new.is_present(), is_present); + assert_eq!(new.mutate_failed(&mut default, &mut mutated), mutate_failed); + assert_eq!(mutated, has_mutated); + } + + fn mut_test_pass(new: impl TryFromConfigSource, default: T) { + _mut_base_test(new, default, IS_PRESENT, NO_MUTATION_FAILURE, MUTATED) + } + + fn mut_test_fail(new: impl TryFromConfigSource, default: T) { + _mut_base_test(new, default, IS_PRESENT, MUTATION_FAILURE, MUTATED) + } + + mod env_var { + use super::*; + + // test for Result + #[test] + fn env_okay_ipv4() { + set_var("TEST_SKY_SYSTEM_HOST", "127.0.0.1"); + mut_test_pass(var("TEST_SKY_SYSTEM_HOST"), DEFAULT_IPV4); + } + + #[test] + fn env_fail_ipv4() { + set_var("TEST_SKY_SYSTEM_HOST2", "127.0.0.1A"); + mut_test_fail(var("TEST_SKY_SYSTEM_HOST2"), DEFAULT_IPV4); + } + } + + mod option_str { + use super::*; + + // test for Option<&str> (as in CLI) + #[test] + fn option_str_okay_ipv4() { + let ip = Some("127.0.0.1"); + mut_test_pass(ip, DEFAULT_IPV4); + } + + #[test] + fn option_str_fail_ipv4() { + let ip = Some("127.0.0.1A"); + mut_test_fail(ip, DEFAULT_IPV4); + } + + #[test] + fn option_str_nomut() { + let ip = None; + _mut_base_test( + ip, + DEFAULT_IPV4, + IS_ABSENT, + NO_MUTATION_FAILURE, + NOT_MUTATED, + ); + } + } + + mod cfgcli_flag { + use super::*; + + #[test] + fn flag_true_if_set_okay_set() { + // this is true if flag is present + let flag = Flag::::new(true); + // we expect true + _mut_base_test_expected(flag, EXPECT_TRUE, IS_PRESENT, NO_MUTATION_FAILURE, MUTATED); + } + + #[test] + fn flag_true_if_set_okay_unset() { + // this is true if flag is present, but the flag here is not present + let flag = Flag::::new(false); + // we expect no mutation because the flag was not set + _mut_base_test( + flag, + EXPECT_FALSE, + IS_ABSENT, + NO_MUTATION_FAILURE, + NOT_MUTATED, + ); + } + + #[test] + fn flag_false_if_set_okay_set() { + // this is false if flag is present + let flag = Flag::::new(true); + // expect mutation to have happened + _mut_base_test_expected(flag, EXPECT_FALSE, IS_PRESENT, NO_MUTATION_FAILURE, MUTATED); + } + + #[test] + fn flag_false_if_set_okay_unset() { + // this is false if flag is present, but the flag is absent + let flag = Flag::::new(false); + // expect no mutation + _mut_base_test( + flag, + EXPECT_FALSE, + IS_ABSENT, + NO_MUTATION_FAILURE, + NOT_MUTATED, + ); + } + } + + mod optional { + use super::*; + + // test for cfg file scenario + #[test] + fn optional_okay_ipv4() { + let ip = Optional::some(DEFAULT_IPV4); + mut_test_pass(ip, DEFAULT_IPV4); + } + + #[test] + fn optional_okay_ipv4_none() { + let ip = Optional::from(None); + _mut_base_test( + ip, + DEFAULT_IPV4, + IS_ABSENT, + NO_MUTATION_FAILURE, + NOT_MUTATED, + ); + } + } + + mod cfgfile_nonull { + use super::*; + use crate::config::cfgfile::NonNull; + + #[test] + fn nonnull_okay() { + let port = NonNull::from(2100); + _mut_base_test_expected(port, 2100, IS_PRESENT, NO_MUTATION_FAILURE, MUTATED); + } + } + + mod optstring { + use super::*; + use crate::config::OptString; + + #[test] + fn optstring_okay() { + let pass = OptString::from(Some("tlspass.txt".to_owned())); + _mut_base_test_expected( + pass, + OptString::from(Some("tlspass.txt".to_owned())), + IS_PRESENT, + NO_MUTATION_FAILURE, + MUTATED, + ); + } + + #[test] + fn optstring_null_okay() { + let pass = OptString::from(None); + _mut_base_test_expected( + pass, + OptString::new_null(), + IS_PRESENT, + NO_MUTATION_FAILURE, + NOT_MUTATED, + ); + } + } +} diff --git a/server/src/protocol/element.rs b/server/src/protocol/element.rs index eb3cae08..102e0bee 100644 --- a/server/src/protocol/element.rs +++ b/server/src/protocol/element.rs @@ -57,7 +57,7 @@ pub enum UnsafeFlatElement { } impl UnsafeElement { - pub const fn is_any_array(&self) -> bool { + pub const fn is_any_array(&self) -> bool { matches!(self, Self::AnyArray(_)) } } From 59a67ba0c53a400db13c2bf17aa80c770f56204f Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 22:34:38 +0530 Subject: [PATCH 15/24] Add cfg file tests --- server/src/config/definitions.rs | 29 ++++-- server/src/config/tests.rs | 163 ++++++++++++++++++++++++++++++- 2 files changed, 184 insertions(+), 8 deletions(-) diff --git a/server/src/config/definitions.rs b/server/src/config/definitions.rs index f60ca9ab..01e293f0 100644 --- a/server/src/config/definitions.rs +++ b/server/src/config/definitions.rs @@ -77,6 +77,21 @@ pub struct ConfigurationSet { } impl ConfigurationSet { + pub const fn new( + noart: bool, + bgsave: BGSave, + snapshot: SnapshotConfig, + ports: PortConfig, + maxcon: usize, + ) -> Self { + Self { + noart, + bgsave, + snapshot, + ports, + maxcon, + } + } /// Create a default `ConfigurationSet` with the following setup defaults: /// - `host`: 127.0.0.1 /// - `port` : 2003 @@ -85,13 +100,13 @@ impl ConfigurationSet { /// - `bgsave_duration` : 120 /// - `ssl` : disabled pub const fn default() -> Self { - ConfigurationSet { - noart: false, - bgsave: BGSave::default(), - snapshot: SnapshotConfig::default(), - ports: PortConfig::new_insecure_only(DEFAULT_IPV4, 2003), - maxcon: MAXIMUM_CONNECTION_LIMIT, - } + Self::new( + false, + BGSave::default(), + SnapshotConfig::default(), + PortConfig::new_insecure_only(DEFAULT_IPV4, 2003), + MAXIMUM_CONNECTION_LIMIT, + ) } /// Returns `false` if `noart` is enabled. Otherwise it returns `true` pub const fn is_artful(&self) -> bool { diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 76ffc8cb..6b4a56d8 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -25,6 +25,7 @@ */ use super::{BGSave, Configset, PortConfig, SnapshotConfig, SnapshotPref, SslOpts, DEFAULT_IPV4}; + pub(super) use libsky::TResult; use std::fs; @@ -313,8 +314,16 @@ fn get_toml_from_examples_dir(filename: &str) -> TResult { mod cfg_file_tests { use super::get_toml_from_examples_dir; use crate::config::{ - cfgfile, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, SslOpts, + cfgfile, BGSave, Configset, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, + SslOpts, DEFAULT_IPV4, DEFAULT_PORT, }; + use crate::dbnet::MAXIMUM_CONNECTION_LIMIT; + use std::net::{IpAddr, Ipv6Addr}; + + fn cfgset_from_toml_str(file: String) -> Result { + let toml = toml::from_str(&file)?; + Ok(cfgfile::from_file(toml)) + } #[test] fn config_file_okay() { @@ -338,6 +347,158 @@ mod cfg_file_tests { // check assert_eq!(cfg_from_file.cfg, expected); } + + #[test] + fn test_config_file_ok() { + let file = get_toml_from_examples_dir("skyd.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!(cfg, ConfigurationSet::default()); + } + + #[test] + fn test_config_file_err() { + let file = get_toml_from_examples_dir("skyd.toml").unwrap(); + let cfg = cfgset_from_toml_str(file); + assert!(cfg.is_err()); + } + + #[test] + fn test_config_file_noart() { + let file = get_toml_from_examples_dir("secure-noart.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet { + noart: true, + bgsave: BGSave::default(), + snapshot: SnapshotConfig::default(), + ports: PortConfig::default(), + maxcon: MAXIMUM_CONNECTION_LIMIT + } + ); + } + + #[test] + fn test_config_file_ipv6() { + let file = get_toml_from_examples_dir("ipv6.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet { + noart: false, + bgsave: BGSave::default(), + snapshot: SnapshotConfig::default(), + ports: PortConfig::new_insecure_only( + IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0x1)), + DEFAULT_PORT + ), + maxcon: MAXIMUM_CONNECTION_LIMIT + } + ); + } + + #[test] + fn test_config_file_template() { + let file = get_toml_from_examples_dir("template.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet::new( + false, + BGSave::default(), + SnapshotConfig::Enabled(SnapshotPref::new(3600, 4, true)), + PortConfig::new_secure_only( + DEFAULT_IPV4, + SslOpts::new( + "/path/to/keyfile.pem".into(), + "/path/to/chain.pem".into(), + 2004, + Some("/path/to/cert/passphrase.txt".to_owned()) + ) + ), + MAXIMUM_CONNECTION_LIMIT + ) + ); + } + + #[test] + fn test_config_file_bad_bgsave_section() { + let file = get_toml_from_examples_dir("badcfg2.toml").unwrap(); + let cfg = cfgset_from_toml_str(file); + assert!(cfg.is_err()); + } + + #[test] + fn test_config_file_custom_bgsave() { + let file = get_toml_from_examples_dir("withcustombgsave.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet { + noart: false, + bgsave: BGSave::new(true, 600), + snapshot: SnapshotConfig::default(), + ports: PortConfig::default(), + maxcon: MAXIMUM_CONNECTION_LIMIT + } + ); + } + + #[test] + fn test_config_file_bgsave_enabled_only() { + /* + * This test demonstrates a case where the user just said that BGSAVE is enabled. + * In that case, we will default to the 120 second duration + */ + let file = get_toml_from_examples_dir("bgsave-justenabled.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet { + noart: false, + bgsave: BGSave::default(), + snapshot: SnapshotConfig::default(), + ports: PortConfig::default(), + maxcon: MAXIMUM_CONNECTION_LIMIT + } + ) + } + + #[test] + fn test_config_file_bgsave_every_only() { + /* + * This test demonstrates a case where the user just gave the value for every + * In that case, it means BGSAVE is enabled and set to `every` seconds + */ + let file = get_toml_from_examples_dir("bgsave-justevery.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet { + noart: false, + bgsave: BGSave::new(true, 600), + snapshot: SnapshotConfig::default(), + ports: PortConfig::default(), + maxcon: MAXIMUM_CONNECTION_LIMIT + } + ) + } + + #[test] + fn test_config_file_snapshot() { + let file = get_toml_from_examples_dir("snapshot.toml").unwrap(); + let cfg = cfgset_from_toml_str(file).unwrap(); + assert_eq!( + cfg, + ConfigurationSet { + snapshot: SnapshotConfig::Enabled(SnapshotPref::new(3600, 4, true)), + bgsave: BGSave::default(), + noart: false, + ports: PortConfig::default(), + maxcon: MAXIMUM_CONNECTION_LIMIT + } + ); + } } mod try_from_config_source_impls { From 857e05529ac097ae52964d9f850aaeb661097cb2 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 23:18:50 +0530 Subject: [PATCH 16/24] Add CLI config tests and improve diagnostic tests --- server/src/config/feedback.rs | 30 ++++++++++ server/src/config/mod.rs | 40 +++++++------- server/src/config/tests.rs | 101 ++++++++++++++++++++++++++++------ 3 files changed, 135 insertions(+), 36 deletions(-) diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index fa2c6c2b..cfe1185e 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -72,6 +72,18 @@ impl fmt::Display for FeedbackStack { } } +impl ops::Deref for FeedbackStack { + type Target = Vec; + fn deref(&self) -> &Self::Target { + &self.stack + } +} +impl ops::DerefMut for FeedbackStack { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.stack + } +} + #[derive(Debug, PartialEq)] pub struct ErrorStack { feedback: FeedbackStack, @@ -187,6 +199,24 @@ impl PartialEq for ConfigError { } } +impl From for ConfigError { + fn from(e: std::io::Error) -> Self { + Self::OSError(e) + } +} + +impl From for ConfigError { + fn from(e: toml::de::Error) -> Self { + Self::ConfigFileParseError(e) + } +} + +impl From for ConfigError { + fn from(e: ErrorStack) -> Self { + Self::CfgError(e) + } +} + impl fmt::Display for ConfigError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 305e3d6a..3a58ccb9 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -251,7 +251,7 @@ impl Configset { /// Push an error onto the error stack fn epush(&mut self, field_key: StaticStr, expected: StaticStr) { self.estack - .push(format!("Bad value for `{field_key}`. Expected ${expected}",)) + .push(format!("Bad value for `{field_key}`. Expected {expected}",)) } /// Check if no errors have occurred pub fn is_okay(&self) -> bool { @@ -325,6 +325,20 @@ impl Configset { other } } + /// Turns self into a Result that can be used by config::get_config() + pub fn into_result(self, restore_file: Option) -> Result { + if self.is_okay() { + // no errors, sweet + if self.is_mutated() { + let Self { cfg, wstack, .. } = self; + Ok(ConfigType::new_custom(cfg, restore_file, wstack)) + } else { + Ok(ConfigType::new_default(restore_file)) + } + } else { + Err(ConfigError::from(self.estack)) + } + } } // server settings @@ -539,14 +553,8 @@ pub fn get_config() -> Result { // get config from file let cfg_from_file = if let Some(file) = matches.value_of("config") { - let file = match fs::read(file) { - Ok(f) => f, - Err(e) => return Err(ConfigError::OSError(e)), - }; - let cfg_file: ConfigFile = match toml::from_slice(&file) { - Ok(cfg) => cfg, - Err(e) => return Err(ConfigError::ConfigFileParseError(e)), - }; + let file = fs::read(file)?; + let cfg_file: ConfigFile = toml::from_slice(&file)?; Some(cfgfile::from_file(cfg_file)) } else { None @@ -569,16 +577,8 @@ pub fn get_config() -> Result { // no configuration, use default Ok(ConfigType::new_default(restore_file)) } else { - let final_config = if let Some(cfg) = cfg_from_file { - cfg - } else { - cfg_from_env.and_then(cfg_from_cli) - }; - if final_config.is_okay() { - let Configset { cfg, wstack, .. } = final_config; - return Ok(ConfigType::new_custom(cfg, restore_file, wstack)); - } else { - return Err(ConfigError::CfgError(final_config.estack)); - } + cfg_from_file + .unwrap_or(cfg_from_env.and_then(cfg_from_cli)) + .into_result(restore_file) } } diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 6b4a56d8..71e15e14 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -63,6 +63,10 @@ fn server_tcp_fail_host() { ); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_SERVER_HOST`. Expected an IPv4/IPv6 address" + ); } #[test] @@ -80,6 +84,10 @@ fn server_tcp_fail_port() { ); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_SERVER_PORT`. Expected a 16-bit positive integer" + ); } #[test] @@ -97,6 +105,14 @@ fn server_tcp_fail_both() { ); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_SERVER_HOST`. Expected an IPv4/IPv6 address" + ); + assert_eq!( + cfgset.estack[1], + "Bad value for `SKY_SERVER_PORT`. Expected a 16-bit positive integer" + ); } // noart @@ -115,6 +131,10 @@ fn server_noart_fail() { cfgset.server_noart(Some("truee"), "SKY_SYSTEM_NOART"); assert!(cfgset.cfg.is_artful()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_SYSTEM_NOART`. Expected true/false" + ); assert!(cfgset.is_mutated()); } @@ -133,6 +153,10 @@ fn server_maxcon_fail() { cfgset.server_maxcon(Some("12345A"), "SKY_SYSTEM_MAXCON"); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_SYSTEM_MAXCON`. Expected a positive integer greater than zero" + ); assert_eq!(cfgset.cfg.maxcon, 50000); } @@ -162,6 +186,10 @@ fn bgsave_fail() { ); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_BGSAVE_ENABLED`. Expected true/false" + ); assert_eq!(cfgset.cfg.bgsave, BGSave::Enabled(128)); } @@ -198,6 +226,10 @@ fn snapshot_fail() { ); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "Bad value for `SKY_SNAPSHOT_FAILSAFE`. Expected true/false" + ); assert_eq!( cfgset.cfg.snapshot, SnapshotConfig::Enabled(SnapshotPref::new(3600, 0, true)) @@ -217,6 +249,10 @@ fn snapshot_fail_with_missing_required_values() { ); assert!(cfgset.is_mutated()); assert!(!cfgset.is_okay()); + assert_eq!( + cfgset.estack[0], + "To use snapshots, pass values for both `SKY_SNAPSHOT_EVERY` and `SKY_SNAPSHOT_ATMOST`" + ); assert_eq!(cfgset.cfg.snapshot, SnapshotConfig::Disabled); } @@ -352,14 +388,7 @@ mod cfg_file_tests { fn test_config_file_ok() { let file = get_toml_from_examples_dir("skyd.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); - assert_eq!(cfg, ConfigurationSet::default()); - } - - #[test] - fn test_config_file_err() { - let file = get_toml_from_examples_dir("skyd.toml").unwrap(); - let cfg = cfgset_from_toml_str(file); - assert!(cfg.is_err()); + assert_eq!(cfg.cfg, ConfigurationSet::default()); } #[test] @@ -367,7 +396,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("secure-noart.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet { noart: true, bgsave: BGSave::default(), @@ -383,7 +412,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("ipv6.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet { noart: false, bgsave: BGSave::default(), @@ -402,7 +431,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("template.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet::new( false, BGSave::default(), @@ -433,7 +462,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("withcustombgsave.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet { noart: false, bgsave: BGSave::new(true, 600), @@ -453,7 +482,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("bgsave-justenabled.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet { noart: false, bgsave: BGSave::default(), @@ -473,7 +502,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("bgsave-justevery.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet { noart: false, bgsave: BGSave::new(true, 600), @@ -489,7 +518,7 @@ mod cfg_file_tests { let file = get_toml_from_examples_dir("snapshot.toml").unwrap(); let cfg = cfgset_from_toml_str(file).unwrap(); assert_eq!( - cfg, + cfg.cfg, ConfigurationSet { snapshot: SnapshotConfig::Enabled(SnapshotPref::new(3600, 4, true)), bgsave: BGSave::default(), @@ -501,6 +530,46 @@ mod cfg_file_tests { } } +mod cli_arg_tests { + use crate::config::{cfgcli, PortConfig}; + use clap::{load_yaml, App}; + #[test] + fn cli_args_okay() { + let cfg_layout = load_yaml!("../cli.yml"); + let cli_args = ["skyd", "--host", "127.0.0.2"]; + let matches = App::from_yaml(cfg_layout).get_matches_from(&cli_args); + let ret = cfgcli::parse_cli_args(matches); + assert_eq!( + ret.cfg.ports, + PortConfig::new_insecure_only("127.0.0.2".parse().unwrap(), 2003) + ); + assert!(ret.is_mutated()); + assert!(ret.is_okay()); + } + #[test] + fn cli_args_okay_no_mut() { + let cfg_layout = load_yaml!("../cli.yml"); + let cli_args = ["skyd", "--restore", "/some/restore/path"]; + let matches = App::from_yaml(cfg_layout).get_matches_from(&cli_args); + let ret = cfgcli::parse_cli_args(matches); + assert!(!ret.is_mutated()); + assert!(ret.is_okay()); + } + #[test] + fn cli_args_fail() { + let cfg_layout = load_yaml!("../cli.yml"); + let cli_args = ["skyd", "--port", "port2003"]; + let matches = App::from_yaml(cfg_layout).get_matches_from(&cli_args); + let ret = cfgcli::parse_cli_args(matches); + assert!(ret.is_mutated()); + assert!(!ret.is_okay()); + assert_eq!( + ret.estack[0], + "Bad value for `--port`. Expected a 16-bit positive integer" + ); + } +} + mod try_from_config_source_impls { use crate::config::{cfgcli::Flag, cfgfile::Optional, TryFromConfigSource, DEFAULT_IPV4}; use std::env::{set_var, var}; @@ -702,7 +771,7 @@ mod try_from_config_source_impls { _mut_base_test_expected( pass, OptString::new_null(), - IS_PRESENT, + IS_ABSENT, NO_MUTATION_FAILURE, NOT_MUTATED, ); From 4325cf9065f241543732871460e6d39157952093 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 20:34:17 -0800 Subject: [PATCH 17/24] Add method to check number of open files on unix-based systems --- server/src/config/feedback.rs | 32 +++++++++- server/src/util/compiler.rs | 68 +++++++++++++++++++++ server/src/{util.rs => util/macros.rs} | 83 +------------------------- server/src/util/mod.rs | 63 +++++++++++++++++++ 4 files changed, 162 insertions(+), 84 deletions(-) create mode 100644 server/src/util/compiler.rs rename server/src/{util.rs => util/macros.rs} (78%) create mode 100644 server/src/util/mod.rs diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index cfe1185e..05d6a26f 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -29,6 +29,7 @@ use toml::de::Error as TomlError; // std imports use core::fmt; use core::ops; +use std::io::Error as IoError; #[cfg(test)] const EMSG_ENV: &str = "Environment"; @@ -181,7 +182,7 @@ Environment warnings: #[derive(Debug)] pub enum ConfigError { - OSError(std::io::Error), + OSError(IoError), CfgError(ErrorStack), ConfigFileParseError(TomlError), Conflict, @@ -199,8 +200,8 @@ impl PartialEq for ConfigError { } } -impl From for ConfigError { - fn from(e: std::io::Error) -> Self { +impl From for ConfigError { + fn from(e: IoError) -> Self { Self::OSError(e) } } @@ -230,3 +231,28 @@ impl fmt::Display for ConfigError { } } } + +#[cfg(unix)] +/// Returns the number of open files +fn get_ulimit() -> Result { + use libc::rlimit; + use libc::RLIMIT_NOFILE; + unsafe { + let rlim = rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + let ret = libc::getrlimit(RLIMIT_NOFILE, &rlim as *const _ as *mut _); + if ret != 0 { + Err(IoError::last_os_error()) + } else { + Ok(rlim.rlim_cur as usize) + } + } +} + +#[test] +#[cfg(unix)] +fn test_ulimit() { + get_ulimit().unwrap(); +} diff --git a/server/src/util/compiler.rs b/server/src/util/compiler.rs new file mode 100644 index 00000000..64632c6c --- /dev/null +++ b/server/src/util/compiler.rs @@ -0,0 +1,68 @@ +/* + * Created on Sat Jan 29 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +//! Dark compiler arts and hackery to defy the normal. Use at your own +//! risk + +use core::mem; + +#[cold] +#[inline(never)] +pub const fn cold() {} + +pub const fn likely(b: bool) -> bool { + if !b { + cold() + } + b +} + +pub const fn unlikely(b: bool) -> bool { + if b { + cold() + } + b +} + +#[cold] +#[inline(never)] +pub const fn cold_err(v: T) -> T { + v +} +#[inline(always)] +pub const fn hot(v: T) -> T { + if false { + cold() + } + v +} + +pub const unsafe fn extend_lifetime<'a, 'b, T>(inp: &'a T) -> &'b T { + mem::transmute(inp) +} +pub unsafe fn extend_lifetime_mut<'a, 'b, T>(inp: &'a mut T) -> &'b mut T { + mem::transmute(inp) +} diff --git a/server/src/util.rs b/server/src/util/macros.rs similarity index 78% rename from server/src/util.rs rename to server/src/util/macros.rs index 2811bb57..2b993a37 100644 --- a/server/src/util.rs +++ b/server/src/util/macros.rs @@ -1,5 +1,5 @@ /* - * Created on Fri Jun 25 2021 + * Created on Sat Jan 29 2022 * * This file is a part of Skytable * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source @@ -7,7 +7,7 @@ * vision to provide flexibility in data modelling without compromising * on performance, queryability or scalability. * - * Copyright (c) 2021, Sayan Nandan + * Copyright (c) 2022, Sayan Nandan * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by @@ -24,22 +24,6 @@ * */ -/// # Unsafe unwrapping -/// -/// This trait provides a method `unsafe_unwrap` that is potentially unsafe and has -/// the ability to **violate multiple safety gurantees** that rust provides. So, -/// if you get `SIGILL`s or `SIGSEGV`s, by using this trait, blame yourself. -pub unsafe trait Unwrappable { - /// Unwrap a _nullable_ (almost) type to get its value while asserting that the value - /// cannot ever be null - /// - /// ## Safety - /// The trait is unsafe, and so is this function. You can wreck potential havoc if you - /// use this heedlessly - /// - unsafe fn unsafe_unwrap(self) -> T; -} - #[macro_export] macro_rules! impossible { () => { @@ -47,24 +31,6 @@ macro_rules! impossible { }; } -unsafe impl Unwrappable for Result { - unsafe fn unsafe_unwrap(self) -> T { - match self { - Ok(t) => t, - Err(_) => impossible!(), - } - } -} - -unsafe impl Unwrappable for Option { - unsafe fn unsafe_unwrap(self) -> T { - match self { - Some(t) => t, - None => impossible!(), - } - } -} - #[macro_export] macro_rules! consts { ($($(#[$attr:meta])* $ident:ident : $ty:ty = $expr:expr;)*) => { @@ -195,51 +161,6 @@ macro_rules! afn_action { }; } -pub mod compiler { - //! Dark compiler arts and hackery to defy the normal. Use at your own - //! risk - - use core::mem; - - #[cold] - #[inline(never)] - pub const fn cold() {} - - pub const fn likely(b: bool) -> bool { - if !b { - cold() - } - b - } - - pub const fn unlikely(b: bool) -> bool { - if b { - cold() - } - b - } - - #[cold] - #[inline(never)] - pub const fn cold_err(v: T) -> T { - v - } - #[inline(always)] - pub const fn hot(v: T) -> T { - if false { - cold() - } - v - } - - pub unsafe fn extend_lifetime<'a, 'b, T>(inp: &'a T) -> &'b T { - mem::transmute(inp) - } - pub unsafe fn extend_lifetime_mut<'a, 'b, T>(inp: &'a mut T) -> &'b mut T { - mem::transmute(inp) - } -} - #[macro_export] macro_rules! byt { ($f:expr) => { diff --git a/server/src/util/mod.rs b/server/src/util/mod.rs new file mode 100644 index 00000000..4cb00ce7 --- /dev/null +++ b/server/src/util/mod.rs @@ -0,0 +1,63 @@ +/* + * Created on Fri Jun 25 2021 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2021, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +#[macro_use] +mod macros; +pub mod compiler; + +/// # Unsafe unwrapping +/// +/// This trait provides a method `unsafe_unwrap` that is potentially unsafe and has +/// the ability to **violate multiple safety gurantees** that rust provides. So, +/// if you get `SIGILL`s or `SIGSEGV`s, by using this trait, blame yourself. +pub unsafe trait Unwrappable { + /// Unwrap a _nullable_ (almost) type to get its value while asserting that the value + /// cannot ever be null + /// + /// ## Safety + /// The trait is unsafe, and so is this function. You can wreck potential havoc if you + /// use this heedlessly + /// + unsafe fn unsafe_unwrap(self) -> T; +} + +unsafe impl Unwrappable for Result { + unsafe fn unsafe_unwrap(self) -> T { + match self { + Ok(t) => t, + Err(_) => impossible!(), + } + } +} + +unsafe impl Unwrappable for Option { + unsafe fn unsafe_unwrap(self) -> T { + match self { + Some(t) => t, + None => impossible!(), + } + } +} From 6b54217fb2a0a6d00e7f92953751486641d4adaf Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Fri, 28 Jan 2022 23:45:04 -0800 Subject: [PATCH 18/24] Add production-mode setting evaluation --- server/src/config/definitions.rs | 7 +++ server/src/config/feedback.rs | 79 +++++++++++++++++++++----------- server/src/config/mod.rs | 2 +- server/src/util/mod.rs | 1 + server/src/util/os.rs | 69 ++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 28 deletions(-) create mode 100644 server/src/util/os.rs diff --git a/server/src/config/definitions.rs b/server/src/config/definitions.rs index 01e293f0..0f5b7093 100644 --- a/server/src/config/definitions.rs +++ b/server/src/config/definitions.rs @@ -57,6 +57,10 @@ impl BGSave { pub const fn default() -> Self { BGSave::new(true, 120) } + /// Check if BGSAVE is disabled + pub const fn is_disabled(&self) -> bool { + matches!(self, Self::Disabled) + } } /// A `ConfigurationSet` which can be used by main::check_args_or_connect() to bind @@ -177,6 +181,9 @@ impl PortConfig { } } } + pub const fn insecure_only(&self) -> bool { + matches!(self, Self::InsecureOnly { .. }) + } } #[derive(Deserialize, Debug, PartialEq)] diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index 05d6a26f..aa7bf904 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -30,9 +30,14 @@ use toml::de::Error as TomlError; use core::fmt; use core::ops; use std::io::Error as IoError; +// internal imports +use super::{Configset, SnapshotConfig, SnapshotPref}; +#[cfg(unix)] +use crate::util::os::ResourceLimit; #[cfg(test)] const EMSG_ENV: &str = "Environment"; +const EMSG_PROD: &str = "Production mode"; const TAB: &str = " "; #[derive(Debug, PartialEq)] @@ -56,9 +61,6 @@ impl FeedbackStack { pub fn push(&mut self, f: impl ToString) { self.stack.push(f.to_string()) } - pub fn is_empty(&self) -> bool { - self.stack.is_empty() - } } impl fmt::Display for FeedbackStack { @@ -186,6 +188,7 @@ pub enum ConfigError { CfgError(ErrorStack), ConfigFileParseError(TomlError), Conflict, + ProdError(ErrorStack), } impl PartialEq for ConfigError { @@ -195,6 +198,7 @@ impl PartialEq for ConfigError { (Self::CfgError(lhs), Self::CfgError(rhs)) => lhs == rhs, (Self::ConfigFileParseError(lhs), Self::ConfigFileParseError(rhs)) => lhs == rhs, (Self::Conflict, Self::Conflict) => true, + (Self::ProdError(lhs), Self::ProdError(rhs)) => lhs == rhs, _ => false, } } @@ -212,12 +216,6 @@ impl From for ConfigError { } } -impl From for ConfigError { - fn from(e: ErrorStack) -> Self { - Self::CfgError(e) - } -} - impl fmt::Display for ConfigError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -228,31 +226,58 @@ impl fmt::Display for ConfigError { f, "Conflict error: Either provide CLI args, environment variables or a config file for configuration" ), + Self::ProdError(e) => write!(f, "You have invalid configuration for production mode. {}", e) } } } -#[cfg(unix)] -/// Returns the number of open files -fn get_ulimit() -> Result { - use libc::rlimit; - use libc::RLIMIT_NOFILE; - unsafe { - let rlim = rlimit { - rlim_cur: 0, - rlim_max: 0, - }; - let ret = libc::getrlimit(RLIMIT_NOFILE, &rlim as *const _ as *mut _); - if ret != 0 { - Err(IoError::last_os_error()) - } else { - Ok(rlim.rlim_cur as usize) +/// Check if the settings are suitable for use in production mode +fn evaluate_prod_settings(cfg: Configset) -> Result { + let mut estack = ErrorStack::new(EMSG_PROD); + // first check BGSAVE + if cfg.cfg.bgsave.is_disabled() { + estack.push("BGSAVE must be enabled"); + } + // now check snapshot settings (failsafe) + if let SnapshotConfig::Enabled(SnapshotPref { poison, .. }) = cfg.cfg.snapshot { + if !poison { + estack.push("Snapshots must be failsafe"); } } + // now check TLS settings + if cfg.cfg.ports.insecure_only() { + estack.push("Either multi-socket (TCP and TLS) or TLS only must be enabled"); + } + // now check maxcon + #[cfg(unix)] + if ResourceLimit::get()?.is_over_limit(cfg.cfg.maxcon) { + estack.push( + "The value for maximum connections exceeds available resources to the server process", + ); + } + if estack.is_empty() { + Ok(cfg) + } else { + Err(ConfigError::ProdError(estack)) + } } #[test] -#[cfg(unix)] -fn test_ulimit() { - get_ulimit().unwrap(); +fn prod_mode_error_fmt() { + let mut estack = ErrorStack::new(EMSG_PROD); + estack.push("BGSAVE must be enabled"); + estack.push("Snapshots must be failsafe"); + estack.push("Either multi-socket (TCP and TLS) or TLS-only mode must be enabled"); + estack.push( + "The value for maximum connections exceeds available resources to the server process", + ); + let e = ConfigError::ProdError(estack); + const EXPECTED: &str = "\ +You have invalid configuration for production mode. Production mode errors: + - BGSAVE must be enabled + - Snapshots must be failsafe + - Either multi-socket (TCP and TLS) or TLS-only mode must be enabled + - The value for maximum connections exceeds available resources to the server process\ +"; + assert_eq!(format!("{}", e), EXPECTED); } diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 3a58ccb9..e66abdae 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -336,7 +336,7 @@ impl Configset { Ok(ConfigType::new_default(restore_file)) } } else { - Err(ConfigError::from(self.estack)) + Err(ConfigError::ProdError(self.estack)) } } } diff --git a/server/src/util/mod.rs b/server/src/util/mod.rs index 4cb00ce7..e251baf4 100644 --- a/server/src/util/mod.rs +++ b/server/src/util/mod.rs @@ -27,6 +27,7 @@ #[macro_use] mod macros; pub mod compiler; +pub mod os; /// # Unsafe unwrapping /// diff --git a/server/src/util/os.rs b/server/src/util/os.rs new file mode 100644 index 00000000..608131ec --- /dev/null +++ b/server/src/util/os.rs @@ -0,0 +1,69 @@ +/* + * Created on Sat Jan 29 2022 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2022, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +#[cfg(unix)] +pub use unix::*; + +#[cfg(unix)] +mod unix { + use libc::{rlimit, RLIMIT_NOFILE}; + use std::io::Error as IoError; + + #[derive(Debug)] + pub struct ResourceLimit { + cur: u64, + max: u64, + } + + impl ResourceLimit { + const fn new(cur: u64, max: u64) -> Self { + Self { cur, max } + } + pub const fn is_over_limit(&self, expected: usize) -> bool { + expected as u64 > self.cur + } + /// Returns the maximum number of open files + pub fn get() -> Result { + unsafe { + let rlim = rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + let ret = libc::getrlimit(RLIMIT_NOFILE, &rlim as *const _ as *mut _); + if ret != 0 { + Err(IoError::last_os_error()) + } else { + Ok(ResourceLimit::new(rlim.rlim_cur, rlim.rlim_max)) + } + } + } + } + + #[test] + fn test_ulimit() { + let _ = ResourceLimit::get().unwrap(); + } +} From 481509d92766f5390e71aa520c800fa3eb42f263 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 29 Jan 2022 00:00:59 -0800 Subject: [PATCH 19/24] Fix rlimit impl for 32-bit --- server/src/util/os.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/util/os.rs b/server/src/util/os.rs index 608131ec..84223771 100644 --- a/server/src/util/os.rs +++ b/server/src/util/os.rs @@ -56,7 +56,10 @@ mod unix { if ret != 0 { Err(IoError::last_os_error()) } else { - Ok(ResourceLimit::new(rlim.rlim_cur, rlim.rlim_max)) + Ok(ResourceLimit::new( + rlim.rlim_cur.into(), + rlim.rlim_max.into(), + )) } } } From d2d96e745ee1acff0f200dc67eff5d0687f91b1c Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 29 Jan 2022 03:50:21 -0800 Subject: [PATCH 20/24] Improve maxcon diagnostic --- server/src/config/feedback.rs | 95 +++++++++++++++++++---------------- server/src/util/os.rs | 8 +++ 2 files changed, 61 insertions(+), 42 deletions(-) diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index aa7bf904..9df0e033 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -119,18 +119,6 @@ impl ops::DerefMut for ErrorStack { } } -#[test] -fn errorstack_fmt() { - const EXPECTED: &str = "\ -Environment errors: - - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer\ -"; - let mut estk = ErrorStack::new(EMSG_ENV); - estk.push("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); - let fmt = format!("{}", estk); - assert_eq!(fmt, EXPECTED); -} - #[derive(Debug, PartialEq)] pub struct WarningStack { feedback: FeedbackStack, @@ -168,20 +156,6 @@ impl fmt::Display for WarningStack { } } -#[test] -fn warningstack_fmt() { - const EXPECTED: &str = "\ -Environment warnings: - - BGSAVE is disabled. You may lose data if the host crashes - - The setting for `maxcon` is too high\ -"; - let mut wstk = WarningStack::new(EMSG_ENV); - wstk.push("BGSAVE is disabled. You may lose data if the host crashes"); - wstk.push("The setting for `maxcon` is too high"); - let fmt = format!("{}", wstk); - assert_eq!(fmt, EXPECTED); -} - #[derive(Debug)] pub enum ConfigError { OSError(IoError), @@ -249,11 +223,18 @@ fn evaluate_prod_settings(cfg: Configset) -> Result { estack.push("Either multi-socket (TCP and TLS) or TLS only must be enabled"); } // now check maxcon - #[cfg(unix)] - if ResourceLimit::get()?.is_over_limit(cfg.cfg.maxcon) { - estack.push( + if cfg!(unix) { + let rlim = ResourceLimit::get()?; + if rlim.is_over_limit(cfg.cfg.maxcon) { + estack.push( "The value for maximum connections exceeds available resources to the server process", - ); + ); + estack.push( + format!( + "The current process is set to a resource limit of {current} and can be set to a maximum limit of {max} in the OS", + current=rlim.current(),max=rlim.max() + )); + } } if estack.is_empty() { Ok(cfg) @@ -262,22 +243,52 @@ fn evaluate_prod_settings(cfg: Configset) -> Result { } } -#[test] -fn prod_mode_error_fmt() { - let mut estack = ErrorStack::new(EMSG_PROD); - estack.push("BGSAVE must be enabled"); - estack.push("Snapshots must be failsafe"); - estack.push("Either multi-socket (TCP and TLS) or TLS-only mode must be enabled"); - estack.push( - "The value for maximum connections exceeds available resources to the server process", - ); - let e = ConfigError::ProdError(estack); - const EXPECTED: &str = "\ +#[cfg(test)] +mod test { + use super::{ConfigError, ErrorStack, WarningStack, EMSG_ENV, EMSG_PROD}; + + #[test] + fn errorstack_fmt() { + const EXPECTED: &str = "\ +Environment errors: + - Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer\ +"; + let mut estk = ErrorStack::new(EMSG_ENV); + estk.push("Invalid value for `SKY_SYSTEM_PORT`. Expected a 16-bit integer"); + let fmt = format!("{}", estk); + assert_eq!(fmt, EXPECTED); + } + + #[test] + fn warningstack_fmt() { + const EXPECTED: &str = "\ + Environment warnings: + - BGSAVE is disabled. You may lose data if the host crashes + - The setting for `maxcon` is too high\ + "; + let mut wstk = WarningStack::new(EMSG_ENV); + wstk.push("BGSAVE is disabled. You may lose data if the host crashes"); + wstk.push("The setting for `maxcon` is too high"); + let fmt = format!("{}", wstk); + assert_eq!(fmt, EXPECTED); + } + #[test] + fn prod_mode_error_fmt() { + let mut estack = ErrorStack::new(EMSG_PROD); + estack.push("BGSAVE must be enabled"); + estack.push("Snapshots must be failsafe"); + estack.push("Either multi-socket (TCP and TLS) or TLS-only mode must be enabled"); + estack.push( + "The value for maximum connections exceeds available resources to the server process", + ); + let e = ConfigError::ProdError(estack); + const EXPECTED: &str = "\ You have invalid configuration for production mode. Production mode errors: - BGSAVE must be enabled - Snapshots must be failsafe - Either multi-socket (TCP and TLS) or TLS-only mode must be enabled - The value for maximum connections exceeds available resources to the server process\ "; - assert_eq!(format!("{}", e), EXPECTED); + assert_eq!(format!("{}", e), EXPECTED); + } } diff --git a/server/src/util/os.rs b/server/src/util/os.rs index 84223771..c00a846e 100644 --- a/server/src/util/os.rs +++ b/server/src/util/os.rs @@ -63,6 +63,14 @@ mod unix { } } } + /// Returns the current limit + pub const fn current(&self) -> u64 { + self.cur + } + /// Returns the max limit + pub const fn max(&self) -> u64 { + self.max + } } #[test] From 0d2a143e126893447300f392dd824b761e96cf10 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 29 Jan 2022 03:59:13 -0800 Subject: [PATCH 21/24] Fix warningstack_fmt test --- server/src/config/feedback.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index 9df0e033..e431f80a 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -262,9 +262,9 @@ Environment errors: #[test] fn warningstack_fmt() { const EXPECTED: &str = "\ - Environment warnings: - - BGSAVE is disabled. You may lose data if the host crashes - - The setting for `maxcon` is too high\ +Environment warnings: + - BGSAVE is disabled. You may lose data if the host crashes + - The setting for `maxcon` is too high\ "; let mut wstk = WarningStack::new(EMSG_ENV); wstk.push("BGSAVE is disabled. You may lose data if the host crashes"); From d121fa96fb372c2ebc49a04d167789810a8cfcd9 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 29 Jan 2022 05:01:59 -0800 Subject: [PATCH 22/24] Enable config evaluation for prod mode --- server/src/cli.yml | 7 +++ server/src/config/cfgcli.rs | 1 + server/src/config/cfgenv.rs | 1 + server/src/config/cfgfile.rs | 5 +- server/src/config/definitions.rs | 78 ++++++++++++++++++++++++++++++-- server/src/config/feedback.rs | 14 +++--- server/src/config/mod.rs | 41 ++++++++++++----- server/src/config/tests.rs | 63 ++++++++++++++++++++++---- 8 files changed, 178 insertions(+), 32 deletions(-) diff --git a/server/src/cli.yml b/server/src/cli.yml index fcca0f15..51abe55d 100644 --- a/server/src/cli.yml +++ b/server/src/cli.yml @@ -102,6 +102,13 @@ args: takes_value: true help: Set the maximum number of connections value_name: maxcon + - mode: + required: false + long: mode + takes_value: true + short: m + help: Sets the deployment type + value_name: mode subcommands: - upgrade: about: Upgrades old datsets to the latest format supported by this server edition diff --git a/server/src/config/cfgcli.rs b/server/src/config/cfgcli.rs index 3bfa64e9..32e29101 100644 --- a/server/src/config/cfgcli.rs +++ b/server/src/config/cfgcli.rs @@ -85,6 +85,7 @@ pub(super) fn parse_cli_args(matches: ArgMatches) -> Configset { Flag::::new(matches.is_present("noart")), "--noart" ); + fcli!(server_mode, matches.value_of("mode"), "--mode"); fcli!(server_maxcon, matches.value_of("maxcon"), "--maxcon"); // bgsave settings fcli!( diff --git a/server/src/config/cfgenv.rs b/server/src/config/cfgenv.rs index 53615843..47291606 100644 --- a/server/src/config/cfgenv.rs +++ b/server/src/config/cfgenv.rs @@ -48,6 +48,7 @@ pub(super) fn parse_env_config() -> Configset { fenv!(server_tcp, SKY_SYSTEM_HOST, SKY_SYSTEM_PORT); fenv!(server_noart, SKY_SYSTEM_NOART); fenv!(server_maxcon, SKY_SYSTEM_MAXCON); + fenv!(server_mode, SKY_DEPLOY_MODE); // bgsave settings fenv!(bgsave_settings, SKY_BGSAVE_ENABLED, SKY_BGSAVE_DURATION); // snapshot settings diff --git a/server/src/config/cfgfile.rs b/server/src/config/cfgfile.rs index 0331fff7..197833af 100644 --- a/server/src/config/cfgfile.rs +++ b/server/src/config/cfgfile.rs @@ -24,7 +24,7 @@ * */ -use super::{ConfigSourceParseResult, Configset, OptString, TryFromConfigSource}; +use super::{ConfigSourceParseResult, Configset, Modeset, OptString, TryFromConfigSource}; use serde::Deserialize; use std::net::IpAddr; @@ -53,6 +53,8 @@ pub struct ConfigKeyServer { pub(super) noart: Option, /// The maximum number of clients pub(super) maxclient: Option, + /// The deployment mode + pub(super) mode: Option, } /// The BGSAVE section in the config file @@ -170,6 +172,7 @@ pub fn from_file(file: ConfigFile) -> Configset { ); set.server_maxcon(Optional::from(server.maxclient), "server.maxcon"); set.server_noart(Optional::from(server.noart), "server.noart"); + set.server_mode(Optional::from(server.mode), "server.mode"); // bgsave settings if let Some(bgsave) = bgsave { let ConfigKeyBGSAVE { enabled, every } = bgsave; diff --git a/server/src/config/definitions.rs b/server/src/config/definitions.rs index 0f5b7093..2264aca1 100644 --- a/server/src/config/definitions.rs +++ b/server/src/config/definitions.rs @@ -24,10 +24,14 @@ * */ -use super::feedback::WarningStack; -use super::{DEFAULT_IPV4, DEFAULT_PORT}; +use super::{feedback::WarningStack, DEFAULT_IPV4, DEFAULT_PORT}; use crate::dbnet::MAXIMUM_CONNECTION_LIMIT; -use serde::Deserialize; +use core::fmt; +use core::str::FromStr; +use serde::{ + de::{self, Deserializer, Visitor}, + Deserialize, +}; use std::net::IpAddr; /// The BGSAVE configuration @@ -78,6 +82,8 @@ pub struct ConfigurationSet { pub ports: PortConfig, /// The maximum number of connections pub maxcon: usize, + /// The deployment mode + pub mode: Modeset, } impl ConfigurationSet { @@ -87,6 +93,7 @@ impl ConfigurationSet { snapshot: SnapshotConfig, ports: PortConfig, maxcon: usize, + mode: Modeset, ) -> Self { Self { noart, @@ -94,6 +101,7 @@ impl ConfigurationSet { snapshot, ports, maxcon, + mode, } } /// Create a default `ConfigurationSet` with the following setup defaults: @@ -110,6 +118,7 @@ impl ConfigurationSet { SnapshotConfig::default(), PortConfig::new_insecure_only(DEFAULT_IPV4, 2003), MAXIMUM_CONNECTION_LIMIT, + Modeset::User, ) } /// Returns `false` if `noart` is enabled. Otherwise it returns `true` @@ -261,7 +270,7 @@ type RestoreFile = Option; /// - The default configuration /// - A custom supplied configuration pub struct ConfigType { - config: ConfigurationSet, + pub(super) config: ConfigurationSet, restore: RestoreFile, is_custom: bool, warnings: Option, @@ -305,4 +314,65 @@ impl ConfigType { pub fn new_default(restore: RestoreFile) -> Self { Self::_new(ConfigurationSet::default(), restore, false, None) } + /// Check if the current deploy mode is prod + pub const fn is_prod_mode(&self) -> bool { + matches!(self.config.mode, Modeset::Prod) + } + pub fn wpush(&mut self, w: impl ToString) { + match self.warnings.as_mut() { + Some(stack) => stack.push(w), + None => { + self.warnings = { + let mut wstack = WarningStack::new(""); + wstack.push(w); + Some(wstack) + }; + } + } + } +} + +#[derive(Debug, PartialEq)] +pub enum Modeset { + User, + Prod, +} + +impl FromStr for Modeset { + type Err = (); + fn from_str(st: &str) -> Result { + match st { + "user" => Ok(Modeset::User), + "prod" => Ok(Modeset::Prod), + _ => Err(()), + } + } +} + +struct ModesetVisitor; + +impl<'de> Visitor<'de> for ModesetVisitor { + type Value = Modeset; + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Expecting a string with the deployment mode") + } + fn visit_str(self, value: &str) -> Result + where + E: de::Error, + { + match value { + "user" => Ok(Modeset::User), + "prod" => Ok(Modeset::Prod), + _ => return Err(E::custom(format!("Bad value `{value}` for modeset"))), + } + } +} + +impl<'de> Deserialize<'de> for Modeset { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_str(ModesetVisitor) + } } diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index e431f80a..871d750a 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -31,7 +31,7 @@ use core::fmt; use core::ops; use std::io::Error as IoError; // internal imports -use super::{Configset, SnapshotConfig, SnapshotPref}; +use super::{ConfigurationSet, SnapshotConfig, SnapshotPref}; #[cfg(unix)] use crate::util::os::ResourceLimit; @@ -206,26 +206,26 @@ impl fmt::Display for ConfigError { } /// Check if the settings are suitable for use in production mode -fn evaluate_prod_settings(cfg: Configset) -> Result { +pub(super) fn evaluate_prod_settings(cfg: &ConfigurationSet) -> Result<(), ConfigError> { let mut estack = ErrorStack::new(EMSG_PROD); // first check BGSAVE - if cfg.cfg.bgsave.is_disabled() { + if cfg.bgsave.is_disabled() { estack.push("BGSAVE must be enabled"); } // now check snapshot settings (failsafe) - if let SnapshotConfig::Enabled(SnapshotPref { poison, .. }) = cfg.cfg.snapshot { + if let SnapshotConfig::Enabled(SnapshotPref { poison, .. }) = cfg.snapshot { if !poison { estack.push("Snapshots must be failsafe"); } } // now check TLS settings - if cfg.cfg.ports.insecure_only() { + if cfg.ports.insecure_only() { estack.push("Either multi-socket (TCP and TLS) or TLS only must be enabled"); } // now check maxcon if cfg!(unix) { let rlim = ResourceLimit::get()?; - if rlim.is_over_limit(cfg.cfg.maxcon) { + if rlim.is_over_limit(cfg.maxcon) { estack.push( "The value for maximum connections exceeds available resources to the server process", ); @@ -237,7 +237,7 @@ fn evaluate_prod_settings(cfg: Configset) -> Result { } } if estack.is_empty() { - Ok(cfg) + Ok(()) } else { Err(ConfigError::ProdError(estack)) } diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index e66abdae..38a45bda 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -214,7 +214,7 @@ pub struct Configset { impl Configset { const EMSG_ENV: StaticStr = "Environment"; - const EMSG_CLI: StaticStr = "CLI arguments"; + const EMSG_CLI: StaticStr = "CLI"; const EMSG_FILE: StaticStr = "Configuration file"; /// Internal ctor for a given feedback source. We do not want to expose this to avoid @@ -327,16 +327,22 @@ impl Configset { } /// Turns self into a Result that can be used by config::get_config() pub fn into_result(self, restore_file: Option) -> Result { - if self.is_okay() { + let mut target = if self.is_okay() { // no errors, sweet if self.is_mutated() { let Self { cfg, wstack, .. } = self; - Ok(ConfigType::new_custom(cfg, restore_file, wstack)) + ConfigType::new_custom(cfg, restore_file, wstack) } else { - Ok(ConfigType::new_default(restore_file)) + ConfigType::new_default(restore_file) } } else { - Err(ConfigError::ProdError(self.estack)) + return Err(ConfigError::CfgError(self.estack)); + }; + if target.is_prod_mode() { + self::feedback::evaluate_prod_settings(&target.config).map(|_| target) + } else { + target.wpush("Running in `user` mode. Set mode to `prod` in production"); + Ok(target) } } } @@ -376,6 +382,16 @@ impl Configset { ); self.cfg.maxcon = maxcon; } + pub fn server_mode(&mut self, nmode: impl TryFromConfigSource, nmode_key: StaticStr) { + let mut modeset = Modeset::User; + self.try_mutate( + nmode, + &mut modeset, + nmode_key, + "a string with 'user' or 'prod'", + ); + self.cfg.mode = modeset; + } } // bgsave settings @@ -527,18 +543,21 @@ impl Configset { (false, false) => { if nport.is_present() { self.mutated(); - self.wstack - .push("Specifying `{nport_key}` is pointless when TLS is disabled"); + self.wstack.push(format!( + "Specifying `{nport_key}` is pointless when TLS is disabled" + )); } if nonly.is_present() { self.mutated(); - self.wstack - .push("Specifying `{nonly_key}` is pointless when TLS is disabled"); + self.wstack.push(format!( + "Specifying `{nonly_key}` is pointless when TLS is disabled" + )); } if npass.is_present() { self.mutated(); - self.wstack - .push("Specifying `{npass_key}` is pointless when TLS is disabled"); + self.wstack.push(format!( + "Specifying `{npass_key}` is pointless when TLS is disabled" + )); } } } diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 71e15e14..06a6b67d 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -350,8 +350,8 @@ fn get_toml_from_examples_dir(filename: &str) -> TResult { mod cfg_file_tests { use super::get_toml_from_examples_dir; use crate::config::{ - cfgfile, BGSave, Configset, ConfigurationSet, PortConfig, SnapshotConfig, SnapshotPref, - SslOpts, DEFAULT_IPV4, DEFAULT_PORT, + cfgfile, BGSave, Configset, ConfigurationSet, Modeset, PortConfig, SnapshotConfig, + SnapshotPref, SslOpts, DEFAULT_IPV4, DEFAULT_PORT, }; use crate::dbnet::MAXIMUM_CONNECTION_LIMIT; use std::net::{IpAddr, Ipv6Addr}; @@ -402,7 +402,8 @@ mod cfg_file_tests { bgsave: BGSave::default(), snapshot: SnapshotConfig::default(), ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT + maxcon: MAXIMUM_CONNECTION_LIMIT, + mode: Modeset::User } ); } @@ -421,7 +422,8 @@ mod cfg_file_tests { IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0x1)), DEFAULT_PORT ), - maxcon: MAXIMUM_CONNECTION_LIMIT + maxcon: MAXIMUM_CONNECTION_LIMIT, + mode: Modeset::User } ); } @@ -445,7 +447,8 @@ mod cfg_file_tests { Some("/path/to/cert/passphrase.txt".to_owned()) ) ), - MAXIMUM_CONNECTION_LIMIT + MAXIMUM_CONNECTION_LIMIT, + Modeset::User, ) ); } @@ -468,7 +471,8 @@ mod cfg_file_tests { bgsave: BGSave::new(true, 600), snapshot: SnapshotConfig::default(), ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT + maxcon: MAXIMUM_CONNECTION_LIMIT, + mode: Modeset::User } ); } @@ -488,7 +492,8 @@ mod cfg_file_tests { bgsave: BGSave::default(), snapshot: SnapshotConfig::default(), ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT + maxcon: MAXIMUM_CONNECTION_LIMIT, + mode: Modeset::User } ) } @@ -508,7 +513,8 @@ mod cfg_file_tests { bgsave: BGSave::new(true, 600), snapshot: SnapshotConfig::default(), ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT + maxcon: MAXIMUM_CONNECTION_LIMIT, + mode: Modeset::User } ) } @@ -524,7 +530,8 @@ mod cfg_file_tests { bgsave: BGSave::default(), noart: false, ports: PortConfig::default(), - maxcon: MAXIMUM_CONNECTION_LIMIT + maxcon: MAXIMUM_CONNECTION_LIMIT, + mode: Modeset::User } ); } @@ -778,3 +785,41 @@ mod try_from_config_source_impls { } } } + +mod modeset_de { + use crate::config::Modeset; + use serde::Deserialize; + + #[derive(Deserialize, Debug)] + struct Example { + mode: Modeset, + } + + #[test] + fn deserialize_modeset_prod_okay() { + #[derive(Deserialize, Debug)] + struct Example { + mode: Modeset, + } + let toml = r#"mode="prod""#; + let x: Example = toml::from_str(toml).unwrap(); + assert_eq!(x.mode, Modeset::Prod); + } + + #[test] + fn deserialize_modeset_user_okay() { + let toml = r#"mode="user""#; + let x: Example = toml::from_str(toml).unwrap(); + assert_eq!(x.mode, Modeset::User); + } + + #[test] + fn deserialize_modeset_fail() { + let toml = r#"mode="superuser""#; + let e = toml::from_str::(toml).unwrap_err(); + assert_eq!( + e.to_string(), + "Bad value `superuser` for modeset for key `mode` at line 1 column 6" + ); + } +} From 46c048f855f3c5b43a9e9e5f3487924881403370 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 29 Jan 2022 05:09:46 -0800 Subject: [PATCH 23/24] Fix rlimit check on Windows Windows (obviously) doesn't have libc's rlimit so simply avoid any references to it --- server/src/config/feedback.rs | 36 +++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index 871d750a..85728153 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -205,6 +205,27 @@ impl fmt::Display for ConfigError { } } +#[cfg(unix)] +fn check_rlimit_or_err(current: usize, estack: &mut ErrorStack) -> Result<(), ConfigError> { + let rlim = ResourceLimit::get()?; + if rlim.is_over_limit(current) { + estack.push( + "The value for maximum connections exceeds available resources to the server process", + ); + estack.push( + format!( + "The current process is set to a resource limit of {current} and can be set to a maximum limit of {max} in the OS", + current=rlim.current(),max=rlim.max() + )); + } + Ok(()) +} + +#[cfg(not(unix))] +fn check_rlimit_or_err(_: usize, _: &mut ErrorStack) -> Result<(), ConfigError> { + Ok(()) +} + /// Check if the settings are suitable for use in production mode pub(super) fn evaluate_prod_settings(cfg: &ConfigurationSet) -> Result<(), ConfigError> { let mut estack = ErrorStack::new(EMSG_PROD); @@ -222,20 +243,7 @@ pub(super) fn evaluate_prod_settings(cfg: &ConfigurationSet) -> Result<(), Confi if cfg.ports.insecure_only() { estack.push("Either multi-socket (TCP and TLS) or TLS only must be enabled"); } - // now check maxcon - if cfg!(unix) { - let rlim = ResourceLimit::get()?; - if rlim.is_over_limit(cfg.maxcon) { - estack.push( - "The value for maximum connections exceeds available resources to the server process", - ); - estack.push( - format!( - "The current process is set to a resource limit of {current} and can be set to a maximum limit of {max} in the OS", - current=rlim.current(),max=rlim.max() - )); - } - } + check_rlimit_or_err(cfg.maxcon, &mut estack)?; if estack.is_empty() { Ok(()) } else { From d550e0d7a78ec544c7d7a028bfa168f7de39ded5 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 29 Jan 2022 05:29:18 -0800 Subject: [PATCH 24/24] Expect `noart` in production mode This might make one think that we are being outrageously strict, but at the end of the day, it can help investigate crashes or inspect logs without artwork all over the place. Following some discussions, the `user` mode was renamed to `dev` mode. This commit also upgrades some deps, other than clap which has deprecated yaml support (we will continue to use 2.x). Finally, the CHANGELOG was updated. --- CHANGELOG.md | 1 + Cargo.lock | 106 ++++++++++++++++++++++------ cli/Cargo.toml | 4 +- examples/config-files/template.toml | 1 + server/Cargo.toml | 12 ++-- server/src/config/definitions.rs | 8 +-- server/src/config/feedback.rs | 4 ++ server/src/config/mod.rs | 2 +- server/src/config/tests.rs | 18 ++--- sky-bench/Cargo.toml | 4 +- sky-migrate/Cargo.toml | 2 +- 11 files changed, 117 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d08f6cc..2e6ca553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All changes in this project will be noted in this file. ### Additions +- Added `dev/prod` mode for making sure that the recommended production settings are used - Added support for system native endian storage (backward compatible) ## Version 0.7.2 diff --git a/Cargo.lock b/Cargo.lock index 2b46588a..d084e7fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -181,7 +181,7 @@ dependencies = [ "crossterm_winapi", "libc", "mio", - "parking_lot", + "parking_lot 0.11.2", "signal-hook", "signal-hook-mio", "winapi", @@ -287,7 +287,7 @@ checksum = "fcef756dea9cf3db5ce73759cf0467330427a786b47711b8d6c97620d718ceb9" dependencies = [ "cfg-if", "rustix", - "windows-sys", + "windows-sys 0.30.0", ] [[package]] @@ -423,9 +423,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.113" +version = "0.2.116" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eef78b64d87775463c549fbd80e19249ef436ea3bf1de2a1eb7e717ec7fab1e9" +checksum = "565dbd88872dbe4cc8a46e527f26483c1d1f7afa6b884a3bd6cd893d4f98da74" [[package]] name = "libsky" @@ -453,9 +453,9 @@ checksum = "95f5690fef754d905294c56f7ac815836f2513af966aa47f2e07ac79be07827f" [[package]] name = "lock_api" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "712a4d093c9976e24e7dbca41db895dabcbac38eb5f4045393d17a95bdfb1109" +checksum = "88943dd7ef4a2e5a4bfa2753aaab3013e34ce2533d1996fb18ef591e315e2b3b" dependencies = [ "scopeguard", ] @@ -617,7 +617,17 @@ checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" dependencies = [ "instant", "lock_api", - "parking_lot_core", + "parking_lot_core 0.8.5", +] + +[[package]] +name = "parking_lot" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87f5ec2493a61ac0506c0f4199f99070cbe83857b0337006a30f3e6719b8ef58" +dependencies = [ + "lock_api", + "parking_lot_core 0.9.0", ] [[package]] @@ -634,6 +644,19 @@ dependencies = [ "winapi", ] +[[package]] +name = "parking_lot_core" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2f4f894f3865f6c0e02810fc597300f34dc2510f66400da262d8ae10e75767d" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-sys 0.29.0", +] + [[package]] name = "pin-project-lite" version = "0.2.8" @@ -839,18 +862,18 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "serde" -version = "1.0.135" +version = "1.0.136" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cf9235533494ea2ddcdb794665461814781c53f19d87b76e571a1c35acbad2b" +checksum = "ce31e24b01e1e524df96f1c2fdd054405f8d7376249a5110886fb4b658484789" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.135" +version = "1.0.136" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8dcde03d87d4c973c04be249e7d8f0b35db1c848c487bd43032808e59dd8328d" +checksum = "08597e7152fcd306f41838ed3e37be9eaeed2b61c42e2117266a554fab4662f9" dependencies = [ "proc-macro2", "quote", @@ -951,7 +974,7 @@ dependencies = [ "log", "num_cpus", "openssl", - "parking_lot", + "parking_lot 0.12.0", "rand", "regex", "serde", @@ -1081,9 +1104,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.15.0" +version = "1.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fbbf1c778ec206785635ce8ad57fe52b3009ae9e0c9f574a728f3049d3e55838" +checksum = "0c27a64b625de6d309e8c57716ba93021dccf1b3b5c97edd6d3dd2d2135afc0a" dependencies = [ "bytes", "libc", @@ -1091,7 +1114,7 @@ dependencies = [ "mio", "num_cpus", "once_cell", - "parking_lot", + "parking_lot 0.11.2", "pin-project-lite", "signal-hook-registry", "tokio-macros", @@ -1209,43 +1232,86 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-sys" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ceb069ac8b2117d36924190469735767f0990833935ab430155e71a44bafe148" +dependencies = [ + "windows_aarch64_msvc 0.29.0", + "windows_i686_gnu 0.29.0", + "windows_i686_msvc 0.29.0", + "windows_x86_64_gnu 0.29.0", + "windows_x86_64_msvc 0.29.0", +] + [[package]] name = "windows-sys" version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "030b7ff91626e57a05ca64a07c481973cbb2db774e4852c9c7ca342408c6a99a" dependencies = [ - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_msvc", + "windows_aarch64_msvc 0.30.0", + "windows_i686_gnu 0.30.0", + "windows_i686_msvc 0.30.0", + "windows_x86_64_gnu 0.30.0", + "windows_x86_64_msvc 0.30.0", ] +[[package]] +name = "windows_aarch64_msvc" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3d027175d00b01e0cbeb97d6ab6ebe03b12330a35786cbaca5252b1c4bf5d9b" + [[package]] name = "windows_aarch64_msvc" version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "29277a4435d642f775f63c7d1faeb927adba532886ce0287bd985bffb16b6bca" +[[package]] +name = "windows_i686_gnu" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8793f59f7b8e8b01eda1a652b2697d87b93097198ae85f823b969ca5b89bba58" + [[package]] name = "windows_i686_gnu" version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1145e1989da93956c68d1864f32fb97c8f561a8f89a5125f6a2b7ea75524e4b8" +[[package]] +name = "windows_i686_msvc" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8602f6c418b67024be2996c512f5f995de3ba417f4c75af68401ab8756796ae4" + [[package]] name = "windows_i686_msvc" version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d4a09e3a0d4753b73019db171c1339cd4362c8c44baf1bcea336235e955954a6" +[[package]] +name = "windows_x86_64_gnu" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3d615f419543e0bd7d2b3323af0d86ff19cbc4f816e6453f36a2c2ce889c354" + [[package]] name = "windows_x86_64_gnu" version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ca64fcb0220d58db4c119e050e7af03c69e6f4f415ef69ec1773d9aab422d5a" +[[package]] +name = "windows_x86_64_msvc" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11d95421d9ed3672c280884da53201a5c46b7b2765ca6faf34b0d71cf34a3561" + [[package]] name = "windows_x86_64_msvc" version = "0.30.0" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index aa50710d..ea266558 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -14,7 +14,7 @@ skytable = { git = "https://github.com/skytable/client-rust", branch = "next", f "aio-sslv", ], default-features = false } # external deps -tokio = { version = "1.15.0", features = ["full"] } -clap = { version = "2.34.0", features = ["yaml"] } +tokio = { version = "1.16.1", features = ["full"] } +clap = { version = "2", features = ["yaml"] } rustyline = "9.1.2" crossterm = "0.22.1" diff --git a/examples/config-files/template.toml b/examples/config-files/template.toml index 9811e4a9..0da1f975 100644 --- a/examples/config-files/template.toml +++ b/examples/config-files/template.toml @@ -11,6 +11,7 @@ host = "127.0.0.1" # The IP address to which you want sdb to bind to port = 2003 # The port to which you want sdb to bind to noart = false # Set `noart` to true if you want to disable terminal artwork maxcon = 50000 # set the maximum number of clients that the server can accept +mode = "dev" # Set this to `prod` when you're running in production and `dev` when in development # This key is *OPTIONAL* [bgsave] diff --git a/server/Cargo.toml b/server/Cargo.toml index 5fcdf82f..bf77104c 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -14,16 +14,16 @@ skytable = { git = "https://github.com/skytable/client-rust", branch = "next", d ahash = "0.7.6" bytes = "1.1.0" chrono = "0.4.19" -clap = { version = "2.34.0", features = ["yaml"] } +clap = { version = "2", features = ["yaml"] } env_logger = "0.9.0" hashbrown = { version = "0.12.0", features = ["raw"] } log = "0.4.14" num_cpus = "1.13.1" openssl = { version = "0.10.38", features = ["vendored"] } -parking_lot = "0.11.2" +parking_lot = "0.12.0" regex = "1.5.4" -serde = { version = "1.0.135", features = ["derive"] } -tokio = { version = "1.15.0", features = ["full"] } +serde = { version = "1.0.136", features = ["derive"] } +tokio = { version = "1.16.1", features = ["full"] } tokio-openssl = "0.6.3" toml = "0.5.8" @@ -48,10 +48,10 @@ skytable = { git = "https://github.com/skytable/client-rust", features = [ # external deps bincode = "1.3.3" rand = "0.8.4" -tokio = { version = "1.15.0", features = ["test-util"] } +tokio = { version = "1.16.1", features = ["test-util"] } [target.'cfg(unix)'.dependencies] # external deps -libc = "0.2.113" +libc = "0.2.116" [features] nightly = [] diff --git a/server/src/config/definitions.rs b/server/src/config/definitions.rs index 2264aca1..cff91e71 100644 --- a/server/src/config/definitions.rs +++ b/server/src/config/definitions.rs @@ -118,7 +118,7 @@ impl ConfigurationSet { SnapshotConfig::default(), PortConfig::new_insecure_only(DEFAULT_IPV4, 2003), MAXIMUM_CONNECTION_LIMIT, - Modeset::User, + Modeset::Dev, ) } /// Returns `false` if `noart` is enabled. Otherwise it returns `true` @@ -334,7 +334,7 @@ impl ConfigType { #[derive(Debug, PartialEq)] pub enum Modeset { - User, + Dev, Prod, } @@ -342,7 +342,7 @@ impl FromStr for Modeset { type Err = (); fn from_str(st: &str) -> Result { match st { - "user" => Ok(Modeset::User), + "dev" => Ok(Modeset::Dev), "prod" => Ok(Modeset::Prod), _ => Err(()), } @@ -361,7 +361,7 @@ impl<'de> Visitor<'de> for ModesetVisitor { E: de::Error, { match value { - "user" => Ok(Modeset::User), + "dev" => Ok(Modeset::Dev), "prod" => Ok(Modeset::Prod), _ => return Err(E::custom(format!("Bad value `{value}` for modeset"))), } diff --git a/server/src/config/feedback.rs b/server/src/config/feedback.rs index 85728153..d35e5f59 100644 --- a/server/src/config/feedback.rs +++ b/server/src/config/feedback.rs @@ -229,6 +229,10 @@ fn check_rlimit_or_err(_: usize, _: &mut ErrorStack) -> Result<(), ConfigError> /// Check if the settings are suitable for use in production mode pub(super) fn evaluate_prod_settings(cfg: &ConfigurationSet) -> Result<(), ConfigError> { let mut estack = ErrorStack::new(EMSG_PROD); + // check `noart` + if cfg.is_artful() { + estack.push("Terminal artwork should be disabled"); + } // first check BGSAVE if cfg.bgsave.is_disabled() { estack.push("BGSAVE must be enabled"); diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 38a45bda..563df147 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -383,7 +383,7 @@ impl Configset { self.cfg.maxcon = maxcon; } pub fn server_mode(&mut self, nmode: impl TryFromConfigSource, nmode_key: StaticStr) { - let mut modeset = Modeset::User; + let mut modeset = Modeset::Dev; self.try_mutate( nmode, &mut modeset, diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 06a6b67d..b09bae77 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -403,7 +403,7 @@ mod cfg_file_tests { snapshot: SnapshotConfig::default(), ports: PortConfig::default(), maxcon: MAXIMUM_CONNECTION_LIMIT, - mode: Modeset::User + mode: Modeset::Dev } ); } @@ -423,7 +423,7 @@ mod cfg_file_tests { DEFAULT_PORT ), maxcon: MAXIMUM_CONNECTION_LIMIT, - mode: Modeset::User + mode: Modeset::Dev } ); } @@ -448,7 +448,7 @@ mod cfg_file_tests { ) ), MAXIMUM_CONNECTION_LIMIT, - Modeset::User, + Modeset::Dev, ) ); } @@ -472,7 +472,7 @@ mod cfg_file_tests { snapshot: SnapshotConfig::default(), ports: PortConfig::default(), maxcon: MAXIMUM_CONNECTION_LIMIT, - mode: Modeset::User + mode: Modeset::Dev } ); } @@ -493,7 +493,7 @@ mod cfg_file_tests { snapshot: SnapshotConfig::default(), ports: PortConfig::default(), maxcon: MAXIMUM_CONNECTION_LIMIT, - mode: Modeset::User + mode: Modeset::Dev } ) } @@ -514,7 +514,7 @@ mod cfg_file_tests { snapshot: SnapshotConfig::default(), ports: PortConfig::default(), maxcon: MAXIMUM_CONNECTION_LIMIT, - mode: Modeset::User + mode: Modeset::Dev } ) } @@ -531,7 +531,7 @@ mod cfg_file_tests { noart: false, ports: PortConfig::default(), maxcon: MAXIMUM_CONNECTION_LIMIT, - mode: Modeset::User + mode: Modeset::Dev } ); } @@ -808,9 +808,9 @@ mod modeset_de { #[test] fn deserialize_modeset_user_okay() { - let toml = r#"mode="user""#; + let toml = r#"mode="dev""#; let x: Example = toml::from_str(toml).unwrap(); - assert_eq!(x.mode, Modeset::User); + assert_eq!(x.mode, Modeset::Dev); } #[test] diff --git a/sky-bench/Cargo.toml b/sky-bench/Cargo.toml index 1dce6daf..9a243dc5 100644 --- a/sky-bench/Cargo.toml +++ b/sky-bench/Cargo.toml @@ -11,8 +11,8 @@ version = "0.7.2" libstress = { path = "../libstress" } skytable = { git = "https://github.com/skytable/client-rust", branch = "next" } # external deps -clap = { version = "2.34.0", features = ["yaml"] } +clap = { version = "2", features = ["yaml"] } devtimer = "4.0.1" rand = "0.8.4" -serde = { version = "1.0.135", features = ["derive"] } +serde = { version = "1.0.136", features = ["derive"] } serde_json = "1.0.78" diff --git a/sky-migrate/Cargo.toml b/sky-migrate/Cargo.toml index 4071f436..4785aede 100644 --- a/sky-migrate/Cargo.toml +++ b/sky-migrate/Cargo.toml @@ -11,4 +11,4 @@ skytable = { git = "https://github.com/skytable/client-rust.git" } env_logger = "0.9.0" bincode = "1.3.3" log = "0.4.14" -clap = { version = "2.34.0", features = ["yaml"] } +clap = { version = "2", features = ["yaml"] }