mirror of
https://github.com/EasyTier/EasyTier.git
synced 2026-10-10 12:46:14 -08:00
fix(cli): warn about ignored TOML logging configuration (#2661)
* fix(cli): warn about ignored TOML logging configuration Keep logging process-wide and direct users to CLI options or environment variables when legacy logging sections are present. Emit diagnostics for files, config directories, stdin, and config validation. Refs #984 * fix(test): use relocated core binary in nextest archives Resolve the runtime NEXTEST_BIN_EXE path before clearing the child environment, with a compile-time fallback for cargo test. This avoids using the build runner's binary path after extracting an archive on another runner.
This commit is contained in:
1 parent
d96eaa668b
commit
24afb4ecb4
3 files changed
+183
-5
No files matched your search
@@ -16,6 +16,8 @@ pub use easytier_core::config::toml::*;
|
||||
|
||||
#[cfg(feature = "management")]
|
||||
use crate::common::env_parser;
|
||||
#[cfg(feature = "logging")]
|
||||
use crate::common::log;
|
||||
use crate::tunnel::IpScheme;
|
||||
|
||||
#[cfg(feature = "management")]
|
||||
@@ -37,7 +39,39 @@ pub fn parse_encryption_algorithm(value: &str) -> Result<EncryptionAlgorithm, St
|
||||
pub fn load_toml_config_from_path(path: &PathBuf) -> Result<TomlConfigLoader, anyhow::Error> {
|
||||
let config = std::fs::read_to_string(path)
|
||||
.with_context(|| format!("failed to read config file: {}", path.display()))?;
|
||||
TomlConfigLoader::new_from_str_with_source(&path.display().to_string(), &config)
|
||||
load_toml_config_from_str_with_source(&path.display().to_string(), &config)
|
||||
}
|
||||
|
||||
pub fn load_toml_config_from_str_with_source(
|
||||
source_name: &str,
|
||||
config_str: &str,
|
||||
) -> Result<TomlConfigLoader, anyhow::Error> {
|
||||
let config = TomlConfigLoader::new_from_str_with_source(source_name, config_str)?;
|
||||
#[cfg(feature = "logging")]
|
||||
{
|
||||
let ignored_sections = ignored_logging_sections(config_str);
|
||||
if !ignored_sections.is_empty() {
|
||||
log::warn!(
|
||||
config_source = source_name,
|
||||
?ignored_sections,
|
||||
"Logging configuration in TOML is ignored because logging is process-wide. \
|
||||
Use command-line options (e.g. --console-log-level, --file-log-level, --file-log-dir) \
|
||||
or ET_* logging environment variables instead."
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
#[cfg(any(feature = "logging", test))]
|
||||
fn ignored_logging_sections(config_str: &str) -> Vec<&'static str> {
|
||||
let Ok(config) = toml::from_str::<toml::Table>(config_str) else {
|
||||
return Vec::new();
|
||||
};
|
||||
["file_logger", "console_logger"]
|
||||
.into_iter()
|
||||
.filter(|section| config.contains_key(*section))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(feature = "management-rpc")]
|
||||
@@ -71,7 +105,7 @@ pub async fn load_config_from_file(
|
||||
.read_to_string(&mut stdin)
|
||||
.await
|
||||
.context("failed to read config from stdin")?;
|
||||
let config = TomlConfigLoader::new_from_str_with_source("stdin", &stdin)?;
|
||||
let config = load_toml_config_from_str_with_source("stdin", &stdin)?;
|
||||
return Ok((config, ConfigFileControl::STATIC_CONFIG));
|
||||
}
|
||||
|
||||
@@ -91,7 +125,7 @@ pub async fn load_config_from_file(
|
||||
}
|
||||
|
||||
let source_name = config_file.display().to_string();
|
||||
let config = TomlConfigLoader::new_from_str_with_source(&source_name, &expanded_config_str)?;
|
||||
let config = load_toml_config_from_str_with_source(&source_name, &expanded_config_str)?;
|
||||
let mut control = config_file_control_from_path(config_file.clone()).await;
|
||||
|
||||
if uses_env_vars {
|
||||
@@ -119,6 +153,41 @@ mod tests {
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn detects_only_top_level_logging_sections() {
|
||||
let cases: [(&str, &[&str]); 6] = [
|
||||
("", &[]),
|
||||
("[file_logger]\nlevel = \"info\"", &["file_logger"]),
|
||||
("console_logger = { level = \"warn\" }", &["console_logger"]),
|
||||
(
|
||||
"[console_logger]\n[file_logger]",
|
||||
&["file_logger", "console_logger"],
|
||||
),
|
||||
("# [file_logger]\ninstance_name = '[console_logger]'", &[]),
|
||||
("[unknown.file_logger]\nlevel = \"info\"", &[]),
|
||||
];
|
||||
for (input, expected) in cases {
|
||||
assert_eq!(ignored_logging_sections(input), expected, "{input}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignored_logging_values_do_not_change_config_validation() {
|
||||
let input = r#"
|
||||
instance_name = "legacy-logging"
|
||||
file_logger = "ignored"
|
||||
[console_logger]
|
||||
level = 123
|
||||
"#;
|
||||
let config = load_toml_config_from_str_with_source("legacy.toml", input).unwrap();
|
||||
|
||||
assert_eq!(config.get_inst_name(), "legacy-logging");
|
||||
assert_eq!(
|
||||
ignored_logging_sections(input),
|
||||
["file_logger", "console_logger"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_adapter_preserves_file_name_in_parse_error() {
|
||||
let mut file = NamedTempFile::new().unwrap();
|
||||
|
||||
@@ -5,7 +5,8 @@ use crate::{
|
||||
ConfigFileControl, ConfigLoader, ConsoleLoggerConfig, EncryptionAlgorithm,
|
||||
FileLoggerConfig, LoggingConfigLoader, NetworkIdentity, PeerConfig, PortForwardConfig,
|
||||
TomlConfigLoader, VpnPortalClientConfig, VpnPortalConfig, add_proxy_network_to_config,
|
||||
load_config_from_file, load_toml_config_from_path, parse_mapped_listener_urls,
|
||||
load_config_from_file, load_toml_config_from_path,
|
||||
load_toml_config_from_str_with_source, parse_mapped_listener_urls,
|
||||
},
|
||||
constants::EASYTIER_VERSION,
|
||||
log,
|
||||
@@ -1789,6 +1790,10 @@ pub async fn main() -> ExitCode {
|
||||
|
||||
// Verify configurations
|
||||
if cli.check_config {
|
||||
if let Err(error) = log::init_console() {
|
||||
eprintln!("Failed to initialize logging: {error}");
|
||||
return ExitCode::FAILURE;
|
||||
}
|
||||
if let Err(error) = validate_config(&cli).await {
|
||||
log::error!(%error, "Config validation failed");
|
||||
return ExitCode::FAILURE;
|
||||
@@ -1824,7 +1829,7 @@ async fn validate_config(cli: &Cli) -> anyhow::Result<()> {
|
||||
.read_to_string(&mut stdin)
|
||||
.await
|
||||
.context("failed to read config from stdin")?;
|
||||
TomlConfigLoader::new_from_str_with_source("stdin", stdin.as_str())?;
|
||||
load_toml_config_from_str_with_source("stdin", stdin.as_str())?;
|
||||
} else {
|
||||
load_toml_config_from_path(config_file)?;
|
||||
};
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
#![cfg(feature = "management")]
|
||||
|
||||
use std::{
|
||||
io::Write as _,
|
||||
process::{Command, Output, Stdio},
|
||||
};
|
||||
|
||||
use tempfile::NamedTempFile;
|
||||
|
||||
fn check_config_command() -> Command {
|
||||
// Nextest remaps this path when running tests from an extracted archive.
|
||||
let binary = std::env::var_os("NEXTEST_BIN_EXE_easytier_core")
|
||||
.unwrap_or_else(|| env!("CARGO_BIN_EXE_easytier-core").into());
|
||||
let mut command = Command::new(binary);
|
||||
command
|
||||
.env_clear()
|
||||
.arg("--check-config")
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped());
|
||||
command
|
||||
}
|
||||
|
||||
fn stderr_on_success(output: Output) -> String {
|
||||
let stderr = String::from_utf8(output.stderr).unwrap();
|
||||
assert!(output.status.success(), "{stderr}");
|
||||
stderr
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_config_warns_for_each_file_with_ignored_logging() {
|
||||
let mut console_config = NamedTempFile::new().unwrap();
|
||||
writeln!(console_config, "[console_logger]\nlevel = 'off'").unwrap();
|
||||
let mut file_config = NamedTempFile::new().unwrap();
|
||||
writeln!(file_config, "[file_logger]\nlevel = 'info'").unwrap();
|
||||
|
||||
let output = check_config_command()
|
||||
.arg("--config-file")
|
||||
.arg(console_config.path())
|
||||
.arg(file_config.path())
|
||||
.output()
|
||||
.unwrap();
|
||||
let stderr = stderr_on_success(output);
|
||||
|
||||
assert_eq!(
|
||||
stderr
|
||||
.matches("Logging configuration in TOML is ignored")
|
||||
.count(),
|
||||
2
|
||||
);
|
||||
for file in [&console_config, &file_config] {
|
||||
assert!(stderr.contains(&format!("{:?}", file.path().display().to_string())));
|
||||
}
|
||||
assert!(stderr.contains("console_logger"));
|
||||
assert!(stderr.contains("file_logger"));
|
||||
assert!(stderr.contains("process-wide"));
|
||||
assert!(stderr.contains("--console-log-level"));
|
||||
assert!(stderr.contains("--file-log-level"));
|
||||
assert!(stderr.contains("ET_*"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_config_warns_for_stdin_without_validating_ignored_values() {
|
||||
let mut child = check_config_command()
|
||||
.args(["--config-file", "-"])
|
||||
.stdin(Stdio::piped())
|
||||
.spawn()
|
||||
.unwrap();
|
||||
child
|
||||
.stdin
|
||||
.take()
|
||||
.unwrap()
|
||||
.write_all(b"file_logger = 'ignored'\n[console_logger]\nlevel = 123\n")
|
||||
.unwrap();
|
||||
let stderr = stderr_on_success(child.wait_with_output().unwrap());
|
||||
|
||||
assert_eq!(
|
||||
stderr
|
||||
.matches("Logging configuration in TOML is ignored")
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
assert!(stderr.contains("stdin"));
|
||||
assert!(stderr.contains("file_logger"));
|
||||
assert!(stderr.contains("console_logger"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_config_does_not_warn_for_comments_strings_or_nested_sections() {
|
||||
let mut config = NamedTempFile::new().unwrap();
|
||||
writeln!(
|
||||
config,
|
||||
"# [file_logger]\ninstance_name = '[console_logger]'\n[unknown.file_logger]\nlevel = 'info'"
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let output = check_config_command()
|
||||
.arg("--config-file")
|
||||
.arg(config.path())
|
||||
.output()
|
||||
.unwrap();
|
||||
let stderr = stderr_on_success(output);
|
||||
|
||||
assert!(!stderr.contains("Logging configuration in TOML is ignored"));
|
||||
}
|
||||
Reference in new issue
Block a user