chore(config): Add option to turn missing env vars in config into an …

…error

Using missing environment variables in configurations without a default is a
common cause of broken configurations. This starts the process of turning them
into a hard error by allowing users to opt into the new behavior.

docs/DEPRECATIONS.md

@@ -10,12 +10,14 @@ The format for each entry should be: `<version> <identifier> <description>`.
 
 For example:
 
-- legacy_openssl_provider v0.34.0 OpenSSL legacy provider flag should be removed
+- v0.34.0 legacy_openssl_provider OpenSSL legacy provider flag should be removed
 
 ## To be deprecated
 
+- v0.37.0 strict_env_vars Add deprecation warning for missing environment variable interpolation.
+
 ## To be migrated
 
 ## To be removed
 
-- http_internal_metrics v0.35.0 `requests_completed_total`, `request_duration_seconds`, and `requests_received_total` internal metrics should be removed.
+- v0.35.0 http_internal_metrics `requests_completed_total`, `request_duration_seconds`, and `requests_received_total` internal metrics should be removed.

src/app.rs

@@ -16,8 +16,6 @@ use crate::config::enterprise::{
     EnterpriseReporter,
 };
 use crate::extra_context::ExtraContext;
-#[cfg(not(feature = "enterprise-tests"))]
-use crate::metrics;
 #[cfg(feature = "api")]
 use crate::{api, internal_events::ApiStarted};
 use crate::{
@@ -195,7 +193,7 @@ impl Application {
         opts: Opts,
         extra_context: ExtraContext,
     ) -> Result<(Runtime, Self), ExitCode> {
-        init_global(!opts.root.openssl_no_probe);
+        opts.root.init_global();
 
         let color = opts.root.color.use_color();
 
@@ -445,15 +443,6 @@ impl FinishedApplication {
     }
 }
 
-pub fn init_global(openssl_probe: bool) {
-    if openssl_probe {
-        openssl_probe::init_ssl_cert_env_vars();
-    }
-
-    #[cfg(not(feature = "enterprise-tests"))]
-    metrics::init_global().expect("metrics initialization failed");
-}
-
 fn get_log_levels(default: &str) -> String {
     std::env::var("VECTOR_LOG")
         .or_else(|_| {

src/cli.rs

@@ -1,5 +1,6 @@
 #![allow(missing_docs)]
 
+use std::sync::atomic::Ordering;
 use std::{num::NonZeroU64, path::PathBuf};
 
 use clap::{ArgAction, CommandFactory, FromArgMatches, Parser};
@@ -211,6 +212,13 @@ pub struct RootOpts {
     /// `--watch-config`.
     #[arg(long, env = "VECTOR_ALLOW_EMPTY_CONFIG", default_value = "false")]
     pub allow_empty_config: bool,
+
+    /// Turn on strict mode for environment variable interpolation. When set, interpolation of a
+    /// missing environment variable in configuration files will cause an error instead of a
+    /// warning, which will result in a failure to load any such configuration file. This defaults
+    /// to false, but that default is deprecated and will be changed to strict in future versions.
+    #[arg(long, env = "VECTOR_STRICT_ENV_VARS", default_value = "false")]
+    pub strict_env_vars: bool,
 }
 
 impl RootOpts {
@@ -230,6 +238,17 @@ impl RootOpts {
         )
         .collect()
     }
+
+    pub fn init_global(&self) {
+        crate::config::STRICT_ENV_VARS.store(self.strict_env_vars, Ordering::Relaxed);
+
+        if !self.openssl_no_probe {
+            openssl_probe::init_ssl_cert_env_vars();
+        }
+
+        #[cfg(not(feature = "enterprise-tests"))]
+        crate::metrics::init_global().expect("metrics initialization failed");
+    }
 }
 
 #[derive(Parser, Debug)]

src/config/loading/mod.rs

@@ -8,6 +8,7 @@ use std::{
     fmt::Debug,
     fs::{File, ReadDir},
     path::{Path, PathBuf},
+    sync::atomic::{AtomicBool, Ordering},
     sync::Mutex,
 };
 
@@ -27,6 +28,18 @@ use crate::{config::ProviderConfig, signal};
 
 pub static CONFIG_PATHS: Mutex<Vec<ConfigPath>> = Mutex::new(Vec::new());
 
+// Technically, this global should be a parameter to the `config::vars::interpolate` function, which
+// is passed in from its caller `prepare_input` below, etc. However:
+//
+// 1. That ended up needing to have the parameter added to literally dozens of functions, as
+// `prepare_input` has long chains of callers.
+//
+// 2. This variable is only ever set in one place, at program global startup from the command line.
+//
+// 3. This setting is intended to be transitional, anyways, and is marked as deprecated to be
+// removed in a future version after strict mode becomes the default.
+pub static STRICT_ENV_VARS: AtomicBool = AtomicBool::new(false);
+
 pub(super) fn read_dir<P: AsRef<Path> + Debug>(path: P) -> Result<ReadDir, Vec<String>> {
     path.as_ref()
         .read_dir()
@@ -293,7 +306,11 @@ pub fn prepare_input<R: std::io::Read>(mut input: R) -> Result<(String, Vec<Stri
             vars.insert("HOSTNAME".into(), hostname);
         }
     }
-    vars::interpolate(&source_string, &vars)
+    vars::interpolate(
+        &source_string,
+        &vars,
+        STRICT_ENV_VARS.load(Ordering::Relaxed),
+    )
 }
 
 pub fn load<R: std::io::Read, T>(input: R, format: Format) -> Result<(T, Vec<String>), Vec<String>>

src/config/mod.rs

@@ -53,7 +53,7 @@ pub use id::{ComponentKey, Inputs};
 pub use loading::{
     load, load_builder_from_paths, load_from_paths, load_from_paths_with_provider_and_secrets,
     load_from_str, load_source_from_paths, merge_path_lists, process_paths, COLLECTOR,
-    CONFIG_PATHS,
+    CONFIG_PATHS, STRICT_ENV_VARS,
 };
 pub use provider::ProviderConfig;
 pub use secret::SecretBackend;

src/config/vars.rs

@@ -24,6 +24,7 @@ pub static ENVIRONMENT_VARIABLE_INTERPOLATION_REGEX: Lazy<Regex> = Lazy::new(||
 pub fn interpolate(
     input: &str,
     vars: &HashMap<String, String>,
+    strict_vars: bool,
 ) -> Result<(String, Vec<String>), Vec<String>> {
     let mut errors = Vec::new();
     let mut warnings = Vec::new();
@@ -47,22 +48,27 @@ pub fn interpolate(
                             Some(v) if !v.is_empty() => v,
                             _ => {
                                 errors.push(format!(
-                                    "Non-empty env var required in config. name = {:?}, error = {:?}",
-                                    name, def_or_err
+                                    "Non-empty environment variable required in config. name = {name:?}, error = {def_or_err:?}",
                                 ));
                                 ""
                             },
                         }
                         "?" => val.unwrap_or_else(|| {
                             errors.push(format!(
-                                "Missing env var required in config. name = {:?}, error = {:?}",
-                                name, def_or_err
+                                "Missing environment variable required in config. name = {name:?}, error = {def_or_err:?}",
                             ));
                             ""
                         }),
-                        _ => val.unwrap_or_else(|| {
+                        _ => val.unwrap_or_else(|| if strict_vars {
+                            errors.push(format!(
+                                "Missing environment variable in config. name = {name:?}",
+                            ));
+                            ""
+                        } else {
                             warnings
-                                .push(format!("Unknown env var in config. name = {:?}", name));
+                                .push(format!(
+                                    "Unknown environment variable in config. name = {name:?}",
+                                ));
                             ""
                         }),
                     }
@@ -94,34 +100,53 @@ mod test {
         .into_iter()
         .collect();
 
-        assert_eq!("dogs", interpolate("$FOO", &vars).unwrap().0);
-        assert_eq!("dogs", interpolate("${FOO}", &vars).unwrap().0);
-        assert_eq!("cats", interpolate("${FOOBAR}", &vars).unwrap().0);
-        assert_eq!("xcatsy", interpolate("x${FOOBAR}y", &vars).unwrap().0);
-        assert_eq!("x", interpolate("x$FOOBARy", &vars).unwrap().0);
-        assert_eq!("$ x", interpolate("$ x", &vars).unwrap().0);
-        assert_eq!("$FOO", interpolate("$$FOO", &vars).unwrap().0);
-        assert_eq!("dogs=bar", interpolate("$FOO=bar", &vars).unwrap().0);
-        assert_eq!("", interpolate("$NOT_FOO", &vars).unwrap().0);
-        assert_eq!("-FOO", interpolate("$NOT-FOO", &vars).unwrap().0);
-        assert_eq!("turtles", interpolate("$FOO.BAR", &vars).unwrap().0);
-        assert_eq!("${FOO x", interpolate("${FOO x", &vars).unwrap().0);
-        assert_eq!("${}", interpolate("${}", &vars).unwrap().0);
-        assert_eq!("dogs", interpolate("${FOO:-cats}", &vars).unwrap().0);
-        assert_eq!("dogcats", interpolate("${NOT:-dogcats}", &vars).unwrap().0);
+        assert_eq!("dogs", interpolate("$FOO", &vars, true).unwrap().0);
+        assert_eq!("dogs", interpolate("${FOO}", &vars, true).unwrap().0);
+        assert_eq!("cats", interpolate("${FOOBAR}", &vars, true).unwrap().0);
+        assert_eq!("xcatsy", interpolate("x${FOOBAR}y", &vars, true).unwrap().0);
+        assert_eq!("x", interpolate("x$FOOBARy", &vars, false).unwrap().0);
+        assert!(interpolate("x$FOOBARy", &vars, true).is_err());
+        assert_eq!("$ x", interpolate("$ x", &vars, false).unwrap().0);
+        assert_eq!("$ x", interpolate("$ x", &vars, true).unwrap().0);
+        assert_eq!("$FOO", interpolate("$$FOO", &vars, true).unwrap().0);
+        assert_eq!("dogs=bar", interpolate("$FOO=bar", &vars, true).unwrap().0);
+        assert_eq!("", interpolate("$NOT_FOO", &vars, false).unwrap().0);
+        assert!(interpolate("$NOT_FOO", &vars, true).is_err());
+        assert_eq!("-FOO", interpolate("$NOT-FOO", &vars, false).unwrap().0);
+        assert!(interpolate("$NOT-FOO", &vars, true).is_err());
+        assert_eq!("turtles", interpolate("$FOO.BAR", &vars, true).unwrap().0);
+        assert_eq!("${FOO x", interpolate("${FOO x", &vars, true).unwrap().0);
+        assert_eq!("${}", interpolate("${}", &vars, true).unwrap().0);
+        assert_eq!("dogs", interpolate("${FOO:-cats}", &vars, true).unwrap().0);
+        assert_eq!(
+            "dogcats",
+            interpolate("${NOT:-dogcats}", &vars, true).unwrap().0
+        );
         assert_eq!(
             "dogs and cats",
-            interpolate("${NOT:-dogs and cats}", &vars).unwrap().0
+            interpolate("${NOT:-dogs and cats}", &vars, true).unwrap().0
+        );
+        assert_eq!(
+            "${:-cats}",
+            interpolate("${:-cats}", &vars, true).unwrap().0
+        );
+        assert_eq!("", interpolate("${NOT:-}", &vars, true).unwrap().0);
+        assert_eq!("cats", interpolate("${NOT-cats}", &vars, true).unwrap().0);
+        assert_eq!("", interpolate("${EMPTY-cats}", &vars, true).unwrap().0);
+        assert_eq!(
+            "dogs",
+            interpolate("${FOO:?error cats}", &vars, true).unwrap().0
+        );
+        assert_eq!(
+            "dogs",
+            interpolate("${FOO?error cats}", &vars, true).unwrap().0
+        );
+        assert_eq!(
+            "",
+            interpolate("${EMPTY?error cats}", &vars, true).unwrap().0
         );
-        assert_eq!("${:-cats}", interpolate("${:-cats}", &vars).unwrap().0);
-        assert_eq!("", interpolate("${NOT:-}", &vars).unwrap().0);
-        assert_eq!("cats", interpolate("${NOT-cats}", &vars).unwrap().0);
-        assert_eq!("", interpolate("${EMPTY-cats}", &vars).unwrap().0);
-        assert_eq!("dogs", interpolate("${FOO:?error cats}", &vars).unwrap().0);
-        assert_eq!("dogs", interpolate("${FOO?error cats}", &vars).unwrap().0);
-        assert_eq!("", interpolate("${EMPTY?error cats}", &vars).unwrap().0);
-        assert!(interpolate("${NOT:?error cats}", &vars).is_err());
-        assert!(interpolate("${NOT?error cats}", &vars).is_err());
-        assert!(interpolate("${EMPTY:?error cats}", &vars).is_err());
+        assert!(interpolate("${NOT:?error cats}", &vars, true).is_err());
+        assert!(interpolate("${NOT?error cats}", &vars, true).is_err());
+        assert!(interpolate("${EMPTY:?error cats}", &vars, true).is_err());
     }
 }