From 9cf1ea9b08ed745e3872c1cc81757f6078c82419 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 20 Sep 2022 12:02:07 -0600 Subject: [PATCH] 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 | 63 ++++++++++++++++++++ src/config/builder.rs | 40 ++----------- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/lib/vector-core/src/config/global_options.rs b/lib/vector-core/src/config/global_options.rs index b6aee56d32ff7..2ac3b180443bd 100644 --- a/lib/vector-core/src/config/global_options.rs +++ b/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> { + 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(this: &Option, that: &Option) -> bool { + matches!((this, that), (Some(this), Some(that)) if this != that) } diff --git a/src/config/builder.rs b/src/config/builder.rs index 44a7f9ac6c2c4..6c3acfac4f27d 100644 --- a/src/config/builder.rs +++ b/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| {