feat(config)!: parse config first, then interpolate into string leaves

changelog.d/parse_first_interpolation.breaking.md

@@ -0,0 +1,50 @@
+# Config is now parsed before environment variable and secret interpolation
+
+## Summary
+
+Vector now parses configuration files into a typed value tree before performing
+environment variable (`${VAR}`) and `SECRET[...]` substitution. Interpolation operates
+only on string-typed leaves in the parsed tree. An unquoted placeholder in a non-string
+position of a TOML or JSON config (e.g. `count = ${MY_COUNT}`) is no longer valid and
+will cause the config to fail to load.
+
+YAML configurations are unaffected: YAML parses `${VAR}` as a string scalar, and the
+new schema-coercion pass converts that string to the declared type at load time.
+
+## Migration
+
+In TOML or JSON, wrap any placeholder that appears in a non-string field in quotes so
+the parser sees a string scalar. Vector will coerce the value to the declared type
+(integer, boolean, float) automatically.
+
+Before (TOML):
+
+```toml
+[sources.in]
+type = "demo_logs"
+count = ${MY_COUNT}
+```
+
+After (TOML):
+
+```toml
+[sources.in]
+type = "demo_logs"
+count = "${MY_COUNT}"
+```
+
+Before (JSON):
+
+```json
+{ "sources": { "in": { "type": "demo_logs", "count": ${MY_COUNT} } } }
+```
+
+After (JSON):
+
+```json
+{ "sources": { "in": { "type": "demo_logs", "count": "${MY_COUNT}" } } }
+```
+
+The same applies to `SECRET[backend.key]` references in non-string fields.
+
+authors: pront

changelog.d/parse_first_interpolation.enhancement.md

@@ -0,0 +1,8 @@
+Vector now parses configuration files before performing environment variable and `SECRET[...]` substitution.
+Interpolation runs only on string-valued leaves in the parsed value tree, and a JSON-Schema-driven coercion
+pass converts string scalars to the types declared by each component. This produces clearer error messages
+with full field paths (e.g. `sources.my_source.count`) and makes the `--disable-env-var-interpolation` flag
+behave consistently across the `vector`, `vector validate`, and `vector config` commands as well as the
+HTTP provider.
+
+authors: pront

src/config/cmd.rs

@@ -4,7 +4,8 @@ use clap::Parser;
 use serde_json::Value;
 
 use super::{ConfigBuilder, load_source_from_paths, loading::ConfigBuilderLoader, process_paths};
-use crate::{cli::handle_config_errors, config};
+use crate::cli::handle_config_errors;
+use crate::config;
 
 #[derive(Parser, Debug, Clone)]
 #[command(rename_all = "kebab-case")]
@@ -213,13 +214,15 @@ mod tests {
         SinkDescription, SourceDescription, TransformDescription,
     };
 
-    use super::merge_json;
+    use crate::config::{Format, interpolate};
     use crate::{
-        config::{ConfigBuilder, Format, cmd::serialize_to_json, vars},
+        config::{ConfigBuilder, cmd::serialize_to_json},
         generate,
         generate::{TransformInputsStrategy, generate_example},
     };
 
+    use super::merge_json;
+
     #[test]
     fn test_array_override() {
         let mut json = json!({
@@ -248,14 +251,15 @@ mod tests {
             r#"
             [sources.in]
             type = "demo_logs"
-            format = "${{{env_var}}}"
+            format = "${{{}}}"
 
             [sinks.out]
             type = "blackhole"
-            inputs = ["${{{env_var_in_arr}}}"]
-        "#
+            inputs = ["${{{}}}"]
+        "#,
+            env_var, env_var_in_arr
         );
-        let interpolated_config_source = vars::interpolate(
+        let interpolated_config_source = interpolate(
             config_source.as_ref(),
             &HashMap::from([
                 (env_var.to_string(), "syslog".to_string()),
@@ -320,24 +324,19 @@ mod tests {
         // We also append a fixed `remap` transform to the transforms list. This
         // ensures sink inputs are consistent since `generate` uses the last
         // transform the input for each sink.
+        fn pair(s: &&str) -> String {
+            format!("{s}:{s}")
+        }
         let generate_config_str = format!(
             "{}/{}/{}",
-            sources
-                .iter()
-                .map(|source| format!("{source}:{source}"))
-                .collect::<Vec<_>>()
-                .join(","),
+            sources.iter().map(pair).collect::<Vec<_>>().join(","),
             transforms
                 .iter()
-                .map(|transform| format!("{transform}:{transform}"))
+                .map(pair)
                 .chain(vec!["manually-added-remap:remap".to_string()])
                 .collect::<Vec<_>>()
                 .join(","),
-            sinks
-                .iter()
-                .map(|sink| format!("{sink}:{sink}"))
-                .collect::<Vec<_>>()
-                .join(","),
+            sinks.iter().map(pair).collect::<Vec<_>>().join(","),
         );
         let opts = generate::Opts {
             fragment: true,

src/config/loading/config_builder.rs

@@ -1,9 +1,12 @@
-use std::{collections::HashMap, io::Read};
+use std::collections::HashMap;
 
 use indexmap::IndexMap;
 use toml::value::Table;
 
-use super::{ComponentHint, Process, deserialize_table, loader, prepare_input, secret};
+use super::{
+    ComponentHint, ConfigPath, Format, Process, deserialize_table, deserialize_table_wrapped,
+    env_var_interpolation_enabled, interpolate_toml_table_with_secrets, loader, loader_from_paths,
+};
 use crate::config::{
     ComponentKey, ConfigBuilder, EnrichmentTableOuter, SinkOuter, SourceOuter, TestDefinition,
     TransformOuter,
@@ -38,16 +41,16 @@ impl ConfigBuilderLoader {
     /// Builds the ConfigBuilderLoader and loads configuration from the specified paths.
     pub fn load_from_paths(
         self,
-        config_paths: &[super::ConfigPath],
+        config_paths: &[ConfigPath],
     ) -> Result<ConfigBuilder, Vec<String>> {
-        super::loader_from_paths(self, config_paths)
+        loader_from_paths(self, config_paths)
     }
 
     /// Builds the ConfigBuilderLoader and loads configuration from an input reader.
-    pub fn load_from_input<R: Read>(
+    pub fn load_from_input<R: std::io::Read>(
         self,
         input: R,
-        format: super::Format,
+        format: Format,
     ) -> Result<ConfigBuilder, Vec<String>> {
         super::loader_from_input(self, input, format)
     }
@@ -58,48 +61,53 @@ impl Default for ConfigBuilderLoader {
         Self {
             builder: ConfigBuilder::default(),
             secrets: HashMap::new(),
-            interpolate_env: super::env_var_interpolation_enabled(),
+            interpolate_env: env_var_interpolation_enabled(),
         }
     }
 }
 
 impl Process for ConfigBuilderLoader {
-    /// Prepares input for a `ConfigBuilder` by interpolating environment variables.
-    fn prepare<R: Read>(&mut self, input: R) -> Result<String, Vec<String>> {
-        let prepared_input = prepare_input(input, self.interpolate_env)?;
-        Ok(if self.secrets.is_empty() {
-            prepared_input
+    fn should_interpolate_env(&self) -> bool {
+        self.interpolate_env
+    }
+
+    fn postprocess(&mut self, table: Table) -> Result<Table, Vec<String>> {
+        if self.secrets.is_empty() {
+            Ok(table)
         } else {
-            secret::interpolate(&prepared_input, &self.secrets)?
-        })
+            interpolate_toml_table_with_secrets(&table, &self.secrets)
+        }
     }
 
-    /// Merge a TOML `Table` with a `ConfigBuilder`. Component types extend specific keys.
     fn merge(&mut self, table: Table, hint: Option<ComponentHint>) -> Result<(), Vec<String>> {
         match hint {
             Some(ComponentHint::Source) => {
-                self.builder.sources.extend(deserialize_table::<
+                self.builder.sources.extend(deserialize_table_wrapped::<
                     IndexMap<ComponentKey, SourceOuter>,
-                >(table)?);
+                >(table, "sources")?);
             }
             Some(ComponentHint::Sink) => {
-                self.builder.sinks.extend(
-                    deserialize_table::<IndexMap<ComponentKey, SinkOuter<_>>>(table)?,
-                );
+                self.builder.sinks.extend(deserialize_table_wrapped::<
+                    IndexMap<ComponentKey, SinkOuter<_>>,
+                >(table, "sinks")?);
             }
             Some(ComponentHint::Transform) => {
-                self.builder.transforms.extend(deserialize_table::<
+                self.builder.transforms.extend(deserialize_table_wrapped::<
                     IndexMap<ComponentKey, TransformOuter<_>>,
-                >(table)?);
+                >(table, "transforms")?);
             }
             Some(ComponentHint::EnrichmentTable) => {
-                self.builder.enrichment_tables.extend(deserialize_table::<
-                    IndexMap<ComponentKey, EnrichmentTableOuter<_>>,
-                >(table)?);
+                self.builder
+                    .enrichment_tables
+                    .extend(deserialize_table_wrapped::<
+                        IndexMap<ComponentKey, EnrichmentTableOuter<_>>,
+                    >(table, "enrichment_tables")?);
             }
             Some(ComponentHint::Test) => {
-                // This serializes to a `Vec<TestDefinition<_>>`, so we need to first expand
-                // it to an ordered map, and then pull out the value, ignoring the keys.
+                // Tests are loaded as a name -> TestDefinition map from
+                // namespaced dirs and converted to Vec<TestDefinition> at the
+                // builder; the schema represents tests as a Vec, so this branch
+                // skips the wrap-and-coerce path that the component hints use.
                 self.builder.tests.extend(
                     deserialize_table::<IndexMap<String, TestDefinition<String>>>(table)?
                         .into_iter()
@@ -116,103 +124,7 @@ impl Process for ConfigBuilderLoader {
 }
 
 impl loader::Loader<ConfigBuilder> for ConfigBuilderLoader {
-    /// Returns the resulting `ConfigBuilder`.
     fn take(self) -> ConfigBuilder {
         self.builder
     }
 }
-
-#[cfg(all(
-    test,
-    feature = "sinks-elasticsearch",
-    feature = "transforms-sample",
-    feature = "sources-demo_logs",
-    feature = "sinks-console"
-))]
-mod tests {
-    use std::path::PathBuf;
-
-    use super::ConfigBuilderLoader;
-    use crate::config::{ComponentKey, ConfigPath};
-
-    #[test]
-    fn load_namespacing_folder() {
-        let path = PathBuf::from(".")
-            .join("tests")
-            .join("namespacing")
-            .join("success");
-        let configs = vec![ConfigPath::Dir(path)];
-        let builder = ConfigBuilderLoader::default()
-            .interpolate_env(true)
-            .load_from_paths(&configs)
-            .unwrap();
-        assert!(
-            builder
-                .transforms
-                .contains_key(&ComponentKey::from("apache_parser"))
-        );
-        assert!(
-            builder
-                .sources
-                .contains_key(&ComponentKey::from("apache_logs"))
-        );
-        assert!(
-            builder
-                .sinks
-                .contains_key(&ComponentKey::from("es_cluster"))
-        );
-        assert_eq!(builder.tests.len(), 2);
-    }
-
-    #[test]
-    fn load_namespacing_ignore_invalid() {
-        let path = PathBuf::from(".")
-            .join("tests")
-            .join("namespacing")
-            .join("ignore-invalid");
-        let configs = vec![ConfigPath::Dir(path)];
-        ConfigBuilderLoader::default()
-            .interpolate_env(true)
-            .load_from_paths(&configs)
-            .unwrap();
-    }
-
-    #[test]
-    fn load_directory_ignores_unknown_file_formats() {
-        let path = PathBuf::from(".")
-            .join("tests")
-            .join("config-dir")
-            .join("ignore-unknown");
-        let configs = vec![ConfigPath::Dir(path)];
-        ConfigBuilderLoader::default()
-            .interpolate_env(true)
-            .load_from_paths(&configs)
-            .unwrap();
-    }
-
-    #[test]
-    fn load_directory_globals() {
-        let path = PathBuf::from(".")
-            .join("tests")
-            .join("config-dir")
-            .join("globals");
-        let configs = vec![ConfigPath::Dir(path)];
-        ConfigBuilderLoader::default()
-            .interpolate_env(true)
-            .load_from_paths(&configs)
-            .unwrap();
-    }
-
-    #[test]
-    fn load_directory_globals_duplicates() {
-        let path = PathBuf::from(".")
-            .join("tests")
-            .join("config-dir")
-            .join("globals-duplicate");
-        let configs = vec![ConfigPath::Dir(path)];
-        ConfigBuilderLoader::default()
-            .interpolate_env(true)
-            .load_from_paths(&configs)
-            .unwrap();
-    }
-}