From e5c6ea28087f99bcc1ddae4f58f1b8798b4d170c Mon Sep 17 00:00:00 2001 From: Adrien Guillo Date: Tue, 6 Jun 2023 05:11:34 -0400 Subject: [PATCH] Ignore commented lines when rendering templated config files (#3498) Co-authored-by: Paul Masurel --- config/quickwit.yaml | 4 +- quickwit/quickwit-config/src/templating.rs | 126 +++++++++++++-------- 2 files changed, 79 insertions(+), 51 deletions(-) diff --git a/config/quickwit.yaml b/config/quickwit.yaml index ef6ab62d1e7..cc2748ccc78 100644 --- a/config/quickwit.yaml +++ b/config/quickwit.yaml @@ -80,8 +80,8 @@ version: 0.6 # access_key: ${QW_AZURE_STORAGE_ACCESS_KEY} # # s3: -# endpoint: ${QW_S3_ENDPOINT:-http://localhost:4566} -# force_path_style_access: true +# endpoint: ${QW_S3_ENDPOINT} +# force_path_style_access: ${QW_S3_FORCE_PATH_STYLE_ACCESS:-true} # disable_multi_object_delete_requests: true # # -------------------------------- Metastore settings -------------------------------- diff --git a/quickwit/quickwit-config/src/templating.rs b/quickwit/quickwit-config/src/templating.rs index 2ac62c95f63..0905a82e996 100644 --- a/quickwit/quickwit-config/src/templating.rs +++ b/quickwit/quickwit-config/src/templating.rs @@ -18,6 +18,7 @@ // along with this program. If not, see . use std::collections::HashMap; +use std::io::BufRead; use anyhow::{bail, Context, Result}; use new_string_template::template::Template; @@ -29,7 +30,8 @@ use tracing::debug; // ENV_VAR or ENV_VAR:DEFAULT // Ignores whitespaces in curly braces static TEMPLATE_ENV_VAR_CAPTURE: Lazy = Lazy::new(|| { - Regex::new(r"\$\{\s*([A-Za-z0-9_]+)(?:(?::\-)([\S]+))?\s*}").expect("Failed to compile regular expression. This should never happen! Please, report on https://github.com/quickwit-oss/quickwit/issues.") + Regex::new(r"\$\{\s*([A-Za-z0-9_]+)\s*(?::\-\s*([\S]+)\s*)?}") + .expect("The regular expression should compile.") }); pub fn render_config(config_content: &[u8]) -> Result { @@ -38,39 +40,54 @@ pub fn render_config(config_content: &[u8]) -> Result { let mut values = HashMap::new(); - for captures in TEMPLATE_ENV_VAR_CAPTURE.captures_iter(template_str) { - let env_var_key = captures - .get(1) - .expect("Captures should always have at least one match.") - .as_str(); - let substitution_value = { - if let Ok(env_var_value) = std::env::var(env_var_key) { - debug!( - env_var_name=%env_var_key, - env_var_value=%env_var_value, - "Environment variable is set, substituting with environment variable value." - ); - env_var_value - } else if let Some(default_match) = captures.get(2) { - let default_value = default_match.as_str().to_string(); - debug!( - env_var_name=%env_var_key, - default_value=%default_value, - "Environment variable is not set, substituting with default value." - ); - default_value - } else { - bail!( - "Failed to render config file template: environment variable `{env_var_key}` \ - is not set and no default value was provided." - ); - } - }; - values.insert(env_var_key, substitution_value); + for (line_no, line_res) in config_content.lines().enumerate() { + let line = line_res?; + + for captures in TEMPLATE_ENV_VAR_CAPTURE.captures_iter(&line) { + let env_var_key = captures + .get(1) + .expect("Captures should always have at least one match.") + .as_str(); + let substitution_value = { + if line.trim_start().starts_with('#') { + debug!( + env_var_name=%env_var_key, + "Config file line #{line_no} is commented out, skipping." + ); + // This line is commented out, return the line as is. + captures + .get(0) + .expect("The 0th capture should aways be set.") + .as_str() + .to_string() + } else if let Ok(env_var_value) = std::env::var(env_var_key) { + debug!( + env_var_name=%env_var_key, + env_var_value=%env_var_value, + "Environment variable is set, substituting with environment variable value." + ); + env_var_value + } else if let Some(default_match) = captures.get(2) { + let default_value = default_match.as_str().to_string(); + debug!( + env_var_name=%env_var_key, + default_value=%default_value, + "Environment variable is not set, substituting with default value." + ); + default_value + } else { + bail!( + "Failed to render config file template: environment variable \ + `{env_var_key}` is not set and no default value is provided." + ); + } + }; + values.insert(env_var_key.to_string(), substitution_value); + } } let template = Template::new(template_str).with_regex(&TEMPLATE_ENV_VAR_CAPTURE); let rendered = template - .render(&values) + .render_string(&values) .context("Failed to render config file template.")?; Ok(rendered) } @@ -94,44 +111,37 @@ mod test { } #[test] - fn test_template_render_whitespaces() { + fn test_template_render_supports_whitespaces() { env::set_var( "TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST", "s3://test-bucket/metastore", ); { - let config_content = b"metastore_uri: ${TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST}"; - let rendered = render_config(config_content).unwrap(); - assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore"); - } - { - let config_content = b"metastore_uri: ${TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST }"; + let config_content = b"metastore_uri: ${ TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST }"; let rendered = render_config(config_content).unwrap(); assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore"); } + } + + #[test] + fn test_template_render_with_default_value() { { - let config_content = b"metastore_uri: ${ TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST}"; + let config_content = + b"metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST:-s3://test-bucket/metastore}"; let rendered = render_config(config_content).unwrap(); assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore"); } { - let config_content = b"metastore_uri: ${ TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST }"; + let config_content = + b"metastore_uri: ${ QW_ENV_VAR_DOES_NOT_EXIST :- s3://test-bucket/metastore }"; let rendered = render_config(config_content).unwrap(); assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore"); } } - #[test] - fn test_template_render_default_value() { - let config_content = - b"metastore_uri: ${QW_NO_ENV_WITH_THIS_NAME:-s3://test-bucket/metastore}"; - let rendered = render_config(config_content).unwrap(); - assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore"); - } - #[test] fn test_template_render_should_panic() { - let config_content = b"metastore_uri: ${QW_NO_ENV_WITH_THIS_NAME}"; + let config_content = b"metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST}"; render_config(config_content).unwrap_err(); } @@ -147,4 +157,22 @@ mod test { std::env::remove_var("TEST_TEMPLATE_RENDER_ENV_VAR_DEFAULT_USE_ENV"); assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore"); } + + #[test] + fn test_template_render_ignores_commented_lines() { + { + let config_content = b"# metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST}"; + let rendered = render_config(config_content).unwrap(); + assert_eq!(rendered, "# metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST}"); + } + { + let config_content = + b" # metastore_uri: ${ QW_ENV_VAR_DOES_NOT_EXIST :- default-value }"; + let rendered = render_config(config_content).unwrap(); + assert_eq!( + rendered, + " # metastore_uri: ${ QW_ENV_VAR_DOES_NOT_EXIST :- default-value }" + ); + } + } }