diff --git a/Cargo.lock b/Cargo.lock index 35dc9a882..51911ba55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3320,6 +3320,7 @@ dependencies = [ "pbkdf2", "petgraph", "rand", + "regex", "serde", "test-macros", "thiserror", diff --git a/crates/curp/src/server/curp_node.rs b/crates/curp/src/server/curp_node.rs index 51866774f..c4818da9a 100644 --- a/crates/curp/src/server/curp_node.rs +++ b/crates/curp/src/server/curp_node.rs @@ -28,8 +28,10 @@ use utils::{ use super::{ cmd_board::{CmdBoardRef, CommandBoard}, cmd_worker::{conflict_checked_mpmc, start_cmd_workers}, - conflict::spec_pool_new::{SpObject, SpeculativePool}, - conflict::uncommitted_pool::{UcpObject, UncommittedPool}, + conflict::{ + spec_pool_new::{SpObject, SpeculativePool}, + uncommitted_pool::{UcpObject, UncommittedPool}, + }, gc::gc_cmd_board, lease_manager::LeaseManager, raw_curp::{AppendEntries, RawCurp, Vote}, diff --git a/crates/curp/src/server/mod.rs b/crates/curp/src/server/mod.rs index b6fca3a99..8ee55a599 100644 --- a/crates/curp/src/server/mod.rs +++ b/crates/curp/src/server/mod.rs @@ -10,8 +10,10 @@ use utils::ClientTlsConfig; use utils::{config::CurpConfig, task_manager::TaskManager, tracing::Extract}; use self::curp_node::CurpNode; -pub use self::raw_curp::RawCurp; -pub use self::{conflict::spec_pool_new::SpObject, conflict::uncommitted_pool::UcpObject}; +pub use self::{ + conflict::{spec_pool_new::SpObject, uncommitted_pool::UcpObject}, + raw_curp::RawCurp, +}; use crate::{ cmd::{Command, CommandExecutor}, members::{ClusterInfo, ServerId}, diff --git a/crates/curp/src/server/raw_curp/mod.rs b/crates/curp/src/server/raw_curp/mod.rs index a129a9c71..b8e2437b5 100644 --- a/crates/curp/src/server/raw_curp/mod.rs +++ b/crates/curp/src/server/raw_curp/mod.rs @@ -47,9 +47,11 @@ use self::{ state::{CandidateState, LeaderState, State}, }; use super::{ - cmd_worker::CEEventTxApi, conflict::spec_pool_new::SpeculativePool, - conflict::uncommitted_pool::UncommittedPool, lease_manager::LeaseManagerRef, - storage::StorageApi, DB, + cmd_worker::CEEventTxApi, + conflict::{spec_pool_new::SpeculativePool, uncommitted_pool::UncommittedPool}, + lease_manager::LeaseManagerRef, + storage::StorageApi, + DB, }; use crate::{ cmd::Command, diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 96a2bc3b5..528912973 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -29,6 +29,7 @@ parking_lot = { version = "0.12.2", optional = true } pbkdf2 = { version = "0.12.2", features = ["simple"] } petgraph = "0.6.4" rand = "0.8.5" +regex = "1.10.4" serde = { version = "1.0.199", features = ["derive"] } thiserror = "1.0.61" tokio = { version = "0.2.25", package = "madsim-tokio", features = [ diff --git a/crates/utils/src/config.rs b/crates/utils/src/config.rs index dcf7e547d..42213b1e0 100644 --- a/crates/utils/src/config.rs +++ b/crates/utils/src/config.rs @@ -801,9 +801,9 @@ impl LogConfig { /// Generate a new `LogConfig` object #[must_use] #[inline] - pub fn new(path: PathBuf, rotation: RotationConfig, level: LevelConfig) -> Self { + pub fn new(path: Option, rotation: RotationConfig, level: LevelConfig) -> Self { Self { - path: Some(path), + path, rotation, level, } @@ -1326,7 +1326,7 @@ mod tests { assert_eq!( config.log, LogConfig::new( - PathBuf::from("/var/log/xline"), + Some(PathBuf::from("/var/log/xline")), RotationConfig::Daily, LevelConfig::INFO ) @@ -1452,7 +1452,7 @@ mod tests { assert_eq!( config.log, LogConfig::new( - PathBuf::from("/var/log/xline"), + Some(PathBuf::from("/var/log/xline")), RotationConfig::Daily, LevelConfig::INFO ) diff --git a/crates/utils/src/parser.rs b/crates/utils/src/parser.rs index ba7186074..c37019c08 100644 --- a/crates/utils/src/parser.rs +++ b/crates/utils/src/parser.rs @@ -1,6 +1,7 @@ -use std::{collections::HashMap, time::Duration}; +use std::{collections::HashMap, path::PathBuf, time::Duration}; use clippy_utilities::OverflowArithmetic; +use regex::Regex; use thiserror::Error; use crate::config::{ @@ -162,6 +163,37 @@ pub fn parse_state(s: &str) -> Result { } } +/// Parse `LOG_PATH` from string +/// # Errors +/// Return error when parsing the given string to `PathBuf` failed +#[inline] +pub fn parse_log_file(s: &str) -> Result { + if s.is_empty() { + return Err(ConfigParseError::InvalidValue( + "the log path should not be empty".to_owned(), + )); + } + // This regular expression matches file paths in a specific format. + // Explanation of the regex pattern: + // ^(\.|\.\.|~)? - Matches an optional prefix consisting of a dot (.), two dots (..), or a tilde (~). + // (/[a-zA-Z0-9._-]+)+/? - Matches one or more occurrences of a forward slash (/) followed by one or more alphanumeric characters, dots (.), underscores (_), or hyphens (-). + // /?$ - Matches an optional trailing forward slash (/) at the end of the string. // Overall, this regex pattern is used to validate file paths that follow + // a specific format commonly used in systems like Cargo in Rust. + let re = Regex::new(r"^(\.|\.\.|~)?(/[a-zA-Z0-9._-]+)+/?$").unwrap_or_else(|err| { + unreachable!( + r"regex expression (^(\.|\.\.|~)?(/[a-zA-Z0-9._-]+)+/?$) should not return Error: {}", + err.to_string() + ) + }); + if re.is_match(s) { + Ok(PathBuf::from(s)) + } else { + Err(ConfigParseError::InvalidValue(format!( + "Invalid log path {s}" + ))) + } +} + /// Parse `LevelConfig` from string /// # Errors /// Return error when parsing the given string to `LevelConfig` failed @@ -366,4 +398,30 @@ mod test { ); assert!(parse_metrics_push_protocol("thrift").is_err()); } + + #[test] + fn test_parse_log_file() { + // Test case 1: Valid log file path + assert!(parse_log_file("/path/to/log/file.log").is_ok()); + + // Test case 2: Empty log file path + assert!(parse_log_file("").is_err()); + + // Test case 4: Log file path with spaces + assert!(parse_log_file("/path/with spaces/log file.log").is_err()); + + // Test case 5: Log file path with special character + assert!(parse_log_file("/path/with@spaces/log?!file.log").is_err()); + + // Test case 6: Log file path with special character + assert!(parse_log_file("/path/with-spaces/log_file.log-123.456-789").is_ok()); + + // Test case 7: Log file path starts with ., .. or ~ + assert!(parse_log_file("./path/with-spaces/log_file.log-123.456-789").is_ok()); + assert!(parse_log_file("../path/with-spaces/log_file.log-123.456-789").is_ok()); + assert!(parse_log_file("~/path/with-spaces/log_file.log-123.456-789").is_ok()); + + assert!(parse_log_file(".../path/with-spaces/log_file.log-123.456-789").is_err()); + assert!(parse_log_file("~~/path/with-spaces/log_file.log-123.456-789").is_err()); + } } diff --git a/crates/xline/src/utils/args.rs b/crates/xline/src/utils/args.rs index cf24b669c..f16c55002 100644 --- a/crates/xline/src/utils/args.rs +++ b/crates/xline/src/utils/args.rs @@ -20,8 +20,8 @@ use utils::{ MetricsPushProtocol, RotationConfig, ServerTimeout, StorageConfig, TlsConfig, TraceConfig, XlineServerConfig, }, - parse_batch_bytes, parse_duration, parse_log_level, parse_members, parse_metrics_push_protocol, - parse_rotation, parse_state, ConfigFileError, + parse_batch_bytes, parse_duration, parse_log_file, parse_log_level, parse_members, + parse_metrics_push_protocol, parse_rotation, parse_state, ConfigFileError, }; /// Xline server config path env name @@ -92,8 +92,8 @@ pub struct ServerArgs { #[clap(long, value_parser = parse_metrics_push_protocol, default_value_t = default_metrics_push_protocol())] metrics_push_protocol: MetricsPushProtocol, /// Log file path - #[clap(long, default_value = "/stdout")] - log_file: PathBuf, + #[clap(long, value_parser = parse_log_file, default_value = None)] + log_file: Option, /// Log rotate strategy, eg: never, hourly, daily #[clap(long, value_parser = parse_rotation, default_value_t = default_rotation())] log_rotate: RotationConfig, diff --git a/crates/xline/src/utils/trace.rs b/crates/xline/src/utils/trace.rs index 9de3cf7ff..b605c9972 100644 --- a/crates/xline/src/utils/trace.rs +++ b/crates/xline/src/utils/trace.rs @@ -8,15 +8,13 @@ use utils::config::{default_rotation, file_appender, LogConfig, TraceConfig}; /// Return a Box trait from the config fn generate_writer(name: &str, log_config: &LogConfig) -> Box { - match *log_config.path() { - Some(ref file_path) if file_path.to_string_lossy() == "/stdout" => { - if *log_config.rotation() != default_rotation() { - warn!("The log is output to the terminal, so the rotation parameter is ignored."); - } - Box::new(std::io::stdout()) + if let Some(ref file_path) = *log_config.path() { + Box::new(file_appender(*log_config.rotation(), file_path, name)) + } else { + if *log_config.rotation() != default_rotation() { + warn!("The log is output to the terminal, so the rotation parameter is ignored."); } - Some(ref file_path) => Box::new(file_appender(*log_config.rotation(), file_path, name)), - None => unreachable!("the path of log cannot be empty in config"), + Box::new(std::io::stdout()) } } diff --git a/scripts/common.sh b/scripts/common.sh index eee0ad2f9..9c5d7de99 100644 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -37,10 +37,10 @@ function common::run_xline() { ith=${1} mount_point="-v ${DIR}:/mnt" if [ -n "$LOG_PATH" ]; then - mkdir -p ${LOG_PATH}/node${ith} - mount_point="${mount_point} -v ${LOG_PATH}/node${ith}:/var/log/xline" + mkdir -p ${DIR}/logs/node${ith} + mount_point="${mount_point} -v ${DIR}/logs/node${ith}:${LOG_PATH}" fi - + log::info starting container node${ith} ... cmd="/usr/local/bin/xline \ --name node${1} \ @@ -56,7 +56,7 @@ function common::run_xline() { --initial-cluster-state=${3}" if [ -n "${LOG_PATH}" ]; then - cmd="${cmd} --log-file /var/log/xline" + cmd="${cmd} --log-file ${LOG_PATH}" fi if [ -n "$LOG_LEVEL" ]; then @@ -68,18 +68,19 @@ function common::run_xline() { fi if [ -n "$RUST_LOG" ]; then - docker run -e RUST_LOG=${RUST_LOG} -d -it --rm --name=node${ith} --net=xline_net \ + docker_cmd="docker run -e RUST_LOG=${RUST_LOG} -d -it --rm --name=node${ith} --net=xline_net \ --ip=${SERVERS[$ith]} --cap-add=NET_ADMIN --cpu-shares=1024 \ - -m=512M ${mount_point} ${XLINE_IMAGE} ${cmd} - else - docker run -d -it --rm --name=node${ith} --net=xline_net \ + -m=512M ${mount_point} ${XLINE_IMAGE} ${cmd}" + else + docker_cmd="docker run -d -it --rm --name=node${ith} --net=xline_net \ --ip=${SERVERS[$ith]} --cap-add=NET_ADMIN --cpu-shares=1024 \ - -m=512M ${mount_point} ${XLINE_IMAGE} ${cmd} + -m=512M ${mount_point} ${XLINE_IMAGE} ${cmd}" fi - - - + + eval ${docker_cmd} wait $! - log::info "container node${ith} started: ${cmd}" + log::info "${docker_cmd}" + + } diff --git a/scripts/quick_start.sh b/scripts/quick_start.sh index de2c90dab..406171b88 100755 --- a/scripts/quick_start.sh +++ b/scripts/quick_start.sh @@ -11,11 +11,7 @@ source $DIR/log.sh stop_all() { log::info stopping for name in "node1" "node2" "node3" "client"; do - docker_id=$(docker ps -qf "name=${name}") - if [ -n "$docker_id" ]; then - docker exec $docker_id rm -rf $LOG_PATH/$name - docker stop $docker_id -t 1 - fi + common::stop_container ${name} done docker network rm xline_net >/dev/null 2>&1 docker stop "prometheus" > /dev/null 2>&1 diff --git a/scripts/validation_test.sh b/scripts/validation_test.sh index d454d4775..f53a8d9c7 100755 --- a/scripts/validation_test.sh +++ b/scripts/validation_test.sh @@ -6,7 +6,7 @@ source $DIR/log.sh QUICK_START="${DIR}/quick_start.sh" ETCDCTL="docker exec -i client etcdctl --endpoints=http://172.20.0.3:2379,http://172.20.0.4:2379" LOCK_CLIENT="docker exec -i client /mnt/validation_lock_client --endpoints=http://172.20.0.3:2379,http://172.20.0.4:2379,http://172.20.0.5:2379" -export LOG_PATH=/mnt/logs +export LOG_PATH=/var/log/xline export LOG_LEVEL=debug bash ${QUICK_START} @@ -30,7 +30,6 @@ function run_new_member() { if [ -n "$docker_id" ]; then docker stop $docker_id fi - common::run_container 4 common::run_xline 4 ${NEWMEMBERS} existing }