Skip to content

Commit e5c6ea2

Browse files
guilloadfulmicoton
andauthored
Ignore commented lines when rendering templated config files (#3498)
Co-authored-by: Paul Masurel <[email protected]>
1 parent 153d5c6 commit e5c6ea2

File tree

2 files changed

+79
-51
lines changed

2 files changed

+79
-51
lines changed

config/quickwit.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ version: 0.6
8080
# access_key: ${QW_AZURE_STORAGE_ACCESS_KEY}
8181
#
8282
# s3:
83-
# endpoint: ${QW_S3_ENDPOINT:-http://localhost:4566}
84-
# force_path_style_access: true
83+
# endpoint: ${QW_S3_ENDPOINT}
84+
# force_path_style_access: ${QW_S3_FORCE_PATH_STYLE_ACCESS:-true}
8585
# disable_multi_object_delete_requests: true
8686
#
8787
# -------------------------------- Metastore settings --------------------------------

quickwit/quickwit-config/src/templating.rs

Lines changed: 77 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
// along with this program. If not, see <http://www.gnu.org/licenses/>.
1919

2020
use std::collections::HashMap;
21+
use std::io::BufRead;
2122

2223
use anyhow::{bail, Context, Result};
2324
use new_string_template::template::Template;
@@ -29,7 +30,8 @@ use tracing::debug;
2930
// ENV_VAR or ENV_VAR:DEFAULT
3031
// Ignores whitespaces in curly braces
3132
static TEMPLATE_ENV_VAR_CAPTURE: Lazy<Regex> = Lazy::new(|| {
32-
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.")
33+
Regex::new(r"\$\{\s*([A-Za-z0-9_]+)\s*(?::\-\s*([\S]+)\s*)?}")
34+
.expect("The regular expression should compile.")
3335
});
3436

3537
pub fn render_config(config_content: &[u8]) -> Result<String> {
@@ -38,39 +40,54 @@ pub fn render_config(config_content: &[u8]) -> Result<String> {
3840

3941
let mut values = HashMap::new();
4042

41-
for captures in TEMPLATE_ENV_VAR_CAPTURE.captures_iter(template_str) {
42-
let env_var_key = captures
43-
.get(1)
44-
.expect("Captures should always have at least one match.")
45-
.as_str();
46-
let substitution_value = {
47-
if let Ok(env_var_value) = std::env::var(env_var_key) {
48-
debug!(
49-
env_var_name=%env_var_key,
50-
env_var_value=%env_var_value,
51-
"Environment variable is set, substituting with environment variable value."
52-
);
53-
env_var_value
54-
} else if let Some(default_match) = captures.get(2) {
55-
let default_value = default_match.as_str().to_string();
56-
debug!(
57-
env_var_name=%env_var_key,
58-
default_value=%default_value,
59-
"Environment variable is not set, substituting with default value."
60-
);
61-
default_value
62-
} else {
63-
bail!(
64-
"Failed to render config file template: environment variable `{env_var_key}` \
65-
is not set and no default value was provided."
66-
);
67-
}
68-
};
69-
values.insert(env_var_key, substitution_value);
43+
for (line_no, line_res) in config_content.lines().enumerate() {
44+
let line = line_res?;
45+
46+
for captures in TEMPLATE_ENV_VAR_CAPTURE.captures_iter(&line) {
47+
let env_var_key = captures
48+
.get(1)
49+
.expect("Captures should always have at least one match.")
50+
.as_str();
51+
let substitution_value = {
52+
if line.trim_start().starts_with('#') {
53+
debug!(
54+
env_var_name=%env_var_key,
55+
"Config file line #{line_no} is commented out, skipping."
56+
);
57+
// This line is commented out, return the line as is.
58+
captures
59+
.get(0)
60+
.expect("The 0th capture should aways be set.")
61+
.as_str()
62+
.to_string()
63+
} else if let Ok(env_var_value) = std::env::var(env_var_key) {
64+
debug!(
65+
env_var_name=%env_var_key,
66+
env_var_value=%env_var_value,
67+
"Environment variable is set, substituting with environment variable value."
68+
);
69+
env_var_value
70+
} else if let Some(default_match) = captures.get(2) {
71+
let default_value = default_match.as_str().to_string();
72+
debug!(
73+
env_var_name=%env_var_key,
74+
default_value=%default_value,
75+
"Environment variable is not set, substituting with default value."
76+
);
77+
default_value
78+
} else {
79+
bail!(
80+
"Failed to render config file template: environment variable \
81+
`{env_var_key}` is not set and no default value is provided."
82+
);
83+
}
84+
};
85+
values.insert(env_var_key.to_string(), substitution_value);
86+
}
7087
}
7188
let template = Template::new(template_str).with_regex(&TEMPLATE_ENV_VAR_CAPTURE);
7289
let rendered = template
73-
.render(&values)
90+
.render_string(&values)
7491
.context("Failed to render config file template.")?;
7592
Ok(rendered)
7693
}
@@ -94,44 +111,37 @@ mod test {
94111
}
95112

96113
#[test]
97-
fn test_template_render_whitespaces() {
114+
fn test_template_render_supports_whitespaces() {
98115
env::set_var(
99116
"TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST",
100117
"s3://test-bucket/metastore",
101118
);
102119
{
103-
let config_content = b"metastore_uri: ${TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST}";
104-
let rendered = render_config(config_content).unwrap();
105-
assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore");
106-
}
107-
{
108-
let config_content = b"metastore_uri: ${TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST }";
120+
let config_content = b"metastore_uri: ${ TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST }";
109121
let rendered = render_config(config_content).unwrap();
110122
assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore");
111123
}
124+
}
125+
126+
#[test]
127+
fn test_template_render_with_default_value() {
112128
{
113-
let config_content = b"metastore_uri: ${ TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST}";
129+
let config_content =
130+
b"metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST:-s3://test-bucket/metastore}";
114131
let rendered = render_config(config_content).unwrap();
115132
assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore");
116133
}
117134
{
118-
let config_content = b"metastore_uri: ${ TEST_TEMPLATE_RENDER_WHITESPACE_QW_TEST }";
135+
let config_content =
136+
b"metastore_uri: ${ QW_ENV_VAR_DOES_NOT_EXIST :- s3://test-bucket/metastore }";
119137
let rendered = render_config(config_content).unwrap();
120138
assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore");
121139
}
122140
}
123141

124-
#[test]
125-
fn test_template_render_default_value() {
126-
let config_content =
127-
b"metastore_uri: ${QW_NO_ENV_WITH_THIS_NAME:-s3://test-bucket/metastore}";
128-
let rendered = render_config(config_content).unwrap();
129-
assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore");
130-
}
131-
132142
#[test]
133143
fn test_template_render_should_panic() {
134-
let config_content = b"metastore_uri: ${QW_NO_ENV_WITH_THIS_NAME}";
144+
let config_content = b"metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST}";
135145
render_config(config_content).unwrap_err();
136146
}
137147

@@ -147,4 +157,22 @@ mod test {
147157
std::env::remove_var("TEST_TEMPLATE_RENDER_ENV_VAR_DEFAULT_USE_ENV");
148158
assert_eq!(rendered, "metastore_uri: s3://test-bucket/metastore");
149159
}
160+
161+
#[test]
162+
fn test_template_render_ignores_commented_lines() {
163+
{
164+
let config_content = b"# metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST}";
165+
let rendered = render_config(config_content).unwrap();
166+
assert_eq!(rendered, "# metastore_uri: ${QW_ENV_VAR_DOES_NOT_EXIST}");
167+
}
168+
{
169+
let config_content =
170+
b" # metastore_uri: ${ QW_ENV_VAR_DOES_NOT_EXIST :- default-value }";
171+
let rendered = render_config(config_content).unwrap();
172+
assert_eq!(
173+
rendered,
174+
" # metastore_uri: ${ QW_ENV_VAR_DOES_NOT_EXIST :- default-value }"
175+
);
176+
}
177+
}
150178
}

0 commit comments

Comments
 (0)