chore(config)!: Remove deprecated --strict-env-vars flag (#20422)

* chore(config)!: Remove deprecated `--strict-env-vars` flag Signed-off-by: Jesse Szwedko <[email protected]> * spelling Signed-off-by: Jesse Szwedko <[email protected]> * Remove interpolation warnings Signed-off-by: Jesse Szwedko <[email protected]> * spelling Signed-off-by: Jesse Szwedko <[email protected]> * typo Signed-off-by: Jesse Szwedko <[email protected]> * More changes to remove config loading warnings Signed-off-by: Jesse Szwedko <[email protected]> --------- Signed-off-by: Jesse Szwedko <[email protected]>
vectordotdev · May 2, 2024 · 8ed9ec2 · 8ed9ec2
1 parent ca00cc8
commit 8ed9ec2
Show file tree

Hide file tree

Showing 16 changed files with 107 additions and 223 deletions.
diff --git a/changelog.d/remove_strict_env_vars.breaking.md b/changelog.d/remove_strict_env_vars.breaking.md
@@ -0,0 +1,5 @@
+The deprecated `--strict-env-vars` flag has been removed. The previous behavior of defaulting unset
+environment variables can be accomplished by syntax like `${FOO-}` (which will default `FOO` to
+empty string if unset). See the [configuration environment variables
+docs](https://vector.dev/docs/reference/configuration/#environment-variables) for more about this
+syntax.
diff --git a/docs/DEPRECATIONS.md b/docs/DEPRECATIONS.md
@@ -17,6 +17,3 @@ For example:
 ## To be migrated
 
 ## To be removed
-
-- v0.38.0 strict_env_vars Remove option for configuring missing environment variable interpolation
-  to be a warning rather than an error
diff --git a/src/cli.rs b/src/cli.rs
@@ -1,6 +1,5 @@
 #![allow(missing_docs)]
 
-use std::sync::atomic::Ordering;
 use std::{num::NonZeroU64, path::PathBuf};
 
 use clap::{ArgAction, CommandFactory, FromArgMatches, Parser};
@@ -212,22 +211,6 @@ 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 option
-    /// is deprecated and will be removed in a future version to remove the ability to downgrade
-    /// missing environment variables to warnings.
-    #[arg(
-        long,
-        env = "VECTOR_STRICT_ENV_VARS",
-        default_value = "true",
-        default_missing_value = "true",
-        num_args = 0..=1,
-        require_equals = true,
-        action = ArgAction::Set
-    )]
-    pub strict_env_vars: bool,
 }
 
 impl RootOpts {
@@ -249,8 +232,6 @@ impl RootOpts {
     }
 
     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();
         }

diff --git a/src/config/cmd.rs b/src/config/cmd.rs
@@ -168,15 +168,15 @@ pub fn cmd(opts: &Opts) -> exitcode::ExitCode {
     // builder fields which we'll use to error out if required.
     let (paths, builder) = match process_paths(&paths) {
         Some(paths) => match load_builder_from_paths(&paths) {
-            Ok((builder, _)) => (paths, builder),
+            Ok(builder) => (paths, builder),
             Err(errs) => return handle_config_errors(errs),
         },
         None => return exitcode::CONFIG,
     };
 
     // Load source TOML.
     let source = match load_source_from_paths(&paths) {
-        Ok((map, _)) => map,
+        Ok(map) => map,
         Err(errs) => return handle_config_errors(errs),
     };
 
@@ -249,13 +249,12 @@ mod tests {
         "#,
             env_var, env_var_in_arr
         );
-        let (interpolated_config_source, _) = vars::interpolate(
+        let interpolated_config_source = vars::interpolate(
             config_source.as_ref(),
             &HashMap::from([
                 (env_var.to_string(), "syslog".to_string()),
                 (env_var_in_arr.to_string(), "in".to_string()),
             ]),
-            true,
         )
         .unwrap();
 

diff --git a/src/config/enterprise.rs b/src/config/enterprise.rs
@@ -630,7 +630,7 @@ pub(crate) fn report_configuration(
 
         // We need to create a JSON representation of config, based on the original files
         // that Vector was spawned with.
-        let (table, _) = process_paths(&config_paths)
+        let table = process_paths(&config_paths)
             .and_then(|paths| load_source_from_paths(&paths).ok())
             .expect("Couldn't load source from config paths. Please report.");
 

diff --git a/src/config/loading/config_builder.rs b/src/config/loading/config_builder.rs
@@ -33,14 +33,14 @@ impl ConfigBuilderLoader {
 
 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>), Vec<String>> {
-        let (prepared_input, warnings) = prepare_input(input)?;
+    fn prepare<R: Read>(&mut self, input: R) -> Result<String, Vec<String>> {
+        let prepared_input = prepare_input(input)?;
         let prepared_input = self
             .secrets
             .as_ref()
             .map(|s| secret::interpolate(&prepared_input, s))
             .unwrap_or(Ok(prepared_input))?;
-        Ok((prepared_input, warnings))
+        Ok(prepared_input)
     }
 
     /// Merge a TOML `Table` with a `ConfigBuilder`. Component types extend specific keys.

diff --git a/src/config/loading/loader.rs b/src/config/loading/loader.rs
@@ -54,20 +54,16 @@ pub(super) mod process {
     pub trait Process {
         /// Prepares input for serialization. This can be a useful step to interpolate
         /// environment variables or perform some other pre-processing on the input.
-        fn prepare<R: Read>(&mut self, input: R) -> Result<(String, Vec<String>), Vec<String>>;
+        fn prepare<R: Read>(&mut self, input: R) -> Result<String, Vec<String>>;
 
         /// Calls into the `prepare` method, and deserializes a `Read` to a `T`.
-        fn load<R: std::io::Read, T>(
-            &mut self,
-            input: R,
-            format: Format,
-        ) -> Result<(T, Vec<String>), Vec<String>>
+        fn load<R: std::io::Read, T>(&mut self, input: R, format: Format) -> Result<T, Vec<String>>
         where
             T: serde::de::DeserializeOwned,
         {
-            let (value, warnings) = self.prepare(input)?;
+            let value = self.prepare(input)?;
 
-            format::deserialize(&value, format).map(|builder| (builder, warnings))
+            format::deserialize(&value, format)
         }
 
         /// Helper method used by other methods to recursively handle file/dir loading, merging
@@ -77,9 +73,8 @@ pub(super) mod process {
             path: &Path,
             result: &mut Table,
             recurse: bool,
-        ) -> Result<Vec<String>, Vec<String>> {
+        ) -> Result<(), Vec<String>> {
             let mut errors = Vec::new();
-            let mut warnings = Vec::new();
             let readdir = read_dir(path)?;
 
             let mut files = Vec::new();
@@ -126,11 +121,9 @@ pub(super) mod process {
                 };
 
                 match loaded {
-                    Ok(Some((name, inner, warns))) => {
+                    Ok(Some((name, inner))) => {
                         if let Err(errs) = merge_with_value(result, name, Value::Table(inner)) {
                             errors.extend(errs);
-                        } else {
-                            warnings.extend(warns);
                         }
                     }
                     Ok(None) => {}
@@ -146,9 +139,8 @@ pub(super) mod process {
                     if let Ok(name) = component_name(&entry) {
                         if !result.contains_key(&name) {
                             match self.load_dir(&entry, true) {
-                                Ok((table, warns)) => {
+                                Ok(table) => {
                                     result.insert(name, Value::Table(table));
-                                    warnings.extend(warns);
                                 }
                                 Err(errs) => {
                                     errors.extend(errs);
@@ -160,7 +152,7 @@ pub(super) mod process {
             }
 
             if errors.is_empty() {
-                Ok(warnings)
+                Ok(())
             } else {
                 Err(errors)
             }
@@ -171,10 +163,9 @@ pub(super) mod process {
             &mut self,
             path: &Path,
             format: Format,
-        ) -> Result<Option<(String, Table, Vec<String>)>, Vec<String>> {
+        ) -> Result<Option<(String, Table)>, Vec<String>> {
             if let (Ok(name), Some(file)) = (component_name(path), open_file(path)) {
-                self.load(file, format)
-                    .map(|(value, warnings)| Some((name, value, warnings)))
+                self.load(file, format).map(|value| Some((name, value)))
             } else {
                 Ok(None)
             }
@@ -186,29 +177,25 @@ pub(super) mod process {
             &mut self,
             path: &Path,
             format: Format,
-        ) -> Result<Option<(String, Table, Vec<String>)>, Vec<String>> {
-            if let Some((name, mut table, mut warnings)) = self.load_file(path, format)? {
+        ) -> Result<Option<(String, Table)>, Vec<String>> {
+            if let Some((name, mut table)) = self.load_file(path, format)? {
                 if let Some(subdir) = path.parent().map(|p| p.join(&name)) {
                     if subdir.is_dir() && subdir.exists() {
-                        warnings.extend(self.load_dir_into(&subdir, &mut table, true)?);
+                        self.load_dir_into(&subdir, &mut table, true)?;
                     }
                 }
-                Ok(Some((name, table, warnings)))
+                Ok(Some((name, table)))
             } else {
                 Ok(None)
             }
         }
 
         /// Loads a directory (optionally, recursively), returning a TOML `Table`. This will
         /// create an initial `Table` and pass it into `load_dir_into` for recursion handling.
-        fn load_dir(
-            &mut self,
-            path: &Path,
-            recurse: bool,
-        ) -> Result<(Table, Vec<String>), Vec<String>> {
+        fn load_dir(&mut self, path: &Path, recurse: bool) -> Result<Table, Vec<String>> {
             let mut result = Table::new();
-            let warnings = self.load_dir_into(path, &mut result, recurse)?;
-            Ok((result, warnings))
+            self.load_dir_into(path, &mut result, recurse)?;
+            Ok(result)
         }
 
         /// Merge a provided TOML `Table` in an implementation-specific way. Contains an
@@ -229,18 +216,18 @@ where
 
     /// Deserializes a file with the provided format, and makes the result available via `take`.
     /// Returns a vector of non-fatal warnings on success, or a vector of error strings on failure.
-    fn load_from_file(&mut self, path: &Path, format: Format) -> Result<Vec<String>, Vec<String>> {
-        if let Some((_, table, warnings)) = self.load_file(path, format)? {
+    fn load_from_file(&mut self, path: &Path, format: Format) -> Result<(), Vec<String>> {
+        if let Some((_, table)) = self.load_file(path, format)? {
             self.merge(table, None)?;
-            Ok(warnings)
+            Ok(())
         } else {
-            Ok(vec![])
+            Ok(())
         }
     }
 
     /// Deserializes a dir with the provided format, and makes the result available via `take`.
     /// Returns a vector of non-fatal warnings on success, or a vector of error strings on failure.
-    fn load_from_dir(&mut self, path: &Path) -> Result<Vec<String>, Vec<String>> {
+    fn load_from_dir(&mut self, path: &Path) -> Result<(), Vec<String>> {
         // Iterator containing component-specific sub-folders to attempt traversing into.
         let hints = [
             ComponentHint::Source,
@@ -257,7 +244,7 @@ where
         // Get files from the root of the folder. These represent top-level config settings,
         // and need to merged down first to represent a more 'complete' config.
         let mut root = Table::new();
-        let (table, mut warnings) = self.load_dir(path, false)?;
+        let table = self.load_dir(path, false)?;
 
         // Discard the named part of the path, since these don't form any component names.
         for (_, value) in table {
@@ -277,16 +264,13 @@ where
             if path.exists() && path.is_dir() {
                 // Transforms are treated differently from other component types; they can be
                 // arbitrarily nested.
-                let (table, warns) =
-                    self.load_dir(&path, matches!(hint, ComponentHint::Transform))?;
+                let table = self.load_dir(&path, matches!(hint, ComponentHint::Transform))?;
 
                 self.merge(table, Some(hint))?;
-
-                warnings.extend(warns);
             }
         }
 
-        Ok(warnings)
+        Ok(())
     }
 }