chore(config): Move global config merging into core (#14442)

Merging two global configs has had several bugs when fields were not properly
handled. This change reorganizes the merge operation to create a new structure,
thus ensuring any new fields will be handled, and moves the operation into the
appropriate impl block, to make it easier to locate where to make the changes.

lib/vector-core/src/config/global_options.rs

@@ -4,6 +4,7 @@ use snafu::{ResultExt, Snafu};
 use vector_common::TimeZone;
 use vector_config::configurable_component;
 
+use super::super::default_data_dir;
 use super::{proxy::ProxyConfig, AcknowledgementsConfig, LogSchema};
 use crate::serde::bool_or_struct;
 
@@ -147,4 +148,66 @@ impl GlobalOptions {
             .with_context(|_| CouldNotCreateSnafu { subdir, data_dir })?;
         Ok(data_subdir)
     }
+
+    /// Merge a second global configuration into self, and return the new merged data.
+    ///
+    /// # Errors
+    ///
+    /// Returns a list of textual errors if there is a merge conflict between the two global
+    /// configs.
+    pub fn merge(&self, with: Self) -> Result<Self, Vec<String>> {
+        let mut errors = Vec::new();
+
+        if conflicts(&self.proxy.http, &with.proxy.http) {
+            errors.push("conflicting values for 'proxy.http' found".to_owned());
+        }
+
+        if conflicts(&self.proxy.https, &with.proxy.https) {
+            errors.push("conflicting values for 'proxy.https' found".to_owned());
+        }
+
+        if !self.proxy.no_proxy.is_empty() && !with.proxy.no_proxy.is_empty() {
+            errors.push("conflicting values for 'proxy.no_proxy' found".to_owned());
+        }
+
+        let data_dir = if self.data_dir.is_none() || self.data_dir == default_data_dir() {
+            with.data_dir
+        } else if with.data_dir != default_data_dir() && self.data_dir != with.data_dir {
+            // If two configs both set 'data_dir' and have conflicting values
+            // we consider this an error.
+            errors.push("conflicting values for 'data_dir' found".to_owned());
+            None
+        } else {
+            self.data_dir.clone()
+        };
+
+        // If the user has multiple config files, we must *merge* log schemas
+        // until we meet a conflict, then we are allowed to error.
+        let mut log_schema = self.log_schema.clone();
+        if let Err(merge_errors) = log_schema.merge(&with.log_schema) {
+            errors.extend(merge_errors);
+        }
+
+        if self.timezone != with.timezone {
+            errors.push("conflicting values for 'timezone' found".to_owned());
+        }
+
+        if errors.is_empty() {
+            Ok(Self {
+                data_dir,
+                log_schema,
+                acknowledgements: self.acknowledgements.merge_default(&with.acknowledgements),
+                timezone: self.timezone,
+                proxy: self.proxy.merge(&with.proxy),
+                expire_metrics: self.expire_metrics.or(with.expire_metrics),
+                expire_metrics_secs: self.expire_metrics_secs.or(with.expire_metrics_secs),
+            })
+        } else {
+            Err(errors)
+        }
+    }
+}
+
+fn conflicts<T: PartialEq>(this: &Option<T>, that: &Option<T>) -> bool {
+    matches!((this, that), (Some(this), Some(that)) if this != that)
 }

src/config/builder.rs

@@ -6,7 +6,7 @@ use indexmap::IndexMap;
 #[cfg(feature = "enterprise")]
 use serde_json::Value;
 use vector_config::configurable_component;
-use vector_core::{config::GlobalOptions, default_data_dir};
+use vector_core::config::GlobalOptions;
 
 use crate::{
     enrichment_tables::EnrichmentTables, providers::Providers, secrets::SecretBackends,
@@ -318,47 +318,15 @@ impl ConfigBuilder {
 
         self.provider = with.provider;
 
-        if self.global.proxy.http.is_some() && with.global.proxy.http.is_some() {
-            errors.push("conflicting values for 'proxy.http' found".to_owned());
+        match self.global.merge(with.global) {
+            Err(errs) => errors.extend(errs),
+            Ok(new_global) => self.global = new_global,
         }
 
-        if self.global.proxy.https.is_some() && with.global.proxy.https.is_some() {
-            errors.push("conflicting values for 'proxy.https' found".to_owned());
-        }
-
-        if !self.global.proxy.no_proxy.is_empty() && !with.global.proxy.no_proxy.is_empty() {
-            errors.push("conflicting values for 'proxy.no_proxy' found".to_owned());
-        }
-
-        self.global.proxy = self.global.proxy.merge(&with.global.proxy);
-
-        self.global.expire_metrics = self.global.expire_metrics.or(with.global.expire_metrics);
-
-        self.global.expire_metrics_secs = self
-            .global
-            .expire_metrics_secs
-            .or(with.global.expire_metrics_secs);
-
         self.schema.append(with.schema, &mut errors);
 
         self.schema.log_namespace = self.schema.log_namespace.or(with.schema.log_namespace);
 
-        if self.global.data_dir.is_none() || self.global.data_dir == default_data_dir() {
-            self.global.data_dir = with.global.data_dir;
-        } else if with.global.data_dir != default_data_dir()
-            && self.global.data_dir != with.global.data_dir
-        {
-            // If two configs both set 'data_dir' and have conflicting values
-            // we consider this an error.
-            errors.push("conflicting values for 'data_dir' found".to_owned());
-        }
-
-        // If the user has multiple config files, we must *merge* log schemas
-        // until we meet a conflict, then we are allowed to error.
-        if let Err(merge_errors) = self.global.log_schema.merge(&with.global.log_schema) {
-            errors.extend(merge_errors);
-        }
-
         self.healthchecks.merge(with.healthchecks);
 
         with.enrichment_tables.keys().for_each(|k| {